You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[v2] e2e protocol:timeout:reset-on-progress leaves unhandled rejections on its known-failing SSE variants, so vitest exits 1 on macOS #3170
Upstream CI on Linux runs the same suite with the same expected failures and reports no unhandled errors, so this only shows up locally.
Where it comes from
protocol:timeout:reset-on-progress lists sse under knownFailures in test/e2e/requirements.ts (the SSE batching race shared with protocol:progress:callback). On those two variants the test fails as expected at:
While vi.waitFor polls, it advances the fake clock, so the 200ms request timeout fires and call rejects. The test has already left the path that reaches await call, and nothing else handles call, so the rejection is reported as unhandled.
Isolating by transport (-t in test/e2e, main + #3130, macOS, Node 24.16.0, 3 runs each):
Variants
Result
reset-on-progress [sse ...] only
2 expected fail, 1–2 unhandled errors every run
reset-on-progress, all other transports
10 passed, no errors
Possible fix
Attach a handler right after call is created, as protocol:timeout:max-total already does:
call.catch(()=>{});
The passing variants still await call and assert on the result, so nothing is lost. I tried it locally: reset-on-progress still gives 10 passed and 2 expected fail with no unhandled errors (3 runs), and the full e2e suite exits 0 (3126 passed, 141 expected fail).
I haven't looked into why Linux doesn't report these rejections. Happy to open a one-line test-only PR once #3130 is settled.
Context
Found while working on #3045 / #3130. That issue attributed an unhandled Request timed out to max-total; it was this test, as the #3130 description notes.
The handler belongs at creation, not in the failure branch, and the timing is the reason. The known-failure branch is entered after the first poll iteration, and the 200ms timer can fire before the poll that would have reached it. Anything attached later competes with the rejection it is meant to catch.
Two refinements on the fix itself, since it will live in the suite for a long time.
Record the rejection instead of discarding it. A handler that swallows whatever arrives, catch(() => {}), also swallows a real protocol failure that happens to surface on that promise, and the suite loses the only signal it had. Capturing the reason and asserting on it in the known-failure branch keeps the test honest: it still proves the expected outcome is a timeout, and a different rejection fails the run with a useful message rather than being absorbed. The variant list already says which cases are expected to fail, so the assertion is adding no new knowledge, only keeping it checkable.
On the macOS and Linux difference, I would not guess without one measurement, because there are at least two candidates that look identical from outside. The first is harness configuration: pool mode and the settings that govern unhandled errors differ between runs, and a local config that relaxes them would hide the rejection locally rather than creating it locally, which is the opposite of what you see. The second is attribution: whether the rejection is stamped to the test that owns the timer depends on when the timer callback runs relative to the test boundary, and a slower or faster platform can move a rejection from inside a test to after it. Printing process.platform and the owning test name from an unhandledRejection hook, then comparing local and CI, separates them in one run and costs less than reasoning about which is likelier.
The broader point deserves a line in the PR description. A suite that exits nonzero for environmental reasons trains reviewers to ignore the exit code, and the next real unhandled rejection arrives in silence. Fixing this one is what restores the signal.
Your read of #3045 is worth stating plainly in the PR: the earlier attribution to max-total was wrong, and the evidence is that isolating the variants by transport reproduces the failures only on the SSE pair that the requirements file already marks as known failures. That is a stronger justification for a test-only change than the one-line diff suggests.
Activity
media-ctrl commentedon Nov 11, 2026
The handler belongs at creation, not in the failure branch, and the timing is the reason. The known-failure branch is entered after the first poll iteration, and the 200ms timer can fire before the poll that would have reached it. Anything attached later competes with the rejection it is meant to catch.
Two refinements on the fix itself, since it will live in the suite for a long time.
Record the rejection instead of discarding it. A handler that swallows whatever arrives,
catch(() => {}), also swallows a real protocol failure that happens to surface on that promise, and the suite loses the only signal it had. Capturing the reason and asserting on it in the known-failure branch keeps the test honest: it still proves the expected outcome is a timeout, and a different rejection fails the run with a useful message rather than being absorbed. The variant list already says which cases are expected to fail, so the assertion is adding no new knowledge, only keeping it checkable.On the macOS and Linux difference, I would not guess without one measurement, because there are at least two candidates that look identical from outside. The first is harness configuration: pool mode and the settings that govern unhandled errors differ between runs, and a local config that relaxes them would hide the rejection locally rather than creating it locally, which is the opposite of what you see. The second is attribution: whether the rejection is stamped to the test that owns the timer depends on when the timer callback runs relative to the test boundary, and a slower or faster platform can move a rejection from inside a test to after it. Printing
process.platformand the owning test name from anunhandledRejectionhook, then comparing local and CI, separates them in one run and costs less than reasoning about which is likelier.The broader point deserves a line in the PR description. A suite that exits nonzero for environmental reasons trains reviewers to ignore the exit code, and the next real unhandled rejection arrives in silence. Fixing this one is what restores the signal.
Your read of #3045 is worth stating plainly in the PR: the earlier attribution to
max-totalwas wrong, and the evidence is that isolating the variants by transport reproduces the failures only on the SSE pair that the requirements file already marks as known failures. That is a stronger justification for a test-only change than the one-line diff suggests.