Skip to content

Commit

Permalink
feat(core): allow unregistering callback through on
Browse files Browse the repository at this point in the history 
This commit updates the return signature of the client's `on` function to be a void function,
which, when executed, unregisters a callback. This adjustment is necessary for managing instances
where objects are created and destroyed, ensuring that callbacks are properly unregistered to
prevent self-referencing in callback closures and facilitate proper garbage collection. Consider,
for instance, a scenario in Angular where the error handler is created and destroyed with each app
rendering cycle. In such a case, setting up the `afterSendEvent` event listener each time the error
handler is created would result in accumulation without removing the previous ones.

Typically, changing a type from `void` to `() => void` (or `VoidFunction`) shouldn't be considered
a breaking change because `void` signifies the absence of a return value, implying that the return
value of the `on` function should never be used by consumers.
  • Loading branch information
@arturovt
arturovt committed Apr 22, 2024
1 parent 7eb000c commit 74cb2ec
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 34 deletions.
53 changes: 33 additions & 20 deletions packages/core/src/baseclient.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,37 +384,37 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
/* eslint-disable @typescript-eslint/unified-signatures */

/** @inheritdoc */
public on(hook: 'spanStart', callback: (span: Span) => void): void;
public on(hook: 'spanStart', callback: (span: Span) => void): () => void;

/** @inheritdoc */
public on(hook: 'spanEnd', callback: (span: Span) => void): void;
public on(hook: 'spanEnd', callback: (span: Span) => void): () => void;

/** @inheritdoc */
public on(hook: 'idleSpanEnableAutoFinish', callback: (span: Span) => void): void;
public on(hook: 'idleSpanEnableAutoFinish', callback: (span: Span) => void): () => void;

/** @inheritdoc */
public on(hook: 'beforeEnvelope', callback: (envelope: Envelope) => void): void;
public on(hook: 'beforeEnvelope', callback: (envelope: Envelope) => void): () => void;

/** @inheritdoc */
public on(hook: 'beforeSendEvent', callback: (event: Event, hint?: EventHint) => void): void;
public on(hook: 'beforeSendEvent', callback: (event: Event, hint?: EventHint) => void): () => void;

/** @inheritdoc */
public on(hook: 'preprocessEvent', callback: (event: Event, hint?: EventHint) => void): void;
public on(hook: 'preprocessEvent', callback: (event: Event, hint?: EventHint) => void): () => void;

/** @inheritdoc */
public on(hook: 'afterSendEvent', callback: (event: Event, sendResponse: TransportMakeRequestResponse) => void): void;
public on(hook: 'afterSendEvent', callback: (event: Event, sendResponse: TransportMakeRequestResponse) => void): () => void;

/** @inheritdoc */
public on(hook: 'beforeAddBreadcrumb', callback: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => void): void;
public on(hook: 'beforeAddBreadcrumb', callback: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => void): () => void;

/** @inheritdoc */
public on(hook: 'createDsc', callback: (dsc: DynamicSamplingContext) => void): void;
public on(hook: 'createDsc', callback: (dsc: DynamicSamplingContext) => void): () => void;

/** @inheritdoc */
public on(
hook: 'beforeSendFeedback',
callback: (feedback: FeedbackEvent, options?: { includeReplay: boolean }) => void,
): void;
): () => void;

/** @inheritdoc */
public on(
Expand All @@ -423,23 +423,36 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
options: StartSpanOptions,
traceOptions?: { sentryTrace?: string | undefined; baggage?: string | undefined },
) => void,
): void;
): () => void;

/** @inheritdoc */
public on(hook: 'startNavigationSpan', callback: (options: StartSpanOptions) => void): void;
public on(hook: 'startNavigationSpan', callback: (options: StartSpanOptions) => void): () => void;

public on(hook: 'flush', callback: () => void): void;
public on(hook: 'flush', callback: () => void): () => void;

public on(hook: 'close', callback: () => void): void;
public on(hook: 'close', callback: () => void): () => void;

/** @inheritdoc */
public on(hook: string, callback: unknown): void {
if (!this._hooks[hook]) {
this._hooks[hook] = [];
}

public on(hook: string, callback: unknown): () => void {
// @ts-expect-error We assue the types are correct
this._hooks[hook].push(callback);
(this._hooks[hook] ??= []).push(callback);

// This function returns a callback execution handler that, when invoked,
// deregisters a callback. This is crucial for managing instances where callbacks
// need to be unregistered to prevent self-referencing in callback closures,
// ensuring proper garbage collection. For instance, consider a scenario in Angular
// where an error handler is created and destroyed with each app rendering cycle.
// In this case, setting up the `afterSendEvent` event listener each time the error
// handler is created would result in accumulation without removing the previous ones.
return () => {
const hooks = this._hooks[hook];

if (hooks) {
// @ts-expect-error We assue the types are correct
const cbIndex = hooks.indexOf(callback);
hooks.splice(cbIndex, 1);
}
};
}

/** @inheritdoc */
Expand Down
28 changes: 14 additions & 14 deletions packages/types/src/client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,52 +176,52 @@ export interface Client<O extends ClientOptions = ClientOptions> {
* Register a callback for whenever a span is started.
* Receives the span as argument.
*/
on(hook: 'spanStart', callback: (span: Span) => void): void;
on(hook: 'spanStart', callback: (span: Span) => void): () => void;

/**
* Register a callback for whenever a span is ended.
* Receives the span as argument.
*/
on(hook: 'spanEnd', callback: (span: Span) => void): void;
on(hook: 'spanEnd', callback: (span: Span) => void): () => void;

/**
* Register a callback for when an idle span is allowed to auto-finish.
*/
on(hook: 'idleSpanEnableAutoFinish', callback: (span: Span) => void): void;
on(hook: 'idleSpanEnableAutoFinish', callback: (span: Span) => void): () => void;

/**
* Register a callback for transaction start and finish.
*/
on(hook: 'beforeEnvelope', callback: (envelope: Envelope) => void): void;
on(hook: 'beforeEnvelope', callback: (envelope: Envelope) => void): () => void;

/**
* Register a callback for before sending an event.
* This is called right before an event is sent and should not be used to mutate the event.
* Receives an Event & EventHint as arguments.
*/
on(hook: 'beforeSendEvent', callback: (event: Event, hint?: EventHint | undefined) => void): void;
on(hook: 'beforeSendEvent', callback: (event: Event, hint?: EventHint | undefined) => void): () => void;

/**
* Register a callback for preprocessing an event,
* before it is passed to (global) event processors.
* Receives an Event & EventHint as arguments.
*/
on(hook: 'preprocessEvent', callback: (event: Event, hint?: EventHint | undefined) => void): void;
on(hook: 'preprocessEvent', callback: (event: Event, hint?: EventHint | undefined) => void): () => void;

/**
* Register a callback for when an event has been sent.
*/
on(hook: 'afterSendEvent', callback: (event: Event, sendResponse: TransportMakeRequestResponse) => void): void;
on(hook: 'afterSendEvent', callback: (event: Event, sendResponse: TransportMakeRequestResponse) => void): () => void;

/**
* Register a callback before a breadcrumb is added.
*/
on(hook: 'beforeAddBreadcrumb', callback: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => void): void;
on(hook: 'beforeAddBreadcrumb', callback: (breadcrumb: Breadcrumb, hint?: BreadcrumbHint) => void): () => void;

/**
* Register a callback when a DSC (Dynamic Sampling Context) is created.
*/
on(hook: 'createDsc', callback: (dsc: DynamicSamplingContext) => void): void;
on(hook: 'createDsc', callback: (dsc: DynamicSamplingContext) => void): () => void;

/**
* Register a callback when a Feedback event has been prepared.
Expand All @@ -231,7 +231,7 @@ export interface Client<O extends ClientOptions = ClientOptions> {
on(
hook: 'beforeSendFeedback',
callback: (feedback: FeedbackEvent, options?: { includeReplay?: boolean }) => void,
): void;
): () => void;

/**
* A hook for the browser tracing integrations to trigger a span start for a page load.
Expand All @@ -242,22 +242,22 @@ export interface Client<O extends ClientOptions = ClientOptions> {
options: StartSpanOptions,
traceOptions?: { sentryTrace?: string | undefined; baggage?: string | undefined },
) => void,
): void;
): () => void;

/**
* A hook for browser tracing integrations to trigger a span for a navigation.
*/
on(hook: 'startNavigationSpan', callback: (options: StartSpanOptions) => void): void;
on(hook: 'startNavigationSpan', callback: (options: StartSpanOptions) => void): () => void;

/**
* A hook that is called when the client is flushing
*/
on(hook: 'flush', callback: () => void): void;
on(hook: 'flush', callback: () => void): () => void;

/**
* A hook that is called when the client is closing
*/
on(hook: 'close', callback: () => void): void;
on(hook: 'close', callback: () => void): () => void;

/** Fire a hook whener a span starts. */
emit(hook: 'spanStart', span: Span): void;
Expand Down

0 comments on commit 74cb2ec

Please sign in to comment.