feat(react): Add support for React 17 Error Boundaries (#3532)

AbhiPrasad · web-flow · commit 60e1982f71b2 · 2021-05-14T12:33:26.000Z
This PR updates the @sentry/react error boundary component to use React 17's better component stack traces. The React component stack trace is generated from the componentStack property taken from the ErrorBoundary's componentDidCatch method.
diff --git a/packages/gatsby/package.json b/packages/gatsby/package.json
@@ -39,7 +39,7 @@
     "jest": "^24.7.1",
     "npm-run-all": "^4.1.2",
     "prettier": "1.19.0",
-    "react": "^16.13.1",
+    "react": "^17.0.0",
     "rimraf": "^2.6.3",
     "typescript": "3.7.5"
   },
diff --git a/packages/gatsby/test/integration.test.tsx b/packages/gatsby/test/integration.test.tsx
@@ -1,5 +1,6 @@
 /* eslint-disable @typescript-eslint/no-explicit-any */
 import { render } from '@testing-library/react';
+import { useEffect } from 'react';
 import * as React from 'react';
 
 import { onClientEntry } from '../gatsby-browser';
@@ -14,7 +15,7 @@ describe('useEffect', () => {
     let calls = 0;
 
     onClientEntry(undefined, {
-      beforeSend: event => {
+      beforeSend: (event: any) => {
         expect(event).not.toBeUndefined();
         calls += 1;
 
@@ -24,7 +25,7 @@ describe('useEffect', () => {
 
     // eslint-disable-next-line @typescript-eslint/no-unused-vars
     function TestComponent() {
-      React.useEffect(() => {
+      useEffect(() => {
         const error = new Error('testing 123');
         (window as any).Sentry.captureException(error);
       });
diff --git a/packages/react/package.json b/packages/react/package.json
@@ -46,8 +46,8 @@
     "jsdom": "^16.2.2",
     "npm-run-all": "^4.1.2",
     "prettier": "1.19.0",
-    "react": "^16.0.0",
-    "react-dom": "^16.0.0",
+    "react": "^17.0.0",
+    "react-dom": "^17.0.0",
     "react-router-3": "npm:react-router@3.2.0",
     "react-router-4": "npm:react-router@4.1.0",
     "react-router-5": "npm:react-router@5.0.0",
diff --git a/packages/react/src/errorboundary.tsx b/packages/react/src/errorboundary.tsx
@@ -1,7 +1,19 @@
-import { captureException, ReportDialogOptions, Scope, showReportDialog, withScope } from '@sentry/browser';
+import {
+  captureEvent,
+  captureException,
+  eventFromException,
+  ReportDialogOptions,
+  Scope,
+  showReportDialog,
+  withScope,
+} from '@sentry/browser';
+import { Event } from '@sentry/types';
+import { parseSemver } from '@sentry/utils';
 import hoistNonReactStatics from 'hoist-non-react-statics';
 import * as React from 'react';
 
+const reactVersion = parseSemver(React.version);
+
 export const UNKNOWN_COMPONENT = 'unknown';
 
 export type FallbackRender = (errorData: {
@@ -53,8 +65,47 @@ const INITIAL_STATE = {
 };
 
 /**
- * A ErrorBoundary component that logs errors to Sentry.
- * Requires React >= 16
+ * Logs react error boundary errors to Sentry. If on React version >= 17, creates stack trace
+ * from componentStack param, otherwise relies on error param for stacktrace.
+ *
+ * @param error An error captured by React Error Boundary
+ * @param componentStack The component stacktrace
+ */
+function captureReactErrorBoundaryError(error: Error, componentStack: string): string {
+  const errorBoundaryError = new Error(error.message);
+  errorBoundaryError.name = `React ErrorBoundary ${errorBoundaryError.name}`;
+  errorBoundaryError.stack = componentStack;
+
+  let errorBoundaryEvent: Event = {};
+  void eventFromException({}, errorBoundaryError).then(e => {
+    errorBoundaryEvent = e;
+  });
+
+  if (
+    errorBoundaryEvent.exception &&
+    Array.isArray(errorBoundaryEvent.exception.values) &&
+    reactVersion.major &&
+    reactVersion.major >= 17
+  ) {
+    let originalEvent: Event = {};
+    void eventFromException({}, error).then(e => {
+      originalEvent = e;
+    });
+    if (originalEvent.exception && Array.isArray(originalEvent.exception.values)) {
+      originalEvent.exception.values = [...errorBoundaryEvent.exception.values, ...originalEvent.exception.values];
+    }
+
+    return captureEvent(originalEvent);
+  }
+
+  return captureException(error, { contexts: { react: { componentStack } } });
+}
+
+/**
+ * A ErrorBoundary component that logs errors to Sentry. Requires React >= 16.
+ * NOTE: If you are a Sentry user, and you are seeing this stack frame, it means the
+ * Sentry React SDK ErrorBoundary caught an error invoking your application code. This
+ * is expected behavior and NOT indicative of a bug with the Sentry React SDK.
  */
 class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundaryState> {
   public state: ErrorBoundaryState = INITIAL_STATE;
@@ -66,7 +117,7 @@ class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundarySta
       if (beforeCapture) {
         beforeCapture(scope, error, componentStack);
       }
-      const eventId = captureException(error, { contexts: { react: { componentStack } } });
+      const eventId = captureReactErrorBoundaryError(error, componentStack);
       if (onError) {
         onError(error, componentStack, eventId);
       }
diff --git a/packages/react/test/errorboundary.test.tsx b/packages/react/test/errorboundary.test.tsx
@@ -1,19 +1,21 @@
 import { Scope } from '@sentry/browser';
+import { Event, Severity } from '@sentry/types';
 import { fireEvent, render, screen } from '@testing-library/react';
 import * as React from 'react';
+import { useState } from 'react';
 
 import { ErrorBoundary, ErrorBoundaryProps, UNKNOWN_COMPONENT, withErrorBoundary } from '../src/errorboundary';
 
-const mockCaptureException = jest.fn();
+const mockCaptureEvent = jest.fn();
 const mockShowReportDialog = jest.fn();
 const EVENT_ID = 'test-id-123';
 
 jest.mock('@sentry/browser', () => {
   const actual = jest.requireActual('@sentry/browser');
   return {
     ...actual,
-    captureException: (err: any, ctx: any) => {
-      mockCaptureException(err, ctx);
+    captureEvent: (event: Event) => {
+      mockCaptureEvent(event);
       return EVENT_ID;
     },
     showReportDialog: (options: any) => {
@@ -22,6 +24,15 @@ jest.mock('@sentry/browser', () => {
   };
 });
 
+function Boo({ title }: { title: string }): JSX.Element {
+  throw new Error(title);
+}
+
+function Bam(): JSX.Element {
+  const [title] = useState('boom');
+  return <Boo title={title} />;
+}
+
 const TestApp: React.FC<ErrorBoundaryProps> = ({ children, ...props }) => {
   const [isError, setError] = React.useState(false);
   return (
@@ -45,10 +56,6 @@ const TestApp: React.FC<ErrorBoundaryProps> = ({ children, ...props }) => {
   );
 };
 
-function Bam(): JSX.Element {
-  throw new Error('boom');
-}
-
 describe('withErrorBoundary', () => {
   it('sets displayName properly', () => {
     const TestComponent = () => <h1>Hello World</h1>;
@@ -67,7 +74,7 @@ describe('ErrorBoundary', () => {
   jest.spyOn(console, 'error').mockImplementation();
 
   afterEach(() => {
-    mockCaptureException.mockClear();
+    mockCaptureEvent.mockClear();
     mockShowReportDialog.mockClear();
   });
 
@@ -170,10 +177,15 @@ describe('ErrorBoundary', () => {
       expect(container.innerHTML).toBe('<div>Fallback here</div>');
 
       expect(errorString).toBe('Error: boom');
-      expect(compStack).toBe(`
-    in Bam (created by TestApp)
-    in ErrorBoundary (created by TestApp)
-    in TestApp`);
+      /*
+        at Boo (/path/to/sentry-javascript/packages/react/test/errorboundary.test.tsx:23:20)
+        at Bam (/path/to/sentry-javascript/packages/react/test/errorboundary.test.tsx:40:11)
+        at ErrorBoundary (/path/to/sentry-javascript/packages/react/src/errorboundary.tsx:2026:39)
+        at TestApp (/path/to/sentry-javascript/packages/react/test/errorboundary.test.tsx:22:23)
+      */
+      expect(compStack).toMatch(
+        /\s+(at Boo) \(.*?\)\s+(at Bam) \(.*?\)\s+(at ErrorBoundary) \(.*?\)\s+(at TestApp) \(.*?\)/g,
+      );
       expect(eventIdString).toBe(EVENT_ID);
     });
   });
@@ -188,25 +200,60 @@ describe('ErrorBoundary', () => {
       );
 
       expect(mockOnError).toHaveBeenCalledTimes(0);
-      expect(mockCaptureException).toHaveBeenCalledTimes(0);
+      expect(mockCaptureEvent).toHaveBeenCalledTimes(0);
 
       const btn = screen.getByTestId('errorBtn');
       fireEvent.click(btn);
 
       expect(mockOnError).toHaveBeenCalledTimes(1);
       expect(mockOnError).toHaveBeenCalledWith(expect.any(Error), expect.any(String), expect.any(String));
 
-      expect(mockCaptureException).toHaveBeenCalledTimes(1);
-      expect(mockCaptureException).toHaveBeenCalledWith(expect.any(Error), {
-        contexts: { react: { componentStack: expect.any(String) } },
-      });
+      expect(mockCaptureEvent).toHaveBeenCalledTimes(1);
+
+      // We do a detailed assert on the stacktrace as a regression test against future
+      // react changes (that way we can update the docs if frames change in a major way).
+      const event = mockCaptureEvent.mock.calls[0][0];
+      expect(event.exception.values).toHaveLength(2);
+      expect(event.level).toBe(Severity.Error);
+
+      expect(event.exception.values[0].type).toEqual('React ErrorBoundary Error');
+      expect(event.exception.values[0].stacktrace.frames).toEqual([
+        {
+          colno: expect.any(Number),
+          filename: expect.stringContaining('errorboundary.test.tsx'),
+          function: 'TestApp',
+          in_app: true,
+          lineno: expect.any(Number),
+        },
+        {
+          colno: expect.any(Number),
+          filename: expect.stringContaining('errorboundary.tsx'),
+          function: 'ErrorBoundary',
+          in_app: true,
+          lineno: expect.any(Number),
+        },
+        {
+          colno: expect.any(Number),
+          filename: expect.stringContaining('errorboundary.test.tsx'),
+          function: 'Bam',
+          in_app: true,
+          lineno: expect.any(Number),
+        },
+        {
+          colno: expect.any(Number),
+          filename: expect.stringContaining('errorboundary.test.tsx'),
+          function: 'Boo',
+          in_app: true,
+          lineno: expect.any(Number),
+        },
+      ]);
     });
 
     it('calls `beforeCapture()` when an error occurs', () => {
       const mockBeforeCapture = jest.fn();
 
       const testBeforeCapture = (...args: any[]) => {
-        expect(mockCaptureException).toHaveBeenCalledTimes(0);
+        expect(mockCaptureEvent).toHaveBeenCalledTimes(0);
         mockBeforeCapture(...args);
       };
 
@@ -217,14 +264,14 @@ describe('ErrorBoundary', () => {
       );
 
       expect(mockBeforeCapture).toHaveBeenCalledTimes(0);
-      expect(mockCaptureException).toHaveBeenCalledTimes(0);
+      expect(mockCaptureEvent).toHaveBeenCalledTimes(0);
 
       const btn = screen.getByTestId('errorBtn');
       fireEvent.click(btn);
 
       expect(mockBeforeCapture).toHaveBeenCalledTimes(1);
       expect(mockBeforeCapture).toHaveBeenLastCalledWith(expect.any(Scope), expect.any(Error), expect.any(String));
-      expect(mockCaptureException).toHaveBeenCalledTimes(1);
+      expect(mockCaptureEvent).toHaveBeenCalledTimes(1);
     });
 
     it('shows a Sentry Report Dialog with correct options', () => {
diff --git a/yarn.lock b/yarn.lock
@@ -16583,15 +16583,14 @@ rc@^1.0.1, rc@^1.1.6:
     minimist "^1.2.0"
     strip-json-comments "~2.0.1"
 
-react-dom@^16.0.0:
-  version "16.14.0"
-  resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.14.0.tgz#7ad838ec29a777fb3c75c3a190f661cf92ab8b89"
-  integrity sha512-1gCeQXDLoIqMgqD3IO2Ah9bnf0w9kzhwN5q4FGnHZ67hBm9yePzB5JJAIQCc8x3pFnNlwFq4RidZggNAAkzWWw==
+react-dom@^17.0.0:
+  version "17.0.2"
+  resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-17.0.2.tgz#ecffb6845e3ad8dbfcdc498f0d0a939736502c23"
+  integrity sha512-s4h96KtLDUQlsENhMn1ar8t2bEa+q/YAtj8pPPdIjPDGBDIVNsrD9aXNWqspUe6AzKCIG0C1HZZLqLV7qpOBGA==
   dependencies:
     loose-envify "^1.1.0"
     object-assign "^4.1.1"
-    prop-types "^15.6.2"
-    scheduler "^0.19.1"
+    scheduler "^0.20.2"
 
 react-error-boundary@^3.1.0:
   version "3.1.1"
@@ -16667,14 +16666,13 @@ react-test-renderer@^16.13.1:
     react-is "^16.8.6"
     scheduler "^0.19.1"
 
-react@^16.0.0, react@^16.13.1:
-  version "16.14.0"
-  resolved "https://registry.yarnpkg.com/react/-/react-16.14.0.tgz#94d776ddd0aaa37da3eda8fc5b6b18a4c9a3114d"
-  integrity sha512-0X2CImDkJGApiAlcf0ODKIneSwBPhqJawOa5wCtKbu7ZECrmS26NvtSILynQ66cgkT/RJ4LidJOc3bUESwmU8g==
+react@^17.0.0:
+  version "17.0.2"
+  resolved "https://registry.yarnpkg.com/react/-/react-17.0.2.tgz#d0b5cc516d29eb3eee383f75b62864cfb6800037"
+  integrity sha512-gnhPt75i/dq/z3/6q/0asP78D0u592D5L1pd7M8P+dck6Fu/jJeL6iVVK23fptSUZj8Vjf++7wXA8UNclGQcbA==
   dependencies:
     loose-envify "^1.1.0"
     object-assign "^4.1.1"
-    prop-types "^15.6.2"
 
 read-cmd-shim@^1.0.1:
   version "1.0.5"
@@ -17583,6 +17581,14 @@ scheduler@^0.19.1:
     loose-envify "^1.1.0"
     object-assign "^4.1.1"
 
+scheduler@^0.20.2:
+  version "0.20.2"
+  resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.20.2.tgz#4baee39436e34aa93b4874bddcbf0fe8b8b50e91"
+  integrity sha512-2eWfGgAqqWFGqtdMmcL5zCMK1U8KlXv8SQFGglL3CEtd0aDVDWgeF/YoCmvln55m5zSk3J/20hTaSBeSObsQDQ==
+  dependencies:
+    loose-envify "^1.1.0"
+    object-assign "^4.1.1"
+
 schema-utils@^1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/schema-utils/-/schema-utils-1.0.0.tgz#0b79a93204d7b600d4b2850d1f66c2a34951c770"
