fix(react): Remove unused react.componentStack event context#21183
fix(react): Remove unused react.componentStack event context#21183zhongrenfei1-hub wants to merge 1 commit into
react.componentStack event context#21183Conversation
The `react.componentStack` event context set by `captureReactException`
on every captured error never gets sourcemaps applied, so the string it
ships is mostly unreadable. For React >= 17 the same component stack is
already attached via `error.cause` (which does get sourcemaps); for
React < 17 it offers little value either.
Drop the `setContext('react', ...)` call. The surrounding `withScope`
wrapper has no other mutation, so collapse it to a direct
`captureException` call and drop the now-unused `withScope` import.
Sync the spy assertions in `errorboundary.test.tsx`: remove the
`scopeSetContextSpy` setup and rewrite the `cause.stack`-versus-mock
expectations to `expect.any(String)` — the strongest invariant we can
still observe once the spy is gone, since the underlying `setCause` /
`errorBoundaryError.stack = componentStack` chain is unchanged. The
negation case in the recursive-cause test collapses into the existing
`cause.name` assertion, with an inline comment explaining why the
original (non-ErrorBoundary) cause is preserved when the chain loops.
Closes getsentry#20094
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
zhongrenfei1-hub
requested review from
nicohrubec and
s1gr1d
and removed request for
a team
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 89d1a79. Configure here.
| mechanism: { handled: true, type: 'auto.function.react.error_boundary' }, | ||
| }); | ||
|
|
||
| expect(scopeSetContextSpy).toHaveBeenCalledTimes(1); |
There was a problem hiding this comment.
Leftover mockClear() call on non-spy method breaks tests
High Severity
The vi.spyOn(mockScope, 'setContext') call was removed, but the afterEach block still calls (mockScope.setContext as any).mockClear(). Since mockScope.setContext is no longer a spy, it's a plain Scope method without a .mockClear() property — this will throw a TypeError at runtime, causing every test inside describe('ErrorBoundary', ...) to fail.
Reviewed by Cursor Bugbot for commit 89d1a79. Configure here.
|
Hi, I just looked at the linked issue and I'm wondering if this should even considered a "bug". Does removing this fix a problem? |
I think the problem is the unnecessary and confusing minified stack trace added to events as scope! |
3 participants
Summary
Removes the
react.componentStackevent context set bycaptureReactException.The shipped string never has sourcemaps applied, so it's mostly unreadable in the Sentry UI. For React >= 17 the same component stack is already attached via
error.cause(which does get sourcemaps); for React < 17 it offers little value either. Maintainer-confirmed in #20094.Root cause
captureReactExceptionwrapped the capture inwithScopesolely to attach thereact.componentStackcontext. With that context gone thewithScopewrapper has no other mutation, so it collapses to a directcaptureExceptioncall (andwithScopedrops out of the imports).Test sync:
scopeSetContextSpyinerrorboundary.test.tsxis removed. The twoexpect(cause.stack).toEqual(scopeSetContextSpy.mock.calls[...])assertions becomeexpect.any(String)— the strongest invariant left now that the spy is gone, since the underlyingsetCause/errorBoundaryError.stack = componentStackchain is unchanged. The recursive-cause negation collapses into the existingcause.nameassertion, with an inline comment explaining why the original cause is preserved when the chain loops.Fixes #20094