feat(server): New Instruments API #2068
Conversation
|
Warning Rate limit exceeded@EmrysMyrddin has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 26 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThe pull request updates multiple configuration and code files. The Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant SA as Server Adapter
participant Instru as Instruments
C->>SA: Send Request
SA->>SA: Check for instrumentation plugins
alt Instruments available
SA->>Instru: Wrap handleRequest using getInstrumented
Instru-->>SA: Return instrumented handler
else No instruments
SA->>SA: Execute default handleRequest
end
SA->>C: Return Response
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅
|
✅
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/server/src/createServerAdapter.ts (3)
104-104: Optional doc comment fortracer
Consider adding a brief explanation of thetracerproperty to clarify its optional usage and purpose.
148-155: Improve error details when multiple tracers are found
This block properly enforces the single-tracer constraint, but it might be helpful to include the plugin's identifier or name in the error message to aid in debugging.- throw new Error( - 'Multiple tracers have been declared. Only one plugin with a tracer is allowed.', - ); + throw new Error( + `Multiple tracers have been declared. Only one plugin with a tracer is allowed. Plugin: ${plugin.name || 'unknown'}` + );
252-262: Handle potential undefinedresponse
Iftracer.requestnever invokes the wrapped function,responsemay remain undefined. Consider providing a fallback response or throwing an error, rather than relying on the non-null assertion.- return isPromise(tracerPromise) ? tracerPromise.then(() => response) : response!; + return isPromise(tracerPromise) + ? tracerPromise.then(() => response ?? new Response('No response returned by tracer', { status: 500 })) + : (response ?? new Response('No response returned by tracer', { status: 500 }));packages/server/src/plugins/types.ts (2)
1-1: Remove unused import or adjust usage
The importedMaybePromiseis currently unused. If you plan to unify your promise signatures withMaybePromise, consider applying it to your tracer definition. Otherwise, remove the import.
53-58: Unify signature withMaybePromise
You already importMaybePromise; consider using it here for consistency, for instance:
request: (payload: { request: Request }, wrapped: () => MaybePromise<void>) => MaybePromise<void>;-export type Tracer = { - request: ( - payload: { request: Request }, - wrapped: () => Promise<void> | void, - ) => Promise<void> | void; -}; +export type Tracer = { + request: ( + payload: { request: Request }, + wrapped: () => MaybePromise<void>, + ) => MaybePromise<void>; +};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignore(1 hunks)packages/server/src/createServerAdapter.ts(5 hunks)packages/server/src/plugins/types.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🧰 Additional context used
🪛 Biome (1.9.4)
packages/server/src/plugins/types.ts
[error] 8-8: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: unit / node 23
- GitHub Check: unit / node 22
- GitHub Check: unit / deno
- GitHub Check: unit / node 20
- GitHub Check: unit / bun
- GitHub Check: esm
- GitHub Check: server (undici)
- GitHub Check: e2e / cloudflare-modules
- GitHub Check: server (ponyfill)
- GitHub Check: e2e / cloudflare-workers
- GitHub Check: unit / node 18
- GitHub Check: lint
- GitHub Check: e2e / azure-function
- GitHub Check: node-fetch (consumeBody)
- GitHub Check: prettier
- GitHub Check: server (native)
- GitHub Check: e2e / aws-lambda
- GitHub Check: node-fetch (noConsumeBody)
- GitHub Check: type check
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
packages/server/src/createServerAdapter.ts (2)
3-3: Looking good for the new import ofTracer
No issues spotted with the added import statement.
176-176: Allowing re-assignment of handleRequest
Switching fromconsttoletis correct to accommodate wrapping the request handler with tracer logic.packages/server/src/plugins/types.ts (1)
9-13: Typo in documentation and consider making property optional
- "insance" should be "instance".
- The usage in
createServerAdapterchecks ifplugin.tracerexists, so consider markingtraceras optional (tracer?: Tracer;).
[nitpick, refactor_suggestion_good_to_have]- * A tracer insance. It can be used to wrap... + * A tracer instance. It can be used to wrap...
✅
|
✅
|
✅
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/server/src/plugins/types.ts (2)
8-12: Fix typo in JSDoc comment.There's a typo in the JSDoc comment: "insance" should be "instance".
- * A tracer insance. It can be used to wrap the entire request handling pipeline (including the + * A tracer instance. It can be used to wrap the entire request handling pipeline (including the
52-57: Add JSDoc documentation and consider error handling.The
Tracertype would benefit from:
- JSDoc documentation explaining its purpose and usage
- Explicit error handling guidance for the wrapped function
Add JSDoc documentation and consider error handling:
export type Tracer = { + /** + * Wraps a request handling operation for tracing purposes. + * @param payload - The payload containing the request to be traced + * @param wrapped - The function to be traced + * @throws - Document expected errors and how they should be handled + * @returns A promise that resolves when the traced operation completes + */ request: ( payload: { request: Request }, wrapped: () => Promise<void> | void, ) => Promise<void> | void; };Consider adding error handling guidance by documenting:
- What happens if the wrapped function throws?
- Should the tracer catch and log errors?
- Should errors be propagated or transformed?
Would you like me to propose a more detailed error handling implementation?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/server/src/plugins/types.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: unit / node 22
- GitHub Check: unit / node 18
🚀 Snapshot Release (
|
| Package | Version | Info |
|---|---|---|
@whatwg-node/promise-helpers |
1.2.2-alpha-20250304145637-5149d6730561e4ebb207798aac0719737cbb7cc6 |
npm ↗︎ unpkg ↗︎ |
@whatwg-node/server |
0.10.0-alpha-20250304145637-5149d6730561e4ebb207798aac0719737cbb7cc6 |
npm ↗︎ unpkg ↗︎ |
@whatwg-node/server-plugin-cookies |
1.0.5-alpha-20250304145637-5149d6730561e4ebb207798aac0719737cbb7cc6 |
npm ↗︎ unpkg ↗︎ |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/purple-buses-try.md (1)
5-6: Expand on the Tracer API Description for Better Developer Guidance
The short description "Add new Tracer API" is clear but could benefit from additional details. Consider adding more context about the Tracer API's role (e.g., its purpose for wrapping request phases or performance monitoring) or linking to further documentation. This would help developers understand the impact and usage of the new feature.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/purple-buses-try.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: unit / node 23
- GitHub Check: unit / node 22
- GitHub Check: unit / node 20
- GitHub Check: unit / node 18
- GitHub Check: type check
- GitHub Check: server (undici)
- GitHub Check: node-fetch (consumeBody)
- GitHub Check: unit / deno
- GitHub Check: server (ponyfill)
- GitHub Check: unit / bun
- GitHub Check: e2e / azure-function
- GitHub Check: node-fetch (noConsumeBody)
- GitHub Check: server (native)
🔇 Additional comments (1)
.changeset/purple-buses-try.md (1)
1-3: Ensure Consistent Version Bumping for the New Tracer API
The changeset correctly indicates a minor version update for the package@whatwg-node/server, which aligns with adding a new feature. Ensure that this update is consistent with the release strategy and that related documentation (e.g., changelogs and release notes) is updated accordingly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/server/src/createServerAdapter.ts (1)
176-176: Consider simplifying the tracer implementation.The tracing implementation is solid but could be more concise.
Consider this simplified version:
-if (tracer?.request) { +if (tracer) { const originalRequestHandler = handleRequest; handleRequest = (request, initialContext) => { let response: Promise<Response> | Response; - const tracerPromise = tracer!.request!({ request }, () => { + const tracerPromise = tracer.request?.({ request }, () => { response = originalRequestHandler(request, initialContext); return isPromise(response) ? response.then(() => undefined) : undefined; }); - return isPromise(tracerPromise) ? tracerPromise.then(() => response) : response!; + return tracerPromise ? tracerPromise.then(() => response) : response; }; }This version:
- Removes unnecessary non-null assertions
- Uses optional chaining more effectively
- Simplifies the return statement
Also applies to: 252-262
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/server/src/createServerAdapter.ts(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: unit / node 23
- GitHub Check: unit / deno
- GitHub Check: unit / node 22
- GitHub Check: unit / node 20
- GitHub Check: unit / bun
- GitHub Check: server (undici)
- GitHub Check: unit / node 18
- GitHub Check: server (ponyfill)
- GitHub Check: node-fetch (consumeBody)
- GitHub Check: server (native)
- GitHub Check: e2e / azure-function
- GitHub Check: node-fetch (noConsumeBody)
🔇 Additional comments (2)
packages/server/src/createServerAdapter.ts (2)
3-3: LGTM!Clean import of the
Tracertype, maintaining consistency with existing imports.
104-104: LGTM! Robust tracer validation.The implementation ensures that only one tracer can be declared, with proper error handling and clear error messages.
Also applies to: 148-155
|
yes! |
|
Yes, I wanted to wait for the approbation of the design of the new API before passing time on tests and documentation :-) |
|
I think design is confirmed, no need to wait for additional approvals, we can proceed with testing 🙂 |
EmrysMyrddin
force-pushed
the
feat/tracer
branch
from
de7e6d1 to
9627f0b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/server/src/createServerAdapter.ts (1)
149-156: Consider improving error message for multiple tracersThrowing an error upon finding more than one tracer is a good safeguard. Including which plugin declared the second tracer could help diagnose conflicts more quickly.
- throw new Error( - 'Multiple tracers have been declared. Only one plugin with a tracer is allowed.', - ); + throw new Error( + `Multiple tracers have been declared. Plugin "${plugin.name ?? 'unknown'}" tried to register a second tracer.`, + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/purple-buses-try.md(1 hunks).gitignore(1 hunks)packages/server/src/createServerAdapter.ts(5 hunks)packages/server/src/plugins/types.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- .changeset/purple-buses-try.md
- .gitignore
- packages/server/src/plugins/types.ts
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: unit / node 23
- GitHub Check: unit / node 22
- GitHub Check: unit / node 20
- GitHub Check: unit / deno
- GitHub Check: unit / node 18
- GitHub Check: unit / bun
- GitHub Check: e2e / cloudflare-modules
- GitHub Check: type check
- GitHub Check: e2e / cloudflare-workers
- GitHub Check: server (ponyfill)
- GitHub Check: e2e / azure-function
- GitHub Check: prettier
- GitHub Check: e2e / aws-lambda
🔇 Additional comments (4)
packages/server/src/createServerAdapter.ts (4)
4-4: Import of Tracer is valid and consistentNo issues. The import of
Tracerfrom./plugins/types.jsaligns well with the new functionality.
105-106: Optional tracer variable looks goodDefining
let tracer: Tracer | undefined;is a clean approach to conditionally store the tracer.
177-178: Reassigning handleRequest is appropriateChanging to a
letdeclaration allows reassigning with tracer logic. This is a valid approach.
250-260:❓ Verification inconclusive
Verify that all tracer implementations call the callback
The tracer logic here depends on
tracer.requestinvoking the callback to assignresponse. If any plugin’stracer.requestis missing its callback invocation,responseremains uninitialized, causing potential runtime errors.Below is a script to search for all
tracer.requestusages and confirm callback invocation:
🏁 Script executed:
#!/bin/bash # Description: Find and inspect all calls to tracer.request in the codebase. rg -A 5 "tracer\.request"Length of output: 27
Manual Verification Required: Ensure Callback Invocation in All
tracer.requestUsagesThe automated search for
"tracer\.request"did not yield any additional instances beyond the snippet in question. Given this outcome, please manually verify that every implementation oftracer.request—including those in external plugins—properly invokes the callback. This is crucial because if a plugin omits the callback call, theresponsemay remain uninitialized, potentially leading to runtime issues.
- Action Required: Manually inspect all plugin implementations of
tracer.requestto confirm they call the provided callback, ensuring the assignment toresponseis always executed.- File for Reference:
packages/server/src/createServerAdapter.ts(see lines 250-260)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/@whatwg-node_server-2068-dependencies.md (1)
4-6: Resolve Markdown List Indentation WarningStatic analysis indicates that the unordered list on line 5 is indented by 2 spaces, but the standard (MD007) expects 0 spaces. Consider removing the extra indentation to satisfy markdownlint:
- - Added dependency [`@envelop/instruments@1.0.0-alpha-20250227114734-64d7083775b6c1ee4ce365437ae5f77c33aee3a5` ↗︎](https://www.npmjs.com/package/@envelop/instruments/v/1.0.0) (to `dependencies`) + - Added dependency [`@envelop/instruments@1.0.0-alpha-20250227114734-64d7083775b6c1ee4ce365437ae5f77c33aee3a5` ↗︎](https://www.npmjs.com/package/@envelop/instruments/v/1.0.0) (to `dependencies`)This adjustment will align with the project's markdown style guidelines.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
5-5: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
.changeset/@whatwg-node_server-2068-dependencies.md(1 hunks)packages/server/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/server/package.json
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
.changeset/@whatwg-node_server-2068-dependencies.md
5-5: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
🔇 Additional comments (1)
.changeset/@whatwg-node_server-2068-dependencies.md (1)
1-3: Verify Changeset Header StructureThe changeset header is properly formatted using the conventional YAML-like separator lines and specifies a patch update for
@whatwg-node/server. This structure looks consistent with typical changeset conventions.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/@whatwg-node_server-2068-dependencies.md (1)
5-5: Adjust Unordered List Indentation to Satisfy Markdown LintingThe unordered list item on line 5 is indented by two spaces, which is triggering the MD007 warning regarding unordered list indentation. Consider unindenting the list item to align at the start of the line (0 spaces) for compliance with markdownlint guidelines.
- - Added dependency [`@envelop/instruments@1.0.0-alpha-20250227122402-42e2fa806ec3b7c2936e612924b153a20c8662c3` ↗︎](https://www.npmjs.com/package/@envelop/instruments/v/1.0.0) (to `dependencies`) + - Added dependency [`@envelop/instruments@1.0.0-alpha-20250227122402-42e2fa806ec3b7c2936e612924b153a20c8662c3` ↗︎](https://www.npmjs.com/package/@envelop/instruments/v/1.0.0) (to `dependencies`)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
5-5: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
.changeset/@whatwg-node_server-2068-dependencies.md(1 hunks)packages/server/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/server/package.json
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
.changeset/@whatwg-node_server-2068-dependencies.md
5-5: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
🔇 Additional comments (1)
.changeset/@whatwg-node_server-2068-dependencies.md (1)
1-6: Dependency Update Documentation is Clear and InformativeThe changeset entry correctly documents the addition of the new dependency
@envelop/instruments@1.0.0-alpha-20250227122402-42e2fa806ec3b7c2936e612924b153a20c8662c3. The format, including the version and hyperlink to npm, provides clear context for downstream consumers of the changeset.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
5-5: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.changeset/@whatwg-node_server-2068-dependencies.md (1)
5-6: Fix markdown list indentation.The markdown linter has flagged an inconsistency in the list indentation. According to markdown standards, unordered lists should start at indentation level 0, but they're currently at level 2.
- - Updated dependency [`@whatwg-node/promise-helpers@^1.2.2` ↗︎](https://www.npmjs.com/package/@whatwg-node/promise-helpers/v/1.2.2) (from `^1.0.0`, in `dependencies`) - - Added dependency [`@envelop/instruments@1.0.0-alpha-20250227122402-42e2fa806ec3b7c2936e612924b153a20c8662c3` ↗︎](https://www.npmjs.com/package/@envelop/instruments/v/1.0.0) (to `dependencies`) +- Updated dependency [`@whatwg-node/promise-helpers@^1.2.2` ↗︎](https://www.npmjs.com/package/@whatwg-node/promise-helpers/v/1.2.2) (from `^1.0.0`, in `dependencies`) +- Added dependency [`@envelop/instruments@1.0.0-alpha-20250227122402-42e2fa806ec3b7c2936e612924b153a20c8662c3` ↗︎](https://www.npmjs.com/package/@envelop/instruments/v/1.0.0) (to `dependencies`)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
5-5: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
6-6: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
packages/server/test/instruments.spec.ts (2)
30-31: Consider adding more request scenarios.The test currently makes a single request and verifies the execution order. To increase confidence in the implementation, consider adding more complex scenarios such as:
- Testing with error conditions
- Testing with different request methods or parameters
- Testing the actual response data
await adapter.fetch('http://whatwg-node/graphql'); expect(results).toEqual(['pre-1', 'pre-2', 'pre-3', 'request', 'post-3', 'post-2', 'post-1']); + + // Reset results for the next test + results.length = 0; + + // Test with error scenario + const errorAdapter = createServerAdapter<unknown>( + () => { + results.push('error-request'); + throw new Error('Test error'); + }, + { + plugins: [make('1'), make('2'), make('3')], + }, + ); + + try { + await errorAdapter.fetch('http://whatwg-node/graphql'); + } catch (error) { + // Error expected + } + // Verify instruments still execute in correct order even with errors + expect(results).toEqual(['pre-1', 'pre-2', 'pre-3', 'error-request', 'post-3', 'post-2', 'post-1']);
8-18: Consider adding type for the wrapped function parameter.The
wrappedparameter in therequestinstrument function doesn't have an explicit type, which might make it harder to understand what it's supposed to do. Consider adding a type annotation for clarity.- request: async (_, wrapped) => { + request: async (_, wrapped: () => Promise<void>) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/@whatwg-node_server-2068-dependencies.md(1 hunks)packages/server/test/instruments.spec.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/server/test/instruments.spec.ts
[error] 20-20: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
🪛 markdownlint-cli2 (0.17.2)
.changeset/@whatwg-node_server-2068-dependencies.md
5-5: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
6-6: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
🔇 Additional comments (3)
.changeset/@whatwg-node_server-2068-dependencies.md (1)
1-6:✅ Verification successful
Dependency updates look appropriate for the Tracer API.
The dependency updates align well with the PR's objective of introducing a new Tracer API, which requires the alpha version of
@envelop/instruments. The patch version update is appropriate since this is a non-breaking feature addition.
🏁 Script executed:
#!/bin/bash # Verify if the @envelop/instruments package is available and check its details # Check if the package exists on npm echo "Checking @envelop/instruments package details:" npm view @envelop/instruments version description # Check for @whatwg-node/promise-helpers to ensure it's compatible echo "Checking @whatwg-node/promise-helpers latest version:" npm view @whatwg-node/promise-helpers versionLength of output: 502
Dependency Updates Verified for the Tracer API Integration
- Confirmed that
@whatwg-node/promise-helpersis now at version 1.2.2 as expected.- Verified that the PR intentionally specifies an alpha version of
@envelop/instruments(required for the new Tracer API), even though the latest stable version on npm is 5.0.3.- The patch update for
@whatwg-node/servercorrectly indicates non-breaking changes.These dependency updates align well with the PR’s objective.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
5-5: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
6-6: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
packages/server/test/instruments.spec.ts (2)
1-33: Test coverage for the new Tracer API looks good.The test file effectively validates the core functionality of the new Tracer API by checking that:
- Multiple plugins can provide instrumentation
- The request handlers are wrapped correctly
- The order of execution follows the expected pattern (first-in-last-out)
This ensures the instrumentation implementation is working as expected and composable.
🧰 Tools
🪛 Biome (1.9.4)
[error] 20-20: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
4-33:✅ Verification successful
Verify integration with other server adapter features.
This test ensures that the instrumentation works in isolation, but it would be valuable to verify that it integrates well with other server adapter features like middleware, error handling, etc.
🏁 Script executed:
#!/bin/bash # Description: Check for integration with other server adapter features # Look for other server adapter tests that might need to be updated echo "Checking for existing server adapter tests:" fd --type file "spec.ts|test.ts" packages/server/test/ # Check the server adapter implementation to understand integration points echo "Server adapter implementation:" cat packages/server/src/createServerAdapter.ts | grep -A 10 "instruments" # Check if there are any related middleware or error handling features echo "Looking for middleware implementations:" fd --type file "middleware" packages/server/src/Length of output: 3042
Integration Verification Complete – Instrumentation is Properly Chained with Core Adapter Features
The instrumentation is correctly integrated with the adapter’s plugin hooks (e.g., onRequest and onResponse) as reflected in the implementation in
packages/server/src/createServerAdapter.ts, and related integration (like error handling) is indirectly verified by tests such asuseErrorHandling.spec.ts. Note that no middleware implementations exist at this time, so middleware integration isn’t applicable until such features are introduced.🧰 Tools
🪛 Biome (1.9.4)
[error] 20-20: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
| }; | ||
| } | ||
|
|
||
| const adapter = createServerAdapter<{}>( |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace empty object type with a more specific type.
Using {} as a type is discouraged as it means "any non-nullable value" rather than "empty object" as often expected. Consider using a more specific type or unknown if the generic type parameter isn't important for this test.
-const adapter = createServerAdapter<{}>(
+const adapter = createServerAdapter<unknown>(Alternative solutions depending on the actual requirements:
// If you need to specify an empty object type:
const adapter = createServerAdapter<Record<string, never>>(
// Or if there's a specific context type that should be used:
const adapter = createServerAdapter<YourContextType>(🧰 Tools
🪛 Biome (1.9.4)
[error] 20-20: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.changeset/@whatwg-node_server-2068-dependencies.md (1)
5-6: Fix unordered list indentationThe markdown linter is flagging that the list items are indented with 2 spaces instead of being at column 0.
- dependencies updates: - - Updated dependency [`@whatwg-node/promise-helpers@^1.2.2` ↗︎](https://www.npmjs.com/package/@whatwg-node/promise-helpers/v/1.2.2) (from `^1.0.0`, in `dependencies`) - - Added dependency [`@envelop/instruments@1.0.0-alpha-20250227122402-42e2fa806ec3b7c2936e612924b153a20c8662c3` ↗︎](https://www.npmjs.com/package/@envelop/instruments/v/1.0.0) (to `dependencies`) + dependencies updates: + - Updated dependency [`@whatwg-node/promise-helpers@^1.2.2` ↗︎](https://www.npmjs.com/package/@whatwg-node/promise-helpers/v/1.2.2) (from `^1.0.0`, in `dependencies`) + - Added dependency [`@envelop/instruments@1.0.0-alpha-20250227122402-42e2fa806ec3b7c2936e612924b153a20c8662c3` ↗︎](https://www.npmjs.com/package/@envelop/instruments/v/1.0.0) (to `dependencies`)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
5-5: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
6-6: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
packages/server/test/instruments.spec.ts (1)
4-33: Consider additional test cases for comprehensive coverageThe current test covers the basic happy path for instruments. Consider adding tests for:
- Error handling - what happens if an instrument throws an error?
- Empty plugins array - does it work correctly without any instruments?
- Async operations in the request handler - ensure proper awaiting
- Return value manipulation - can instruments modify the response?
it('should handle errors in instruments gracefully', async () => { const results: string[] = []; function makeWithError(): ServerAdapterPlugin { return { instruments: { request: async (_, wrapped) => { results.push('pre-error'); try { await wrapped(); } catch (error) { results.push('caught-error'); throw error; // re-throw to propagate } }, }, }; } const errorPlugin: ServerAdapterPlugin = { instruments: { request: async () => { throw new Error('Test error'); }, }, }; const adapter = createServerAdapter<Record<string, never>>( () => { results.push('request'); return Response.json({ message: 'Hello, World!' }); }, { plugins: [makeWithError(), errorPlugin], }, ); await expect(adapter.fetch('http://whatwg-node/graphql')).rejects.toThrow(); expect(results).toEqual(['pre-error', 'caught-error']); });🧰 Tools
🪛 Biome (1.9.4)
[error] 20-20: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/@whatwg-node_server-2068-dependencies.md(1 hunks)packages/server/test/instruments.spec.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/server/test/instruments.spec.ts
[error] 20-20: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
🪛 markdownlint-cli2 (0.17.2)
.changeset/@whatwg-node_server-2068-dependencies.md
5-5: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
6-6: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
🔇 Additional comments (3)
.changeset/@whatwg-node_server-2068-dependencies.md (1)
1-6:❓ Verification inconclusive
Marked as patch with alpha dependency - consider versioning strategy
The changeset correctly documents the dependency updates, but I'd like to highlight that you're adding an alpha version of
@envelop/instrumentswhile marking this as a patch change. Alpha dependencies can introduce stability risks in a patch release.For future consideration:
- Consider whether this should be a minor rather than patch version
- Document when you plan to update to a stable release of
@envelop/instruments- Ensure there's a migration plan if the API changes between alpha and stable
🌐 Web query:
What are the best practices for semantic versioning when adding alpha dependencies?💡 Result:
Semantic Versioning (SemVer) provides clear guidelines for managing software dependencies, including alpha or pre-release versions. When incorporating alpha dependencies, these best practices ensure stability and clarity:
Pre-release Version Formatting
- Use the
MAJOR.MINOR.PATCH-alpha.Nformat (e.g.,2.1.0-alpha.1) to denote alpha versions[1][2].- Append labels like
-alpha,-beta, or-rc(release candidate) to distinguish pre-release builds[1][8].Dependency Specification
- Exact versions: Pin alpha dependencies to specific versions (e.g.,
1.0.0-alpha.3) to avoid unintended updates[5][9].- Avoid automatic ranges: Tools like npm default to ignoring pre-releases unless explicitly included. Use
1.0.0-alphaor^1.0.0-alphato allow compatible pre-releases[5].Stability and Compatibility
- Treat alpha dependencies as unstable: They may introduce breaking changes even in minor or patch updates[1][7].
- Isolate alpha usage: Limit alpha dependencies to non-production environments (e.g., testing, development)[9][10].
Versioning Your Own Releases
- Minor version increments: If adding an alpha dependency introduces backward-compatible features, increment the minor version (e.g.,
1.2.0→1.3.0)[3][9].- Major version increments: Required if the alpha dependency’s changes force breaking API modifications in your project[1][8].
Documentation and Communication
- Changelogs: Clearly document alpha dependencies and their potential impact[4][10].
- Warn users: Highlight alpha usage in release notes or README files to set expectations[6][10].
Automation and Tooling
- CI/CD pipelines: Use tools like
semantic-releaseto automate version bumps when integrating alpha dependencies[4].- Lockfiles: Employ package managers’ lockfiles (e.g.,
package-lock.json) to freeze dependency versions[5].By adhering to these practices, teams can safely experiment with alpha dependencies while maintaining compatibility and transparency. Always prioritize stability in production environments and reserve alpha usage for early testing phases[7][9].
Citations:
- 1: https://semver.org
- 2: https://semver.org/spec/v2.0.0-rc.1.html
- 3: https://www.sitepoint.com/semantic-versioning-why-you-should-using/
- 4: https://aws.amazon.com/blogs/devops/using-semantic-versioning-to-simplify-release-management/
- 5: https://docs.npmjs.com/about-semantic-versioning/
- 6: https://launchdarkly.com/blog/software-release-versioning/
- 7: https://clojureverse.org/t/stop-using-semantic-versioning-any-writings-on-this/9951
- 8: https://www.tiny.cloud/blog/improving-our-engineering-best-practices-with-semantic-versioning/
- 9: https://arjancodes.com/blog/how-to-implement-semantic-versioning-in-software-projects/
- 10: https://cpl.thalesgroup.com/software-monetization/software-versioning-basics
Attention: Revisit Versioning Strategy for Alpha Dependency
The changeset accurately captures the dependency updates for
@whatwg-node/server, but including the alpha release of@envelop/instrumentsin a patch bump raises stability concerns. Best practices for semantic versioning with pre-release (alpha) dependencies suggest that such changes may warrant a minor version bump rather than a patch. This approach better reflects the potential for instability and future API changes.Recommendations:
- Version Bump: Confirm if a minor release would be more appropriate given the added alpha dependency.
- Documentation: Clearly document when you plan to transition to a stable release of
@envelop/instruments.- Migration Plan: Ensure a robust migration plan is in place for any breaking changes that might occur as the alpha evolves.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
5-5: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
6-6: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
packages/server/test/instruments.spec.ts (2)
8-18: Well-structured plugin factory functionThe
makefunction creates a plugin with instruments that correctly wraps the request handling logic. The pre/post execution pattern is clean and easily demonstrates the execution flow.
30-31: Well-designed test for verifying execution orderThe test effectively validates that the instrument wrappers are composed correctly - executing in order from outer to inner before the request, and then from inner to outer after the request. This matches the expected behavior of properly composed middleware.
| }; | ||
| } | ||
|
|
||
| const adapter = createServerAdapter<{}>( |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace empty object type with a more specific type
Using {} as a type is discouraged as it means "any non-nullable value" rather than "empty object". It's better to explicitly define the object shape or use a more appropriate type.
- const adapter = createServerAdapter<{}>(
+ const adapter = createServerAdapter<Record<string, never>>(Alternatively, if there's a specific context type that should be used here, consider using that instead.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const adapter = createServerAdapter<{}>( | |
| const adapter = createServerAdapter<Record<string, never>>( |
🧰 Tools
🪛 Biome (1.9.4)
[error] 20-20: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/promise-helpers/src/index.ts (2)
133-144: Consider adding documentation to clarify behavior with null/undefined values.The function properly handles undefined/null values, but it would be helpful to add JSDoc comments explaining this behavior, especially since you've now explicitly updated the type signature to allow these values.
export function iterateAsync<TInput, TOutput>( iterable: Iterable<TInput>, callback: ( input: TInput, endEarly: VoidFunction, index: number, ) => MaybePromise<TOutput | undefined | null>, results?: TOutput[], ): MaybePromise<void> { + // If the callback returns undefined or null, the value is not added to the results array
108-147: Consider aligningiterateAsyncVoidwith the updated type signature.Since
iterateAsyncVoidis an alias foriterateAsync, it inherits the updated type signature. It might be clearer to explicitly re-export with the new type or add a comment clarifying this relationship.-export { iterateAsync as iterateAsyncVoid }; +// Re-export iterateAsync with the same signature allowing for undefined/null return values +export { iterateAsync as iterateAsyncVoid };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/promise-helpers/src/index.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: alpha / snapshot
- GitHub Check: e2e / cloudflare-modules
- GitHub Check: e2e / cloudflare-workers
- GitHub Check: server (undici)
- GitHub Check: e2e / azure-function
- GitHub Check: node-fetch (consumeBody)
- GitHub Check: e2e / aws-lambda
- GitHub Check: server (ponyfill)
- GitHub Check: node-fetch (noConsumeBody)
- GitHub Check: server (native)
🔇 Additional comments (2)
packages/promise-helpers/src/index.ts (2)
110-117: Explicit type signature improvement for callback return values.The updated type signature for the
callbackparameter initerateAsyncnow accurately reflects that the function can handleundefinedornullreturn values. This is consistent with the existing implementation (line 136-138) which already checksif (result)before pushing to the results array.This change improves type safety by making the API contract more explicit and is likely related to the new Tracer API implementation, where instrumentation callbacks might not always need to return a value.
136-138:❓ Verification inconclusive
Verify impact on existing code with the type change.
The function already handles falsy return values correctly, but with the explicit type change, it's important to verify that all existing usages of this function are compatible with the updated behavior.
🏁 Script executed:
#!/bin/bash # Description: Find all usages of iterateAsync to ensure type compatibility with the updated function signature echo "Searching for iterateAsync usage in the codebase..." rg "iterateAsync" --type ts --no-filename | grep -v "export function iterateAsync"Length of output: 470
Attention: Confirm Compatibility with Explicit Type Update
The explicit type change in the function now ensuring falsy return values are properly handled has been applied. The search results show that the function is primarily used via its alias (
iterateAsyncVoid), for example in hook invocations, and exported as such. Please verify that these instances ofiterateAsyncVoidcontinue to work as expected with the updated type signatures.
- Confirm that calls like
iterateAsyncVoid(onResponseHooks, onResponseHook => …)anditerateAsyncVoid(onRequestHooks, (onRequestHook, stopEarly) => …)are fully compatible with the new explicit type.- Ensure that the alias export (
export { iterateAsync as iterateAsyncVoid };) correctly propagates the type changes throughout the codebase.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.changeset/@whatwg-node_server-2068-dependencies.md (1)
5-6: Fix Markdown List Indentation (MD007)The unordered list items on these lines use two spaces for indentation, but markdownlint expects no indentation (0 spaces) for list markers. Please remove the extra spaces to comply with MD007.
- - Updated dependency [`@whatwg-node/promise-helpers@^1.2.2` ↗︎](https://www.npmjs.com/package/@whatwg-node/promise-helpers/v/1.2.2) (from `^1.0.0`, in `dependencies`) - - Added dependency [`@envelop/[email protected]` ↗︎](https://www.npmjs.com/package/@envelop/instruments/v/1.0.0) (to `dependencies`) + - Updated dependency [`@whatwg-node/promise-helpers@^1.2.2` ↗︎](https://www.npmjs.com/package/@whatwg-node/promise-helpers/v/1.2.2) (from `^1.0.0`, in `dependencies`) + - Added dependency [`@envelop/[email protected]` ↗︎](https://www.npmjs.com/package/@envelop/instruments/v/1.0.0) (to `dependencies`)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
5-5: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
6-6: Unordered list indentation
Expected: 0; Actual: 2(MD007, ul-indent)
packages/promise-helpers/src/index.ts (3)
110-117: Type signature enhancement aligns with implementation, but consider refining void usageThe expanded return type
MaybePromise<TOutput | undefined | null | void>better aligns with the implementation at line 136-138 which only adds truthy results to the results array. This change provides more flexibility for callback implementations.Consider addressing the static analysis warning by removing
voidfrom the union type:callback: ( input: TInput, endEarly: VoidFunction, index: number, - ) => MaybePromise<TOutput | undefined | null | void>, + ) => MaybePromise<TOutput | undefined | null>,
voidin union types can be confusing as it represents the absence of a return value rather than an actual value. Since the implementation already handles all falsy values appropriately, using justundefinedandnullwould be more semantically correct.🧰 Tools
🪛 Biome (1.9.4)
[error] 116-116: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
136-138: Make the result check more explicit to match the expanded return typesThe current check uses a simple truthiness test, which handles
undefined,null, andvoidreturns, but also excludes falsy values like0,'', andfalse, which may be valid outputs of typeTOutput.Consider making the check more explicit to specifically exclude only
undefined,null, andvoid:- if (result) { + if (result !== undefined && result !== null) { results?.push(result); }This change would ensure that all non-null, non-undefined values get added to the results, including falsy values like
0or''that might be legitimate TOutput values.
110-117: Add documentation for the expanded return typeConsider adding a JSDoc comment to document the significance of the expanded return type:
+/** + * Iterates over an iterable, calling the callback function for each element. + * + * @param iterable - The iterable to iterate over + * @param callback - A function called for each element, can return a value to be collected + * in the results array, or undefined/null to skip collection + * @param results - Optional array to collect non-null/undefined results + * @returns A promise that resolves when iteration is complete + */ export function iterateAsync<TInput, TOutput>( iterable: Iterable<TInput>, callback: ( input: TInput, endEarly: VoidFunction, index: number, ) => MaybePromise<TOutput | undefined | null | void>, results?: TOutput[], ): MaybePromise<void> {This documentation would help future developers understand the intent and expected behavior of the callback's return types.
🧰 Tools
🪛 Biome (1.9.4)
[error] 116-116: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
.changeset/@whatwg-node_server-2068-dependencies.md(1 hunks)packages/promise-helpers/src/index.ts(1 hunks)packages/server/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/server/package.json
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
.changeset/@whatwg-node_server-2068-dependencies.md
5-5: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
6-6: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
🪛 Biome (1.9.4)
packages/promise-helpers/src/index.ts
[error] 116-116: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (1)
.changeset/@whatwg-node_server-2068-dependencies.md (1)
1-3: New Dependency Update File Structure Looks GoodThis new changeset file correctly documents the dependency updates for the server package. The header and dependency update details are clearly defined.
EmrysMyrddin
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
.changeset/purple-buses-try.md (7)
5-8: Fixing Minor Typos in API Introduction
There is a typo in "Introducation" which should be "Introduction". Please review this section to ensure all descriptive text is free of misspellings.
9-12: Review Documentation Clarity and Spelling
The explanation contains several typos:
- "intup/output" should be "input/output"
- "araound" should be "around"
- "incuding" should be "including"
Consider revising these lines for clarity and professional presentation.
17-42: Improve Usage Example Formatting
The basic usage example effectively demonstrates instrumentation with Sentry. For consistency and clarity, consider updating the span name from "Graphql Operation" to "GraphQL Operation".
44-48: Correct Typos in Multiple Instruments Description
There are a few typos in this section:
- "possilbe" should be "possible"
- "outtermost" should be "outermost"
Please update these to ensure the documentation reads professionally.🧰 Tools
🪛 LanguageTool
[grammar] ~47-~47: Comparison is written ‘the same … as’.
Context: ...y composed by envelop in the same order than the plugin array (first is outtermost, ...(THE_SAME_AS)
[misspelling] ~48-~48: This word is normally spelled as one.
Context: ...gin array (first is outtermost, last is inner most). ```ts import { createServerAdapter }...(EN_COMPOUNDS_INNER_MOST)
50-59: Clarify Import Dependencies in Example Code
The snippet usesuseSentry()anduseOpentelemetry()functions without showing their import statements. It would improve clarity if you either include the relevant imports or note that these helpers are assumed to be in scope.
69-73: Fix Typos in Custom Instruments Ordering Description
There are minor issues in the description:
- "suite" should be "suit"
- "mannually" should be "manually"
A quick revision will enhance the overall professionalism of the documentation.
74-87: Review Custom Instruments Ordering Code Sample
This code sample effectively demonstrates composing instruments manually usingcomposeInstrumentsalongsidecreateServerAdapter. The destructuring ofsentryInstrumentsandotelInstrumentsis clear. Just ensure that, in actual usage, the import statements foruseSentryanduseOpentelemetryare provided.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/purple-buses-try.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.changeset/purple-buses-try.md
[grammar] ~47-~47: Comparison is written ‘the same … as’.
Context: ...y composed by envelop in the same order than the plugin array (first is outtermost, ...
(THE_SAME_AS)
[misspelling] ~48-~48: This word is normally spelled as one.
Context: ...gin array (first is outtermost, last is inner most). ```ts import { createServerAdapter }...
(EN_COMPOUNDS_INNER_MOST)
⏰ Context from checks skipped due to timeout of 90000ms (17)
- GitHub Check: unit / node 23
- GitHub Check: unit / deno
- GitHub Check: unit / node 22
- GitHub Check: unit / node 20
- GitHub Check: unit / node 18
- GitHub Check: unit / bun
- GitHub Check: type check
- GitHub Check: prettier
- GitHub Check: server (undici)
- GitHub Check: lint
- GitHub Check: node-fetch (consumeBody)
- GitHub Check: alpha / snapshot
- GitHub Check: server (ponyfill)
- GitHub Check: esm
- GitHub Check: node-fetch (noConsumeBody)
- GitHub Check: server (native)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
.changeset/purple-buses-try.md (3)
1-4: YAML Front Matter Format Valid!
The YAML front matter specifying the version bump for@whatwg-node/serverlooks correctly formatted.
61-67: Verify Sequence Diagram Accuracy
The Mermaid sequence diagram appears to correctly illustrate the interaction between Sentry, Opentelemetry, and the Server Adapter. Ensure that the depicted sequence aligns with the intended behavior in the instrumentation chain.
89-95: Confirm Custom Instruments Sequence Diagram
The final Mermaid diagram clearly depicts the interaction sequence for custom instrumentation ordering. The visualization is helpful; please verify that it remains in sync with the evolving API design.
Description
This PR is the continuation of graphql-hive/envelop#2430 which introduces a new core API: The Tracer.
A plugin (and only one) can define a
tracerinstance that will wrap every phases of a request pipeline.In the case of
wahtwg-node, it only adds therequestwrapper.Type of change
Please delete options that are not relevant.
Summary by CodeRabbit
New Features
Chores
.gitignoreto prevent tracking of specific configuration files.iterateAsyncfunction to includeundefinedornull.