Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(react): Guard against non-error obj in ErrorBoundary #6181

Merged
merged 1 commit into from Nov 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions packages/react/src/errorboundary.tsx
@@ -1,5 +1,5 @@
import { captureException, ReportDialogOptions, Scope, showReportDialog, withScope } from '@sentry/browser';
import { logger } from '@sentry/utils';
import { isError, logger } from '@sentry/utils';
import hoistNonReactStatics from 'hoist-non-react-statics';
import * as React from 'react';

Expand Down Expand Up @@ -75,7 +75,12 @@ class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundarySta
// If on React version >= 17, create stack trace from componentStack param and links
// to to the original error using `error.cause` otherwise relies on error param for stacktrace.
// Linking errors requires the `LinkedErrors` integration be enabled.
if (isAtLeastReact17(React.version)) {
// See: https://reactjs.org/blog/2020/08/10/react-v17-rc.html#native-component-stacks
//
// Although `componentDidCatch` is typed to accept an `Error` object, it can also be invoked
// with non-error objects. This is why we need to check if the error is an error-like object.
// See: https://github.com/getsentry/sentry-javascript/issues/6167
if (isAtLeastReact17(React.version) && isError(error)) {
const errorBoundaryError = new Error(error.message);
errorBoundaryError.name = `React ErrorBoundary ${errorBoundaryError.name}`;
errorBoundaryError.stack = componentStack;
Expand Down
50 changes: 50 additions & 0 deletions packages/react/test/errorboundary.test.tsx
Expand Up @@ -248,6 +248,56 @@ describe('ErrorBoundary', () => {
expect(cause.message).toEqual(error.message);
});

// Regression test against:
// https://github.com/getsentry/sentry-javascript/issues/6167
it('does not set cause if non Error objected is thrown', () => {
const TestAppThrowingString: React.FC<ErrorBoundaryProps> = ({ children, ...props }) => {
const [isError, setError] = React.useState(false);
function StringBam(): JSX.Element {
throw 'bam';
}
return (
<ErrorBoundary
{...props}
onReset={(...args) => {
setError(false);
if (props.onReset) {
props.onReset(...args);
}
}}
>
{isError ? <StringBam /> : children}
<button
data-testid="errorBtn"
onClick={() => {
setError(true);
}}
/>
</ErrorBoundary>
);
};

render(
<TestAppThrowingString fallback={<p>You have hit an error</p>}>
<h1>children</h1>
</TestAppThrowingString>,
);

expect(mockCaptureException).toHaveBeenCalledTimes(0);

const btn = screen.getByTestId('errorBtn');
fireEvent.click(btn);

expect(mockCaptureException).toHaveBeenCalledTimes(1);
expect(mockCaptureException).toHaveBeenLastCalledWith('bam', {
contexts: { react: { componentStack: expect.any(String) } },
});

// Check if error.cause -> react component stack
const error = mockCaptureException.mock.calls[0][0];
expect(error.cause).not.toBeDefined();
});

it('calls `beforeCapture()` when an error occurs', () => {
const mockBeforeCapture = jest.fn();

Expand Down