Skip to content

Commit 2e949da

Browse files
committed
fix: Cleanup hooks when they are not used anymore
Small optimization using the new hook cleanup capabilities to remove unused hooks.
1 parent 3ad3bdd commit 2e949da

File tree

3 files changed

+47
-30
lines changed

3 files changed

+47
-30
lines changed

packages/core/src/tracing/idleSpan.ts

Lines changed: 38 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,8 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
9696

9797
let _autoFinishAllowed: boolean = !options.disableAutoFinish;
9898

99+
const _cleanupHooks: (() => void)[] = [];
100+
99101
const {
100102
idleTimeout = TRACING_DEFAULTS.idleTimeout,
101103
finalTimeout = TRACING_DEFAULTS.finalTimeout,
@@ -237,6 +239,8 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
237239
}
238240

239241
function onIdleSpanEnded(endTimestamp: number): void {
242+
_cleanupHooks.forEach(cleanup => cleanup());
243+
240244
_finished = true;
241245
activities.clear();
242246

@@ -298,41 +302,47 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
298302
}
299303
}
300304

301-
client.on('spanStart', startedSpan => {
302-
// If we already finished the idle span,
303-
// or if this is the idle span itself being started,
304-
// or if the started span has already been closed,
305-
// we don't care about it for activity
306-
if (_finished || startedSpan === span || !!spanToJSON(startedSpan).timestamp) {
307-
return;
308-
}
305+
_cleanupHooks.push(
306+
client.on('spanStart', startedSpan => {
307+
// If we already finished the idle span,
308+
// or if this is the idle span itself being started,
309+
// or if the started span has already been closed,
310+
// we don't care about it for activity
311+
if (_finished || startedSpan === span || !!spanToJSON(startedSpan).timestamp) {
312+
return;
313+
}
309314

310-
const allSpans = getSpanDescendants(span);
315+
const allSpans = getSpanDescendants(span);
311316

312-
// If the span that was just started is a child of the idle span, we should track it
313-
if (allSpans.includes(startedSpan)) {
314-
_pushActivity(startedSpan.spanContext().spanId);
315-
}
316-
});
317+
// If the span that was just started is a child of the idle span, we should track it
318+
if (allSpans.includes(startedSpan)) {
319+
_pushActivity(startedSpan.spanContext().spanId);
320+
}
321+
}),
322+
);
317323

318-
client.on('spanEnd', endedSpan => {
319-
if (_finished) {
320-
return;
321-
}
324+
_cleanupHooks.push(
325+
client.on('spanEnd', endedSpan => {
326+
if (_finished) {
327+
return;
328+
}
322329

323-
_popActivity(endedSpan.spanContext().spanId);
324-
});
330+
_popActivity(endedSpan.spanContext().spanId);
331+
}),
332+
);
325333

326-
client.on('idleSpanEnableAutoFinish', spanToAllowAutoFinish => {
327-
if (spanToAllowAutoFinish === span) {
328-
_autoFinishAllowed = true;
329-
_restartIdleTimeout();
334+
_cleanupHooks.push(
335+
client.on('idleSpanEnableAutoFinish', spanToAllowAutoFinish => {
336+
if (spanToAllowAutoFinish === span) {
337+
_autoFinishAllowed = true;
338+
_restartIdleTimeout();
330339

331-
if (activities.size) {
332-
_restartChildSpanTimeout();
340+
if (activities.size) {
341+
_restartChildSpanTimeout();
342+
}
333343
}
334-
}
335-
});
344+
}),
345+
);
336346

337347
// We only start the initial idle timeout if we are not delaying the auto finish
338348
if (!options.disableAutoFinish) {

packages/feedback/src/core/sendFeedback.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,13 @@ export const sendFeedback: SendFeedback = (
4040
// After 5s, we want to clear anyhow
4141
const timeout = setTimeout(() => reject('Unable to determine if Feedback was correctly sent.'), 5_000);
4242

43-
client.on('afterSendEvent', (event: Event, response: TransportMakeRequestResponse) => {
43+
const cleanup = client.on('afterSendEvent', (event: Event, response: TransportMakeRequestResponse) => {
4444
if (event.event_id !== eventId) {
4545
return;
4646
}
4747

4848
clearTimeout(timeout);
49+
cleanup();
4950

5051
// Require valid status codes, otherwise can assume feedback was not sent successfully
5152
if (

packages/react/src/errorboundary.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundarySta
7777
private readonly _openFallbackReportDialog: boolean;
7878

7979
private _lastEventId?: string;
80+
private _cleanupHook?: () => void;
8081

8182
public constructor(props: ErrorBoundaryProps) {
8283
super(props);
@@ -87,7 +88,7 @@ class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundarySta
8788
const client = getClient();
8889
if (client && props.showDialog) {
8990
this._openFallbackReportDialog = false;
90-
client.on('afterSendEvent', event => {
91+
this._cleanupHook = client.on('afterSendEvent', event => {
9192
if (!event.type && this._lastEventId && event.event_id === this._lastEventId) {
9293
showReportDialog({ ...props.dialogOptions, eventId: this._lastEventId });
9394
}
@@ -137,6 +138,11 @@ class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundarySta
137138
if (onUnmount) {
138139
onUnmount(error, componentStack, eventId);
139140
}
141+
142+
if (this._cleanupHook) {
143+
this._cleanupHook();
144+
this._cleanupHook = undefined;
145+
}
140146
}
141147

142148
public resetErrorBoundary: () => void = () => {

0 commit comments

Comments
 (0)