fix(react): Only show report dialog if event was sent to Sentry (#7754)

getsentry · Apr 5, 2023 · 367f779 · 367f779
1 parent 2531853
commit 367f779
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 5 deletions.
diff --git a/packages/react/src/errorboundary.tsx b/packages/react/src/errorboundary.tsx
@@ -1,5 +1,5 @@
 import type { ReportDialogOptions, Scope } from '@sentry/browser';
-import { captureException, showReportDialog, withScope } from '@sentry/browser';
+import { captureException, getCurrentHub, showReportDialog, withScope } from '@sentry/browser';
 import { isError, logger } from '@sentry/utils';
 import hoistNonReactStatics from 'hoist-non-react-statics';
 import * as React from 'react';
@@ -94,9 +94,26 @@ function setCause(error: Error & { cause?: Error }, cause: Error): void {
 class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundaryState> {
   public state: ErrorBoundaryState = INITIAL_STATE;
 
+  private readonly _openFallbackReportDialog: boolean = true;
+
+  private _lastEventId?: string;
+
+  public constructor(props: ErrorBoundaryProps) {
+    super(props);
+
+    const client = getCurrentHub().getClient();
+    if (client && client.on && props.showDialog) {
+      this._openFallbackReportDialog = false;
+      client.on('afterSendEvent', event => {
+        if (!event.type && event.event_id === this._lastEventId) {
+          showReportDialog({ ...props.dialogOptions, eventId: this._lastEventId });
+        }
+      });
+    }
+  }
+
   public componentDidCatch(error: Error & { cause?: Error }, { componentStack }: React.ErrorInfo): void {
     const { beforeCapture, onError, showDialog, dialogOptions } = this.props;
-
     withScope(scope => {
       // 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.
@@ -123,7 +140,10 @@ class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundarySta
         onError(error, componentStack, eventId);
       }
       if (showDialog) {
-        showReportDialog({ ...dialogOptions, eventId });
+        this._lastEventId = eventId;
+        if (this._openFallbackReportDialog) {
+          showReportDialog({ ...dialogOptions, eventId });
+        }
       }
 
       // componentDidCatch is used over getDerivedStateFromError

diff --git a/packages/react/test/errorboundary.test.tsx b/packages/react/test/errorboundary.test.tsx
@@ -1,4 +1,4 @@
-import { Scope } from '@sentry/browser';
+import { getCurrentHub, Scope } from '@sentry/browser';
 import { fireEvent, render, screen } from '@testing-library/react';
 import * as React from 'react';
 import { useState } from 'react';
@@ -8,6 +8,7 @@ import { ErrorBoundary, isAtLeastReact17, UNKNOWN_COMPONENT, withErrorBoundary }
 
 const mockCaptureException = jest.fn();
 const mockShowReportDialog = jest.fn();
+const mockClientOn = jest.fn();
 const EVENT_ID = 'test-id-123';
 
 jest.mock('@sentry/browser', () => {
@@ -82,6 +83,7 @@ describe('ErrorBoundary', () => {
   afterEach(() => {
     mockCaptureException.mockClear();
     mockShowReportDialog.mockClear();
+    mockClientOn.mockClear();
   });
 
   it('renders null if not given a valid `fallback` prop', () => {
@@ -405,7 +407,34 @@ describe('ErrorBoundary', () => {
       expect(mockCaptureException).toHaveBeenCalledTimes(1);
     });
 
-    it('shows a Sentry Report Dialog with correct options', () => {
+    it('shows a Sentry Report Dialog with correct options if client does not have hooks', () => {
+      expect(getCurrentHub().getClient()).toBeUndefined();
+
+      const options = { title: 'custom title' };
+      render(
+        <TestApp fallback={<p>You have hit an error</p>} showDialog dialogOptions={options}>
+          <h1>children</h1>
+        </TestApp>,
+      );
+
+      expect(mockShowReportDialog).toHaveBeenCalledTimes(0);
+
+      const btn = screen.getByTestId('errorBtn');
+      fireEvent.click(btn);
+
+      expect(mockShowReportDialog).toHaveBeenCalledTimes(1);
+      expect(mockShowReportDialog).toHaveBeenCalledWith({ ...options, eventId: EVENT_ID });
+    });
+
+    it('shows a Sentry Report Dialog with correct options if client has hooks', () => {
+      let callback: any;
+      const hub = getCurrentHub();
+      // @ts-ignore mock client
+      hub.bindClient({
+        on: (name: string, cb: any) => {
+          callback = cb;
+        },
+      });
       const options = { title: 'custom title' };
       render(
         <TestApp fallback={<p>You have hit an error</p>} showDialog dialogOptions={options}>
@@ -418,8 +447,13 @@ describe('ErrorBoundary', () => {
       const btn = screen.getByTestId('errorBtn');
       fireEvent.click(btn);
 
+      // Simulate hook being fired
+      callback({ event_id: EVENT_ID });
+
       expect(mockShowReportDialog).toHaveBeenCalledTimes(1);
       expect(mockShowReportDialog).toHaveBeenCalledWith({ ...options, eventId: EVENT_ID });
+
+      hub.bindClient(undefined);
     });
 
     it('resets to initial state when reset', async () => {