Skip to content

Commit

Permalink
fix(angular): Remove afterSendEvent listener once root injector is …
Browse files Browse the repository at this point in the history
…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 <lukas.stracke@sentry.io>
  • Loading branch information
arturovt and Lms24 authored Jul 18, 2024
1 parent 803220e commit 7500b06
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 12 deletions.
28 changes: 16 additions & 12 deletions packages/angular/src/errorhandler.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { HttpErrorResponse } from '@angular/common/http';
import type { ErrorHandler as AngularErrorHandler } from '@angular/core';
import type { ErrorHandler as AngularErrorHandler, OnDestroy } from '@angular/core';
import { Inject, Injectable } from '@angular/core';
import * as Sentry from '@sentry/browser';
import type { ReportDialogOptions } from '@sentry/browser';
Expand Down Expand Up @@ -81,21 +81,28 @@ function isErrorOrErrorLikeObject(value: unknown): value is Error {
* Implementation of Angular's ErrorHandler provider that can be used as a drop-in replacement for the stock one.
*/
@Injectable({ providedIn: 'root' })
class SentryErrorHandler implements AngularErrorHandler {
class SentryErrorHandler implements AngularErrorHandler, OnDestroy {
protected readonly _options: ErrorHandlerOptions;

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

public constructor(@Inject('errorHandlerOptions') options?: ErrorHandlerOptions) {
this._registeredAfterSendEventHandler = false;

this._options = {
logErrors: true,
...options,
};
}

/**
* Method executed when the injector is destroyed.
*/
public ngOnDestroy(): void {
if (this._removeAfterSendEventListener) {
this._removeAfterSendEventListener();
}
}

/**
* Method called for every value captured through the ErrorHandler
*/
Expand All @@ -119,17 +126,14 @@ class SentryErrorHandler implements AngularErrorHandler {
if (this._options.showDialog) {
const client = Sentry.getClient();

if (client && !this._registeredAfterSendEventHandler) {
client.on('afterSendEvent', (event: Event) => {
if (client && !this._removeAfterSendEventListener) {
this._removeAfterSendEventListener = client.on('afterSendEvent', (event: Event) => {
if (!event.type && event.event_id) {
runOutsideAngular(() => {
Sentry.showReportDialog({ ...this._options.dialogOptions, eventId: event.event_id! });
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) {
runOutsideAngular(() => {
Sentry.showReportDialog({ ...this._options.dialogOptions, eventId });
Expand Down
44 changes: 44 additions & 0 deletions packages/angular/test/errorhandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -546,5 +546,49 @@ describe('SentryErrorHandler', () => {
expect(showReportDialogSpy).toBeCalledTimes(1);
});
});

it('only registers the client "afterSendEvent" listener to open the dialog once', () => {
const unsubScribeSpy = vi.fn();
const client = {
cbs: [] as ((event: Event) => void)[],
on: vi.fn((_, cb) => {
client.cbs.push(cb);
return unsubScribeSpy;
}),
};

vi.spyOn(SentryBrowser, 'getClient').mockImplementation(() => client as unknown as Client);

const errorhandler = createErrorHandler({ showDialog: true });
expect(client.cbs).toHaveLength(0);

errorhandler.handleError(new Error('error 1'));
expect(client.cbs).toHaveLength(1);

errorhandler.handleError(new Error('error 2'));
errorhandler.handleError(new Error('error 3'));
expect(client.cbs).toHaveLength(1);
});

it('cleans up the "afterSendEvent" listener once the ErrorHandler is destroyed', () => {
const unsubScribeSpy = vi.fn();
const client = {
cbs: [] as ((event: Event) => void)[],
on: vi.fn((_, cb) => {
client.cbs.push(cb);
return unsubScribeSpy;
}),
};

vi.spyOn(SentryBrowser, 'getClient').mockImplementation(() => client as unknown as Client);

const errorhandler = createErrorHandler({ showDialog: true });

errorhandler.handleError(new Error('error 1'));
expect(client.cbs).toHaveLength(1);

errorhandler.ngOnDestroy();
expect(unsubScribeSpy).toHaveBeenCalledTimes(1);
});
});
});

0 comments on commit 7500b06

Please sign in to comment.