Skip to content

fix(node:util): use the shared promisify custom symbol - #570

Open
00200200 wants to merge 1 commit into
unjs:mainfrom
00200200:codex/fix-promisify-custom-symbol
Open

00200200 wants to merge 1 commit into
unjs:mainfrom
00200200:codex/fix-promisify-custom-symbol

Conversation

@00200200

@00200200 00200200 commented Oct 13, 2026 •

Copy link
Copy Markdown

Summary

Resolves #569.

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.
    • Confirmed ordinary callback usage remains supported.

@00200200
00200200 requested a review from pi0 as a code owner October 13, 2026 10:52
@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to f10ba

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 · Passed 4 · Failed 1

Failed Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning 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 Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: using the shared promisify custom symbol in node:util.
Linked Issues check Passed Issue #569 requires unenv/node/util promisify 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 Passed 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

node:util promisify ignores the globally shared custom symbol

1 participant