fix(angular): Only open report dialog if error was sent (#7750)

Fix the report dialog being opened for events that are dropped. Leverage SDK lifecycle hooks (`afterSendEvent`) to open the dialog if hooks are available. Keep the current behaviour as a fallback if hooks are not available.

Author: Lms24

packages/angular-ivy/package.json

@@ -57,7 +57,7 @@
     "lint": "run-s lint:prettier lint:eslint",
     "lint:eslint": "eslint . --format stylish",
     "lint:prettier": "prettier --check \"{src,test,scripts}/**/**.ts\"",
-    "yalc:publish": "ts-node ../../scripts/prepack.ts && yalc publish build --push"
+    "yalc:publish": "yalc publish build --push"
   },
   "volta": {
     "extends": "../../package.json"

packages/angular/package.json

@@ -60,7 +60,7 @@
     "test": "yarn test:unit",
     "test:unit": "jest",
     "test:unit:watch": "jest --watch",
-    "yalc:publish": "ts-node ../../scripts/prepack.ts && yalc publish build --push"
+    "yalc:publish": "yalc publish build --push"
   },
   "volta": {
     "extends": "../../package.json"

packages/angular/src/errorhandler.ts

@@ -83,6 +83,9 @@ function isErrorOrErrorLikeObject(value: unknown): value is Error {
 class SentryErrorHandler implements AngularErrorHandler {
   protected readonly _options: ErrorHandlerOptions;
 
+  /* indicates if we already registered our the afterSendEvent handler */
+  private _registeredAfterSendEventHandler = false;
+
   public constructor(@Inject('errorHandlerOptions') options?: ErrorHandlerOptions) {
     this._options = {
       logErrors: true,
@@ -120,7 +123,20 @@ class SentryErrorHandler implements AngularErrorHandler {
 
     // Optionally show user dialog to provide details on what happened.
     if (this._options.showDialog) {
-      Sentry.showReportDialog({ ...this._options.dialogOptions, eventId });
+      const client = Sentry.getCurrentHub().getClient();
+
+      if (client && client.on && !this._registeredAfterSendEventHandler) {
+        client.on('afterSendEvent', event => {
+          if (!event.type) {
+            Sentry.showReportDialog({ ...this._options.dialogOptions, eventId: event.event_id });
+          }
+        });
+
+        // We only want to register this hook once in the lifetime of the error handler
+        this._registeredAfterSendEventHandler = true;
+      } else if (!client || !client.on) {
+        Sentry.showReportDialog({ ...this._options.dialogOptions, eventId });
+      }
     }
   }
 

packages/angular/test/errorhandler.test.ts

@@ -1,6 +1,7 @@
 import { HttpErrorResponse } from '@angular/common/http';
 import * as SentryBrowser from '@sentry/browser';
 import { Scope } from '@sentry/browser';
+import type { Event } from '@sentry/types';
 import * as SentryUtils from '@sentry/utils';
 
 import { createErrorHandler, SentryErrorHandler } from '../src/errorhandler';
@@ -507,15 +508,6 @@ describe('SentryErrorHandler', () => {
       expect(captureExceptionSpy).toHaveBeenCalledWith('something happened', expect.any(Function));
     });
 
-    it('handleError method shows report dialog', () => {
-      const showReportDialogSpy = jest.spyOn(SentryBrowser, 'showReportDialog');
-
-      const errorHandler = createErrorHandler({ showDialog: true });
-      errorHandler.handleError(new Error('test'));
-
-      expect(showReportDialogSpy).toBeCalledTimes(1);
-    });
-
     it('extracts error with a custom extractor', () => {
       const customExtractor = (error: unknown) => {
         if (typeof error === 'string') {
@@ -530,5 +522,41 @@ describe('SentryErrorHandler', () => {
       expect(captureExceptionSpy).toHaveBeenCalledTimes(1);
       expect(captureExceptionSpy).toHaveBeenCalledWith(new Error('custom error'), expect.any(Function));
     });
+
+    describe('opens the report dialog if `showDialog` is true', () => {
+      it('by using SDK lifecycle hooks if available', () => {
+        const client = {
+          cb: (_: Event) => {},
+          on: jest.fn((_, cb) => {
+            client.cb = cb;
+          }),
+        };
+
+        // @ts-ignore this is a minmal hub, we're missing a few props but that's ok
+        jest.spyOn(SentryBrowser, 'getCurrentHub').mockImplementationOnce(() => {
+          return { getClient: () => client };
+        });
+
+        const showReportDialogSpy = jest.spyOn(SentryBrowser, 'showReportDialog');
+
+        const errorHandler = createErrorHandler({ showDialog: true });
+        errorHandler.handleError(new Error('test'));
+        expect(client.on).toHaveBeenCalledWith('afterSendEvent', expect.any(Function));
+
+        // this simulates the afterSend hook being called
+        client.cb({});
+
+        expect(showReportDialogSpy).toBeCalledTimes(1);
+      });
+
+      it('by just calling `showReportDialog` if hooks are not available', () => {
+        const showReportDialogSpy = jest.spyOn(SentryBrowser, 'showReportDialog');
+
+        const errorHandler = createErrorHandler({ showDialog: true });
+        errorHandler.handleError(new Error('test'));
+
+        expect(showReportDialogSpy).toBeCalledTimes(1);
+      });
+    });
   });
 });