Skip to content

Commit 2531853

Browse files
authored
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.
1 parent 105dcf1 commit 2531853

File tree

4 files changed

+56
-12
lines changed

4 files changed

+56
-12
lines changed

packages/angular-ivy/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757
"lint": "run-s lint:prettier lint:eslint",
5858
"lint:eslint": "eslint . --format stylish",
5959
"lint:prettier": "prettier --check \"{src,test,scripts}/**/**.ts\"",
60-
"yalc:publish": "ts-node ../../scripts/prepack.ts && yalc publish build --push"
60+
"yalc:publish": "yalc publish build --push"
6161
},
6262
"volta": {
6363
"extends": "../../package.json"

packages/angular/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@
6060
"test": "yarn test:unit",
6161
"test:unit": "jest",
6262
"test:unit:watch": "jest --watch",
63-
"yalc:publish": "ts-node ../../scripts/prepack.ts && yalc publish build --push"
63+
"yalc:publish": "yalc publish build --push"
6464
},
6565
"volta": {
6666
"extends": "../../package.json"

packages/angular/src/errorhandler.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ function isErrorOrErrorLikeObject(value: unknown): value is Error {
8383
class SentryErrorHandler implements AngularErrorHandler {
8484
protected readonly _options: ErrorHandlerOptions;
8585

86+
/* indicates if we already registered our the afterSendEvent handler */
87+
private _registeredAfterSendEventHandler = false;
88+
8689
public constructor(@Inject('errorHandlerOptions') options?: ErrorHandlerOptions) {
8790
this._options = {
8891
logErrors: true,
@@ -120,7 +123,20 @@ class SentryErrorHandler implements AngularErrorHandler {
120123

121124
// Optionally show user dialog to provide details on what happened.
122125
if (this._options.showDialog) {
123-
Sentry.showReportDialog({ ...this._options.dialogOptions, eventId });
126+
const client = Sentry.getCurrentHub().getClient();
127+
128+
if (client && client.on && !this._registeredAfterSendEventHandler) {
129+
client.on('afterSendEvent', event => {
130+
if (!event.type) {
131+
Sentry.showReportDialog({ ...this._options.dialogOptions, eventId: event.event_id });
132+
}
133+
});
134+
135+
// We only want to register this hook once in the lifetime of the error handler
136+
this._registeredAfterSendEventHandler = true;
137+
} else if (!client || !client.on) {
138+
Sentry.showReportDialog({ ...this._options.dialogOptions, eventId });
139+
}
124140
}
125141
}
126142

packages/angular/test/errorhandler.test.ts

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { HttpErrorResponse } from '@angular/common/http';
22
import * as SentryBrowser from '@sentry/browser';
33
import { Scope } from '@sentry/browser';
4+
import type { Event } from '@sentry/types';
45
import * as SentryUtils from '@sentry/utils';
56

67
import { createErrorHandler, SentryErrorHandler } from '../src/errorhandler';
@@ -507,15 +508,6 @@ describe('SentryErrorHandler', () => {
507508
expect(captureExceptionSpy).toHaveBeenCalledWith('something happened', expect.any(Function));
508509
});
509510

510-
it('handleError method shows report dialog', () => {
511-
const showReportDialogSpy = jest.spyOn(SentryBrowser, 'showReportDialog');
512-
513-
const errorHandler = createErrorHandler({ showDialog: true });
514-
errorHandler.handleError(new Error('test'));
515-
516-
expect(showReportDialogSpy).toBeCalledTimes(1);
517-
});
518-
519511
it('extracts error with a custom extractor', () => {
520512
const customExtractor = (error: unknown) => {
521513
if (typeof error === 'string') {
@@ -530,5 +522,41 @@ describe('SentryErrorHandler', () => {
530522
expect(captureExceptionSpy).toHaveBeenCalledTimes(1);
531523
expect(captureExceptionSpy).toHaveBeenCalledWith(new Error('custom error'), expect.any(Function));
532524
});
525+
526+
describe('opens the report dialog if `showDialog` is true', () => {
527+
it('by using SDK lifecycle hooks if available', () => {
528+
const client = {
529+
cb: (_: Event) => {},
530+
on: jest.fn((_, cb) => {
531+
client.cb = cb;
532+
}),
533+
};
534+
535+
// @ts-ignore this is a minmal hub, we're missing a few props but that's ok
536+
jest.spyOn(SentryBrowser, 'getCurrentHub').mockImplementationOnce(() => {
537+
return { getClient: () => client };
538+
});
539+
540+
const showReportDialogSpy = jest.spyOn(SentryBrowser, 'showReportDialog');
541+
542+
const errorHandler = createErrorHandler({ showDialog: true });
543+
errorHandler.handleError(new Error('test'));
544+
expect(client.on).toHaveBeenCalledWith('afterSendEvent', expect.any(Function));
545+
546+
// this simulates the afterSend hook being called
547+
client.cb({});
548+
549+
expect(showReportDialogSpy).toBeCalledTimes(1);
550+
});
551+
552+
it('by just calling `showReportDialog` if hooks are not available', () => {
553+
const showReportDialogSpy = jest.spyOn(SentryBrowser, 'showReportDialog');
554+
555+
const errorHandler = createErrorHandler({ showDialog: true });
556+
errorHandler.handleError(new Error('test'));
557+
558+
expect(showReportDialogSpy).toBeCalledTimes(1);
559+
});
560+
});
533561
});
534562
});

0 commit comments

Comments
 (0)