-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: better define the api bounderies of the module #32
base: main
Are you sure you want to change the base?
Changes from 13 commits
b973057
1d8ecbe
c592226
eb1d377
8ef3e2e
73809f8
2b5d438
fcbc2c2
135028b
015bf0f
2967c17
289d76d
0c6443e
7c5b372
7622867
3a5f229
f7acc94
5117f9b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
component_management: | ||
default_rules: | ||
statuses: | ||
- type: project | ||
target: auto | ||
- type: patch | ||
target: auto | ||
individual_components: | ||
- component_id: server | ||
name: Server | ||
paths: | ||
- "apps/server/src/**" | ||
- component_id: config | ||
name: Config | ||
paths: | ||
- libs/config/src/** | ||
- component_id: web | ||
name: Web | ||
paths: | ||
- libs/web/src/** | ||
- component_id: utils | ||
name: Utils | ||
paths: | ||
- libs/util/src/** | ||
- component_id: all_tests | ||
name: All Tests | ||
paths: | ||
- "apps/*/test/**" | ||
- "libs/*/test/**" | ||
|
||
comment: | ||
layout: diff, flags, components, files, header, footer | ||
hide_project_coverage: false | ||
show_carryforward_flags: true |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,6 @@ permissions: | |
actions: read | ||
contents: read | ||
|
||
|
||
jobs: | ||
main: | ||
runs-on: ubuntu-22.04 | ||
|
@@ -25,8 +24,9 @@ jobs: | |
# Run this command as early as possible, before dependencies are installed | ||
# Learn more at https://nx.dev/ci/reference/nx-cloud-cli#npx-nxcloud-startcirun | ||
# Connect your workspace by running "nx connect" and uncomment this | ||
# - run: pnpm dlx nx-cloud start-ci-run --distribute-on="3 linux-medium-js" --stop-agents-after="build" | ||
|
||
- run: pnpm dlx nx-cloud start-ci-run --distribute-on="3 linux-medium-js" --stop-agents-after="build" | ||
env: | ||
NX_CLOUD_AUTH_TOKEN: ${{ secrets.NX_CLOUD_AUTH_TOKEN }} | ||
# Cache node_modules | ||
- uses: actions/setup-node@v3 | ||
with: | ||
|
@@ -35,19 +35,21 @@ jobs: | |
|
||
- run: pnpm install --frozen-lockfile | ||
- uses: nrwl/nx-set-shas@v4 | ||
|
||
env: | ||
NX_CLOUD_AUTH_TOKEN: ${{ secrets.NX_CLOUD_AUTH_TOKEN }} | ||
# Prepend any command with "nx-cloud record --" to record its logs to Nx Cloud | ||
# - run: pnpm exec nx-cloud record -- echo Hello World | ||
# Nx Affected runs only tasks affected by the changes in this PR/commit. Learn more: https://nx.dev/ci/features/affected | ||
- run: make test | ||
env: | ||
NX_CLOUD_AUTH_TOKEN: ${{ secrets.NX_CLOUD_AUTH_TOKEN }} | ||
|
||
- name: Upload test results to Codecov | ||
if: ${{ !cancelled() }} | ||
uses: codecov/test-results-action@v1 | ||
with: | ||
token: ${{ secrets.CODECOV_TOKEN }} | ||
- name: Upload coverage reports to Codecov | ||
uses: codecov/[email protected] | ||
run: make codecov | ||
with: | ||
token: ${{ secrets.CODECOV_TOKEN }} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,20 @@ | ||
test: | ||
./scripts/inject-token.sh | ||
pnpm exec nx affected -t lint build test | ||
pnpm exec nx affected -t lint test build | ||
|
||
@PHONY: test | ||
codecov_install: | ||
pip install codecov-cli | ||
|
||
codecov: codecov_install | ||
codecovcli --verbose upload-process --fail-on-error -t ${{ secrets.CODECOV_TOKEN }} -n Server -F Server -f apps/server/coverage/lcov.info | ||
codecovcli --verbose upload-process --fail-on-error -t ${{ secrets.CODECOV_TOKEN }} -n Config -F Config -f libs/config/coverage/lcov.info | ||
codecovcli --verbose upload-process --fail-on-error -t ${{ secrets.CODECOV_TOKEN }} -n Utils -F Utils -f libs/utils/coverage/lcov.info | ||
drazisil marked this conversation as resolved.
Show resolved
Hide resolved
|
||
codecovcli --verbose upload-process --fail-on-error -t ${{ secrets.CODECOV_TOKEN }} -n Web -F Web -f libs/web/coverage/lcov.info | ||
|
||
codecov_local: | ||
codecovcli --verbose upload-process --fail-on-error -n Server -F Server -f apps/server/coverage/lcov.info | ||
codecovcli --verbose upload-process --fail-on-error -n Config -F Config -f libs/config/coverage/lcov.info | ||
codecovcli --verbose upload-process --fail-on-error -n Utils -F Utils -f libs/utils/coverage/lcov.info | ||
Comment on lines
+16
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip Codebase Verification Ensure coverage files exist for local uploads. The specified coverage files for local uploads do not exist:
Please verify the presence of these files or ensure they are generated correctly. Analysis chainEnsure coverage files exist for local uploads. Verify that the specified coverage files exist for all components in local uploads. Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify that the coverage files specified in the `codecov_local` target exist.
# Test: Check for the existence of the coverage files. Expect: All specified coverage files should exist.
fd --type f 'apps/server/coverage/lcov.info' && fd --type f 'libs/config/coverage/lcov.info' && fd --type f 'libs/utils/coverage/lcov.info' && fd --type f 'libs/web/coverage/lcov.info'
Length of output: 175 Script: #!/bin/bash
# Description: Verify that the coverage files specified in the `codecov_local` target exist.
# Check for the existence of each coverage file and provide detailed output.
echo "Checking apps/server/coverage/lcov.info"
fd --type f 'apps/server/coverage/lcov.info' || echo "File not found: apps/server/coverage/lcov.info"
echo "Checking libs/config/coverage/lcov.info"
fd --type f 'libs/config/coverage/lcov.info' || echo "File not found: libs/config/coverage/lcov.info"
echo "Checking libs/utils/coverage/lcov.info"
fd --type f 'libs/utils/coverage/lcov.info' || echo "File not found: libs/utils/coverage/lcov.info"
echo "Checking libs/web/coverage/lcov.info"
fd --type f 'libs/web/coverage/lcov.info' || echo "File not found: libs/web/coverage/lcov.info"
Length of output: 523 |
||
codecovcli --verbose upload-process --fail-on-error -n Web -F Web -f libs/web/coverage/lcov.info | ||
|
||
|
||
@PHONY: test codecov_install codecov |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
NX_CLOUD_AUTH_TOKEN= | ||
SENTRY_DSN= |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ | |
import { WrappedServer } from "./WrappedServer.js"; | ||
import { exit } from "node:process"; | ||
import { log } from "@rusty/util"; | ||
import { handleWebRequests } from "@rusty/web"; | ||
import { processWebRequests } from "@rusty/web"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip Codebase Verification Ensure The import
Analysis chainVerify the usage of the new import and ensure the old import is removed. The Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify the usage of `processWebRequests` and ensure `handleWebRequests` is removed.
# Test: Search for the usage of `processWebRequests`. Expect: Only occurances of `processWebRequests`.
rg --type python -A 5 $'processWebRequests'
# Test: Ensure `handleWebRequests` is removed. Expect: No occurances of `handleWebRequests`.
rg --type python -A 5 $'handleWebRequests'
Length of output: 146 Script: #!/bin/bash
# Description: Verify the usage of `processWebRequests` and ensure `handleWebRequests` is removed.
# Test: Search for the usage of `processWebRequests`. Expect: Only occurrences of `processWebRequests`.
rg --type typescript -A 5 $'processWebRequests'
# Test: Ensure `handleWebRequests` is removed. Expect: No occurrences of `handleWebRequests`.
rg --type typescript -A 5 $'handleWebRequests'
Length of output: 162 Script: #!/bin/bash
# Description: Verify the usage of `processWebRequests` and ensure `handleWebRequests` is removed.
# Test: Search for the usage of `processWebRequests`. Expect: Only occurrences of `processWebRequests`.
rg -A 5 $'processWebRequests'
# Test: Ensure `handleWebRequests` is removed. Expect: No occurrences of `handleWebRequests`.
rg -A 5 $'handleWebRequests'
Length of output: 1781 ToolsGitHub Check: codecov/patch
|
||
import { headersToRecords } from "./headersToRecords.js"; | ||
|
||
/** | ||
|
@@ -16,7 +16,7 @@ | |
req: import("node:http").IncomingMessage, | ||
res: import("node:http").ServerResponse | ||
) { | ||
const response = await handleWebRequests({ | ||
const response = await processWebRequests({ | ||
drazisil marked this conversation as resolved.
Show resolved
Hide resolved
|
||
headers: headersToRecords(req.headers), | ||
remoteAddress: req.socket.remoteAddress || "", | ||
method: req.method || "", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,27 @@ | ||
// IMPORTANT: Make sure to import `instrument.js` at the top of your file. | ||
// If you're using ECMAScript Modules (ESM) syntax, use `import "./instrument.js";` | ||
void import ("./instrument.js"); | ||
drazisil marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// All other imports below | ||
import * as Sentry from "@sentry/node"; | ||
drazisil marked this conversation as resolved.
Show resolved
Hide resolved
|
||
import { ServerController } from "./ServerController.js"; | ||
|
||
const serverController = new ServerController(); | ||
void serverController.start(); | ||
process.on("unhandledRejection", (reason, promise) => { | ||
console.error("Unhandled Rejection at:", promise, "reason:", reason); | ||
Sentry.captureException(reason as Error); | ||
process.exit(1); | ||
}) | ||
Comment on lines
+9
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LGTM! Consider adding tests for unhandled promise rejections. The addition of global error handling for unhandled promise rejections enhances the robustness of the application by logging and capturing errors with Sentry. However, consider adding tests to ensure this logic works as expected. Do you want me to generate the unit testing code or open a GitHub issue to track this task? ToolsGitHub Check: codecov/patch
|
||
|
||
process.on("uncaughtException", (error) => { | ||
console.error("Uncaught Exception thrown", error); | ||
Sentry.captureException(error); | ||
process.exit(1); | ||
}) | ||
Comment on lines
+15
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LGTM! Consider adding tests for uncaught exceptions. The addition of global error handling for uncaught exceptions enhances the robustness of the application by logging and capturing errors with Sentry. However, consider adding tests to ensure this logic works as expected. Do you want me to generate the unit testing code or open a GitHub issue to track this task? ToolsGitHub Check: codecov/patch
|
||
|
||
function main() { | ||
const serverController = new ServerController(); | ||
void serverController.start(); | ||
} | ||
Comment on lines
+21
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LGTM! Consider adding tests for the Encapsulating the server startup logic within a However, consider adding tests to ensure this logic works as expected. Do you want me to generate the unit testing code or open a GitHub issue to track this task? ToolsGitHub Check: codecov/patch
|
||
|
||
main(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add test coverage for the invocation of the main function. The invocation of the main function is not covered by tests. Do you want me to generate the unit testing code or open a GitHub issue to track this task? ToolsGitHub Check: codecov/patch
|
||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,18 @@ | ||||||||||||||||||||||||||||||||||||
// Import with `import * as Sentry from "@sentry/node"` if you are using ESM | ||||||||||||||||||||||||||||||||||||
drazisil marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||
import * as Sentry from "@sentry/node"; | ||||||||||||||||||||||||||||||||||||
import { nodeProfilingIntegration } from "@sentry/profiling-node"; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
if (typeof process.env.SENTRY_DSN === "undefined" || process.env.SENTRY_DSN === "") { | ||||||||||||||||||||||||||||||||||||
console.error("No SENTRY_DSN provided"); | ||||||||||||||||||||||||||||||||||||
process.exit(1); | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
Comment on lines
+5
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Improve DSN validation. The current DSN validation only checks for undefined or empty values. Consider using a more robust validation method to ensure the DSN is a valid URL. - if (typeof process.env.SENTRY_DSN === "undefined" || process.env.SENTRY_DSN === "") {
+ const isValidUrl = (url) => {
+ try {
+ new URL(url);
+ return true;
+ } catch (_) {
+ return false;
+ }
+ };
+
+ if (!isValidUrl(process.env.SENTRY_DSN)) { Committable suggestion
Suggested change
ToolsGitHub Check: codecov/patch
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
Sentry.init({ | ||||||||||||||||||||||||||||||||||||
dsn: process.env.SENTRY_DSN, | ||||||||||||||||||||||||||||||||||||
integrations: [nodeProfilingIntegration()], | ||||||||||||||||||||||||||||||||||||
// Performance Monitoring | ||||||||||||||||||||||||||||||||||||
tracesSampleRate: 1.0, // Capture 100% of the transactions | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
// Set sampling rate for profiling - this is relative to tracesSampleRate | ||||||||||||||||||||||||||||||||||||
profilesSampleRate: 1.0, | ||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||
drazisil marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
// @ts-check | ||
|
||
import eslint from "@eslint/js"; | ||
import tseslint from "typescript-eslint"; | ||
|
||
export default tseslint.config( | ||
eslint.configs.recommended, | ||
...tseslint.configs.recommendedTypeChecked, | ||
{ | ||
languageOptions: { | ||
parserOptions: { | ||
project: true, | ||
tsconfigRootDir: import.meta.dirname, | ||
}, | ||
}, | ||
}, | ||
{ | ||
files: ["eslint.config.js", "vitest.config.js", "**/*.d.ts"], | ||
...tseslint.configs.disableTypeChecked, | ||
}, | ||
{ | ||
ignores: ["**/coverage/*", "**/dist/*"], | ||
} | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { getServerURL } from "./src/index.js"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
{ | ||
"name": "@rusty/config", | ||
"version": "1.0.0", | ||
"private": true, | ||
"type": "module", | ||
"imports": { | ||
"#internal": "./dist/src/index.js" | ||
}, | ||
"exports": { | ||
".": "./dist/index.js" | ||
}, | ||
"types": "./dist/index.d.ts", | ||
"scripts": { | ||
"clean": "rm -rf dist", | ||
"build": "tsc --build --verbose", | ||
"test": "vitest", | ||
"check-types": "tsc --noEmit", | ||
"lint": "eslint src" | ||
}, | ||
"keywords": [], | ||
"author": "", | ||
"license": "GPL-3.0-only", | ||
"devDependencies": { | ||
"@types/node": "20.14.12", | ||
"@types/pino": "7.0.5", | ||
"@vitest/coverage-v8": "^2.0.4", | ||
"@vitest/ui": "^2.0.4", | ||
"eslint": "^8.57.0", | ||
"nx": "19.5.1", | ||
"ts-node": "^10.9.2", | ||
"tslib": "^2.6.3", | ||
"typescript": "^5.5.4", | ||
"vite": "2.0.4", | ||
"vitest": "^2.0.4" | ||
}, | ||
"dependencies": { | ||
"@rusty/util": "workspace:*", | ||
"@sentry/node": "^8.20.0", | ||
"@sentry/profiling-node": "^8.20.0", | ||
"pino": "9.3.1" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
export function getServerURL(): string { | ||
const serverURL = process.env.SERVER_URL ?? "https://rusty-motors.com"; | ||
return serverURL; | ||
} | ||
drazisil marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { getServerURL } from "./config.js"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
export type rawHttpRequestData = { | ||
headers: Record<string, string>; | ||
remoteAddress: string; | ||
method: string; | ||
url: string; | ||
}; | ||
|
||
export type parsedHttpRequestData = { | ||
headers: Record<string, string>; | ||
remoteAddress: string; | ||
method: string; | ||
pathname: string; | ||
searchParams: URLSearchParams; | ||
}; | ||
export type RequestResponse = { | ||
statusCode: number; | ||
body: string; | ||
headers: Record<string, string>; | ||
}; | ||
|
||
export type User = { | ||
username: string; | ||
password: string; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import { describe, expect, it } from "vitest"; | ||
import { getServerURL } from "@rusty/config"; | ||
|
||
describe("config", () => { | ||
it("should have a default serverURL if SERVER_URL environment variable is not set", () => { | ||
expect(getServerURL()).toBe("https://rusty-motors.com"); | ||
}); | ||
|
||
it("should use the SERVER_URL environment variable if set", () => { | ||
process.env.SERVER_URL = "https://example.com"; | ||
expect(getServerURL()).toBe("https://example.com"); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
{ | ||
"extends": "../../tsconfig.json", | ||
"compilerOptions": { | ||
"outDir": "./dist", | ||
"declarationDir": "./dist", | ||
"composite": true | ||
}, | ||
"include": ["index.ts", "src/**/*.ts", "test/**/*.ts"], | ||
"exclude": ["node_modules", "dist"], | ||
"references": [{ "path": "../util" }] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
import { defineConfig } from "vitest/config"; | ||
|
||
export default defineConfig({ | ||
test: { | ||
coverage: { | ||
enabled: true, | ||
all: true, | ||
exclude: [ | ||
"dist/**", | ||
"eslint.config.js", | ||
"vitest.config.js" | ||
], | ||
reporter: ["lcov", "text", "cobertura"], | ||
}, | ||
reporters: ["junit", "default", "hanging-process"], | ||
outputFile: "mcos.junit.xml", | ||
pool: "forks", | ||
watch: false, | ||
}, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,4 @@ | ||
export { handleWebRequests } from "./src/index.js"; | ||
/** | ||
* External exports for the web library | ||
*/ | ||
export { processWebRequests } from "./src/processors/processWebRequests.js"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tip
Codebase Verification
Verify the specified paths for components.
The following paths do not exist in the codebase and need to be verified:
apps/*/test
libs/*/test
Please ensure these paths are correct or update the configuration accordingly.
Analysis chain
Verify the specified paths for components.
Ensure that the specified paths for the components exist in the codebase.
Let's re-verify the existence of the specified paths using the correct
fd
command with the--full-path
option.Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 395
Script:
Length of output: 374