You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(node:util): use the shared promisify custom symbol - #570
Use Node.js's globally registered promisify.custom symbol so custom implementations work across unenv, native Node.js, and other libraries.
Previously, unenv ignored these hooks, producing fallback callback results or throwing when promisifying native setTimeout. Add regression coverage for both interoperability directions and ordinary callbacks. A minimal reproduction is included in the linked issue.
Validation
On original main: 3 regressions failed, 1 control passed. All 4 tests pass after the fix.
Targeted ESLint and Prettier checks passed.
pnpm test:types, pnpm build, and git diff --check passed.
The built public export unenv/node/util passed custom-hook and native-timer smoke checks.
Checks used Node.js 24.21.0. The full suite and workerd tests were not run. AI disclosure: OpenAI Codex prepared and tested this change; the diff was reviewed and the checks above were run locally.
Summary by CodeRabbit
Bug Fixes
Improved compatibility between the runtime’s promisify behavior and Node.js, including support for shared custom implementations and Node.js functions such as setTimeout.
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: e99c5327-00a3-4ea8-ab78-47b826732244
📥 Commits
Reviewing files that changed from the base of the PR and between f89b7cc and f10ba1a.
📒 Files selected for processing (2)
src/runtime/node/internal/util/promisify.ts
test/node/test-util-promisify.mjs
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
📝 Walkthrough
The runtime promisify now uses Node.js’s globally registered custom symbol. Tests cover shared custom implementations, Node.js setTimeout, and ordinary error-first callbacks.
Changes: Promisify interoperability
Layer / File(s)
Summary
Custom symbol and regression tests src/runtime/node/internal/util/promisify.ts, test/node/test-util-promisify.mjs
customSymbol now uses Symbol.for("nodejs.util.promisify.custom"). Tests cover custom implementation interoperability with Node.js, promisifying setTimeout, and ordinary callback support.
The change aligns runtime promisification with Node.js’s shared custom symbol and adds regression coverage for interoperability. No actionable mergeability risk is apparent.
Security architecture risk:⚪ Minimal · up to f10ba
The change enables intended Node.js interoperability without introducing a new privileged operation or crossing an authorization boundary. Custom implementations remain attached to caller-supplied function objects.
Retained concerns
No architecture-level concerns identified.
Security review details
Security Blast Radius
inferred — The changed exposure is custom-hook selection for function objects passed to promisify. The inspected path does not introduce a network input, credential use, persistent write, or privileged sink; downstream effects depend on the supplied function and its custom implementation.
Trust Boundaries and Controls
inferred — Recognizing the well-known symbol activates hooks that the private-symbol implementation ignored, but does not itself grant authority to attach a hook to another function. The caller already supplies executable behavior, and the previous public custom property already allowed hook registration. Symbol uniqueness therefore was not a demonstrated security boundary in this implementation.
Pre-merge checks · 4 · 1
Failed checks (1 warning)
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files.
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks (4 passed)
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly and concisely describes the main change: using the shared promisify custom symbol in node:util.
Linked Issues check
Issue #569 requires unenv/node/utilpromisify to honor Symbol.for("nodejs.util.promisify.custom"). The implementation now uses that shared symbol in src/runtime/node/internal/util/promisify.ts…
Out of Scope Changes check
The changes are limited to the promisify symbol update and focused regression tests. Each test covers interoperability or callback behavior required by issue #569. No unrelated change is shown.
Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
🔧 ESLint
If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.
ESLint install timed out. The project may have too many dependencies for the sandbox.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves #569.
Use Node.js's globally registered
promisify.customsymbol so custom implementations work across unenv, native Node.js, and other libraries.Previously, unenv ignored these hooks, producing fallback callback results or throwing when promisifying native
setTimeout. Add regression coverage for both interoperability directions and ordinary callbacks. A minimal reproduction is included in the linked issue.Validation
pnpm test:types,pnpm build, andgit diff --checkpassed.unenv/node/utilpassed custom-hook and native-timer smoke checks.Checks used Node.js 24.21.0. The full suite and workerd tests were not run. AI disclosure: OpenAI Codex prepared and tested this change; the diff was reviewed and the checks above were run locally.
Summary by CodeRabbit
setTimeout.