Skip to content

Commit 7500b06

Browse files
arturovtLms24
andauthored
fix(angular): Remove afterSendEvent listener once root injector is destroyed (#12786)
Add cleanup logic to handle the removal of afterSendEvent, which is set up within the Angular error handler. This fixes memory leaks that occur when the event is still being handled after the root view is removed --------- Co-authored-by: Lukas Stracke <[email protected]>
1 parent 803220e commit 7500b06

File tree

2 files changed

+60
-12
lines changed

2 files changed

+60
-12
lines changed

packages/angular/src/errorhandler.ts

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { HttpErrorResponse } from '@angular/common/http';
2-
import type { ErrorHandler as AngularErrorHandler } from '@angular/core';
2+
import type { ErrorHandler as AngularErrorHandler, OnDestroy } from '@angular/core';
33
import { Inject, Injectable } from '@angular/core';
44
import * as Sentry from '@sentry/browser';
55
import type { ReportDialogOptions } from '@sentry/browser';
@@ -81,21 +81,28 @@ function isErrorOrErrorLikeObject(value: unknown): value is Error {
8181
* Implementation of Angular's ErrorHandler provider that can be used as a drop-in replacement for the stock one.
8282
*/
8383
@Injectable({ providedIn: 'root' })
84-
class SentryErrorHandler implements AngularErrorHandler {
84+
class SentryErrorHandler implements AngularErrorHandler, OnDestroy {
8585
protected readonly _options: ErrorHandlerOptions;
8686

87-
/* indicates if we already registered our the afterSendEvent handler */
88-
private _registeredAfterSendEventHandler;
87+
/** The cleanup function is executed when the injector is destroyed. */
88+
private _removeAfterSendEventListener?: () => void;
8989

9090
public constructor(@Inject('errorHandlerOptions') options?: ErrorHandlerOptions) {
91-
this._registeredAfterSendEventHandler = false;
92-
9391
this._options = {
9492
logErrors: true,
9593
...options,
9694
};
9795
}
9896

97+
/**
98+
* Method executed when the injector is destroyed.
99+
*/
100+
public ngOnDestroy(): void {
101+
if (this._removeAfterSendEventListener) {
102+
this._removeAfterSendEventListener();
103+
}
104+
}
105+
99106
/**
100107
* Method called for every value captured through the ErrorHandler
101108
*/
@@ -119,17 +126,14 @@ class SentryErrorHandler implements AngularErrorHandler {
119126
if (this._options.showDialog) {
120127
const client = Sentry.getClient();
121128

122-
if (client && !this._registeredAfterSendEventHandler) {
123-
client.on('afterSendEvent', (event: Event) => {
129+
if (client && !this._removeAfterSendEventListener) {
130+
this._removeAfterSendEventListener = client.on('afterSendEvent', (event: Event) => {
124131
if (!event.type && event.event_id) {
125132
runOutsideAngular(() => {
126-
Sentry.showReportDialog({ ...this._options.dialogOptions, eventId: event.event_id! });
133+
Sentry.showReportDialog({ ...this._options.dialogOptions, eventId: event.event_id });
127134
});
128135
}
129136
});
130-
131-
// We only want to register this hook once in the lifetime of the error handler
132-
this._registeredAfterSendEventHandler = true;
133137
} else if (!client) {
134138
runOutsideAngular(() => {
135139
Sentry.showReportDialog({ ...this._options.dialogOptions, eventId });

packages/angular/test/errorhandler.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,5 +546,49 @@ describe('SentryErrorHandler', () => {
546546
expect(showReportDialogSpy).toBeCalledTimes(1);
547547
});
548548
});
549+
550+
it('only registers the client "afterSendEvent" listener to open the dialog once', () => {
551+
const unsubScribeSpy = vi.fn();
552+
const client = {
553+
cbs: [] as ((event: Event) => void)[],
554+
on: vi.fn((_, cb) => {
555+
client.cbs.push(cb);
556+
return unsubScribeSpy;
557+
}),
558+
};
559+
560+
vi.spyOn(SentryBrowser, 'getClient').mockImplementation(() => client as unknown as Client);
561+
562+
const errorhandler = createErrorHandler({ showDialog: true });
563+
expect(client.cbs).toHaveLength(0);
564+
565+
errorhandler.handleError(new Error('error 1'));
566+
expect(client.cbs).toHaveLength(1);
567+
568+
errorhandler.handleError(new Error('error 2'));
569+
errorhandler.handleError(new Error('error 3'));
570+
expect(client.cbs).toHaveLength(1);
571+
});
572+
573+
it('cleans up the "afterSendEvent" listener once the ErrorHandler is destroyed', () => {
574+
const unsubScribeSpy = vi.fn();
575+
const client = {
576+
cbs: [] as ((event: Event) => void)[],
577+
on: vi.fn((_, cb) => {
578+
client.cbs.push(cb);
579+
return unsubScribeSpy;
580+
}),
581+
};
582+
583+
vi.spyOn(SentryBrowser, 'getClient').mockImplementation(() => client as unknown as Client);
584+
585+
const errorhandler = createErrorHandler({ showDialog: true });
586+
587+
errorhandler.handleError(new Error('error 1'));
588+
expect(client.cbs).toHaveLength(1);
589+
590+
errorhandler.ngOnDestroy();
591+
expect(unsubScribeSpy).toHaveBeenCalledTimes(1);
592+
});
549593
});
550594
});

0 commit comments

Comments
 (0)