lib: propagate aborted state to dependent signals before firing events

[jazelly]

Fixes: #54466
Fixes: #54601

[jazelly]

I am not sure if I should update WPT together with this PR, considering @RedYetiDev has drafted a PR to cover this.

[RedYetiDev]

I am not sure if I should update WPT together with this PR, considering @RedYetiDev has drafted a PR to cover this.

I don't think the WPT files need to be updated in this PR, IMO one thing at a time.

But, if you could add a test, that'd be helpful IMO.

[jazelly]

sure, I'll add a test

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.04%. Comparing base (59c7c55) to head (39b8e88).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #54826   +/-   ##
=======================================
  Coverage   88.03%   88.04%           
=======================================
  Files         652      652           
  Lines      183763   183793   +30     
  Branches    35860    35861    +1     
=======================================
+ Hits       161774   161813   +39     
+ Misses      15237    15232    -5     
+ Partials     6752     6748    -4     

Files with missing lines	Coverage Δ
lib/internal/abort_controller.js	98.03% <100.00%> (+0.12%)	⬆️

... and 26 files with indirect coverage changes

[RedYetiDev]

Nice!

CC @shaseley (from the spec proposal, authored the original issue)

[github-actions]

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @jasnell.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

[github-actions]

Failed to start CI

   ⚠  Something was pushed to the Pull Request branch since the last approving review.
   ✘  Refusing to run CI on potentially unsafe PR

https://github.com/nodejs/node/actions/runs/10855585828

[jasnell]

Probably safest to mark it as such

[jazelly]

looks like I need another approval after addressing the PR comments.

[jakecastelli]

Looks like this PR has merge conflicts now

[RedYetiDev]

• CI Passing
• Collaborator Approvals

I've applied the "author ready" label, as this should be ready for an author to 🚀 land it.

[aduh95]

this should be ready for an author to 🚀 land it.

The only author in the PR is jazelly. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. does not mean the PR should be merged by its author, rather than the author has nothing more to do.

[jakecastelli]

Hi folks, I was wondering if it makes sense to let the PR author also update the WPT related tests in the same PR (in separate commits), I do appreciate the effort of doing so in 54468 @RedYetiDev, on the other hand I feel like it may be more consistent to have them in one PR (easy to know if any relate test would fail or not) plus for future debugging / tracing purpose.
cc. @jazelly @aduh95 @jasnell

[RedYetiDev]

SGTM, I'm happy to have my PR just take care of the leftovers (that haven't been updated from PRs like this)

[jazelly]

Awesome. I have updated the PR with latest wpt for abort with a separate commit. Also addressed PR comments and redundant test cases in the original commit. PTAL

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/62550/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/62553/ 💚