fix unhandledRejection misfires

[anonrig]

Fixes #6020

This change fixes premature unhandledrejection events by running unhandled‑rejection processing only after V8 finishes a microtask checkpoint. We now schedule the check via V8's microtask‑completed callback and request an extra checkpoint so any handlers queued by unhandledrejection listeners run promptly. We also keep promise/value references strong until the event is dispatched, then downgrade to weak to avoid leaks.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 76.81159% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.37%. Comparing base (aef76c7) to head (4f41f1d).
⚠️ Report is 13 commits behind head on main.

Files with missing lines	Patch %	Lines
src/workerd/jsg/promise.c++	76.59%	8 Missing and 3 partials ⚠️
src/workerd/jsg/jsg.c++	64.28%	4 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6049      +/-   ##
==========================================
+ Coverage   70.34%   70.37%   +0.02%     
==========================================
  Files         408      408              
  Lines      108646   108852     +206     
  Branches    17991    18007      +16     
==========================================
+ Hits        76431    76604     +173     
- Misses      21412    21440      +28     
- Partials    10803    10808       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.

[jasnell]

Initial review pass done. I'm going to want to check this out locally at do a thorough test on it tho... which I can't do at the moment as I'm in the middle of another investigation. Will come back to this and test locally as soon as I'm able

[jasnell]

Overall the change is fine. I do worry about the possibility of this breaking someone tho. Specifically, it can change behavior in an existing worker. Consider:

addEventListener('unhandledrejection', () => {
  console.log('does not fire');
});

export default {
  async fetch() {
    Promise.reject('boom');
    await Promise.resolve();
    return new Response('ok');
  }
}

In the current code, the unhandledrejection event will trigger. With the new PR it will not.

If we change the awaited promise to await actual i/o, the handler DOES fire.

addEventListener('unhandledrejection', () => {
  console.log('fires');
});

export default {
  async fetch() {
    Promise.reject('boom');
    await scheduler.resolve();
    return new Response('ok');
  }
}

If a worker has come to rely on the current behavior (unlikely but possible) then their worker could end up broken.

Given that, I think to be completely safe this might need a compat flag... but I'm also open to being convinced otherwise.

[anonrig]

I don't see any problem with being a little bit more cautious and surround this change with a compatibility flag. @jasnell

[codspeed-hq]

Merging this PR will degrade performance by 10.92%

❌ 1 regressed benchmark
✅ 69 untouched benchmarks
⏩ 129 skipped benchmarks¹

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Benchmark	BASE	HEAD	Efficiency
❌	Encode_ASCII_32[TextEncoder][0/0/32]	3.1 ms	3.4 ms	-10.92%

---

_{Comparing yagiz/fix-misfires-in-unhandledrejection (4f41f1d) with main (6ff050c)}

Footnotes

1. 129 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[jasnell]

I'm not super familiar with the microtask checkpoint callback being used here so while the changes LGTM, I'd like for you to get either @erikcorry or @dcarney-cf review just to make sure there's not something lurking there that we're missing.