From cfca490a760c350296340b611ae2f7cc9f9aa4a6 Mon Sep 17 00:00:00 2001
From: Abhijeet Prasad <aprasad@sentry.io>
Date: Thu, 10 Nov 2022 11:59:03 +0100
Subject: [PATCH] fix(react): Guard against non-error obj in ErrorBoundary

---
 packages/react/src/errorboundary.tsx       |  9 +++-
 packages/react/test/errorboundary.test.tsx | 50 ++++++++++++++++++++++
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/packages/react/src/errorboundary.tsx b/packages/react/src/errorboundary.tsx
index 74670e25fa87..7aae1fbf6d08 100644
--- a/packages/react/src/errorboundary.tsx
+++ b/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';
 
@@ -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;
diff --git a/packages/react/test/errorboundary.test.tsx b/packages/react/test/errorboundary.test.tsx
index c2c2ea1ac268..7b0c25dc311a 100644
--- a/packages/react/test/errorboundary.test.tsx
+++ b/packages/react/test/errorboundary.test.tsx
@@ -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();