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.

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

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

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.

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

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

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