Skip to content

async_hooks: use validateBoolean for trackPromises#64731

Open
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:async-hooks-validate-boolean
Open

async_hooks: use validateBoolean for trackPromises#64731
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:async-hooks-validate-boolean

Conversation

@soulee-dev

Copy link
Copy Markdown

The trackPromises validation added in #61415 hand-rolls a check that validateBoolean() already performs β€” same error code, same expected type string, same argument order. This replaces it with the validator.

lib/async_hooks.js already imports validateString() and validateFunction() and uses them for type and fn, so this also makes the file internally consistent. With the last manual check gone, the ERR_INVALID_ARG_TYPE import is dropped as well.

No behavior change: validateBoolean() is wrapped in hideStackFrames(), so the thrown error is identical from a caller's perspective, including the top stack frame. The existing coverage in test/async-hooks/test-track-promises-validation.js already asserts this, so no new tests are added.

Refs: #61415

The `trackPromises` check duplicated `validateBoolean()` exactly: same
error code, same expected type string, same argument order. Replace it
with the validator, matching how this file already validates `type` and
`fn` via `validateString()`/`validateFunction()`.

`ERR_INVALID_ARG_TYPE` had no other use in this file, so drop the
import. Since `validateBoolean` is wrapped in `hideStackFrames()`, the
thrown error is unchanged from a caller's perspective.

Signed-off-by: Soul Lee <alus20x@gmail.com>
@nodejs-github-bot nodejs-github-bot added async_hooks Issues and PRs related to the async hooks subsystem. needs-ci PRs that need a full CI run. labels Jul 25, 2026
@codecov

codecov Bot commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

βœ… All modified and coverable lines are covered by tests.
βœ… Project coverage is 90.14%. Comparing base (0618e9f) to head (3ce515c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64731      +/-   ##
==========================================
- Coverage   90.14%   90.14%   -0.01%     
==========================================
  Files         743      743              
  Lines      242408   242408              
  Branches    45656    45650       -6     
==========================================
- Hits       218520   218507      -13     
  Misses      15387    15387              
- Partials     8501     8514      +13     
Files with missing lines Coverage Ξ”
lib/async_hooks.js 100.00% <100.00%> (ΓΈ)

... and 31 files with indirect coverage changes

πŸš€ New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • πŸ“¦ JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@soulee-dev

Copy link
Copy Markdown
Author

@daeyeon Thanks for approving the CI run!

The only failing check is aarch64-linux: with shared boringssl on parallel/test-repl-function-definition-edge-case.

The same test-shared.yml workflow fails on main, with the same test and error (e.g. https://github.com/nodejs/node/actions/runs/30136256627).

make test passes locally on this branch. Could you re-trigger CI?

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

Labels

async_hooks Issues and PRs related to the async hooks subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants