test(renderer): keep the TaskProvider guard throw inside the component - #264
Merged
Conversation
The guard test rendered a component that throws and let the throw escape the render. React's dev-only replay rethrows an escaped render error through a synthetic DOM event, jsdom turns that into an uncancelled window "error" event, and Vitest's jsdom environment re-emits such an event as an uncaught exception whenever no other error listener happens to be registered at that moment. That fails the whole run and blames whichever file was running at the time. Catch the throw inside the component instead, so React never sees it. A console.error spy pins the behaviour: React logs for every render throw it has to handle itself, so letting the throw escape again fails this test directly rather than surfacing somewhere else in the run.
Zaldaryon
approved these changes
Aug 28, 2026
Zaldaryon
left a comment
Collaborator
There was a problem hiding this comment.
Approving. Test-only change, production code untouched, and it removes a real source of run-level flake.
What I verified
- The mechanism in the PR body holds up. The old
expect(() => renderHook(() => useTaskContext())).toThrow(...)lets the render throw escape into React's dev replay, which reaches jsdom's windowerrorevent, and Vitest's jsdom environment re-emits an uncancelled one as an uncaught exception. That fails the run and pins the error on whatever file was running. - Catching the throw inside the render callback is the right fix. React's
beginWorknever sees an error, so there is no guarded replay and nothing for jsdom or Vitest to escalate. The guard onTaskManagerContext.tsx:404still runs (no provider,useContextreturns null,throw), so branch coverage is unchanged. - Ran locally on
f7d0242:npm run typecheckclean,vitest run tests/renderer-dom/taskManagerFlows.test.tsxgives 28 passed with clean output and none of the two error blocks the PR describes. eslint on the file is clean. - All five required checks and sonarcloud are green.
- The
console.errorspy withnot.toHaveBeenCalled()is a good regression pin: React logs "The above error occurred in ..." for any render throw it handles itself, so a future revert to the escaping form fails in this file rather than as a mystery run-level error.
Non-blocking
- The suite now has two patterns for the same problem: this PR prevents the escape,
configContextSlices.test.tsx:167silences it with aconsole.errormock plus a cancelling window listener. You called that out and scoped it out on purpose. Worth unifying on the catch-inside pattern later, since it does not lean on jsdom internals or global listener state. vi.spyOn(console, "error")without amockImplementationmeans a broken premise both prints noise and fails the assertion. Fine, it fails loud.
This was referenced Aug 28, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What happened
A full
npm run test:coveragerun failed once on a green branch withError: useTaskContext must be used within an TaskProvider, pointing attaskManagerFlows.test.tsx:82. Line 82 is the test that deliberately asserts that guard throws, so the test itself was fine. The error was arriving as a run-level unhandled error, which is why it looked random and why it never showed up when the file ran alone.Reproduction
I could not force a red run. 27 clean shuffled runs of the renderer-dom project (
--sequence.shufflewith seeds 1001 through 1029, minus three runs I contaminated with my own probe file) all passed, and 17 full coverage runs all passed. So no failing seed to hand over.What does reproduce, on every single run, is the error itself. Every green full coverage run printed this at the top of its output, twice, with no test file attributed to it:
That was the only stderr output the whole suite produced. Same message, same stack, same file, same line as the failure. It just usually lands as noise instead of as a failure.
The mechanism
The test rendered a component that throws and let the throw escape the render:
React's development build replays a failed
beginWorkthroughinvokeGuardedCallbackDev, which dispatches a synthetic event on a detached node so the throw reaches the host's uncaught error machinery and devtools can pause on it. That is theHTMLUnknownElement.callCallbackframe in the stack. jsdom then catches the listener exception and fires anerrorevent onwindow, and since nothing cancels it, jsdom reports it to the console. React attempts the render twice, hence two copies.Vitest's jsdom environment installs its own
windowerror listener. When an uncancelled error event arrives and no user listener is currently registered, it callspreventDefault()and re-emits the error asprocess.emit("uncaughtException", ...). I confirmed that route with a throwaway test that dispatches one uncancelled error event and nothing else:The run exits non-zero, the file passes, and the error is pinned on whichever file was running. That is exactly the shape of the reported failure.
The only thing standing between the guard test and that outcome is that React registers its own transient window error listener around the dispatch, which bumps Vitest's user-listener count to one for the duration. I instrumented the window to watch it, and measured the event arriving at depth 1 with
defaultPreventedfalse every time. That count is a single window-wide integer, and Vitest decrements it on anyremoveEventListener("error", ...)whether or not a matching add ever happened. I watched it go to -1 during environment teardown. So the suppression is incidental, not designed, and it holds only as long as nothing else in the same window touches error listeners at the wrong moment.The fix
Catch the throw inside the component so React never handles it. No guarded replay, no synthetic event, no window error, nothing for Vitest to escalate. The guard on
TaskManagerContext.tsx:404is still executed and its message still asserted, so coverage is unchanged.configContextSlices.test.tsxhas the same kind of test and is already safe. It goes the other way and silences the escape with aconsole.errormock plus a listener that cancels the event. That works, so I left it alone.The pin
The test now spies on
console.errorand asserts it was never called. React logs "The above error occurred in ..." for every render throw it has to handle itself, so anyone who reverts to letting the throw escape gets a failure in this file rather than a mystery unhandled error somewhere else in the run. Verified by putting the old escaping version back with the spy in place:Gates
npm run typecheck,npm run lint:ci(0 errors, the same 15 pre-existing warnings) andnpm run format:checkall pass. Three consecutivenpm run test:coverageruns came back green at 137 files and 1633 passed with 2 skipped, coverage at 92.58 lines, 89.8 statements, 92.03 functions and 94.05 branches, all comfortably over the floors. The run output is now clean, where before every run printed those two error blocks.