Skip to content

Commit 99bea1b

Browse files
committed
fix(nestjs): do not dangle spans for async iterators
1 parent cff4680 commit 99bea1b

2 files changed

Lines changed: 83 additions & 5 deletions

File tree

packages/server-utils/src/integrations/tracing-channel/nestjs-decorators.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -167,15 +167,17 @@ function patchInterceptor(target: InjectableTarget, intercept: AnyFn, seenContex
167167
throw e;
168168
}
169169

170-
if (!afterSpan) {
171-
return returned;
172-
}
173-
174170
// async interceptor: returns a Promise<Observable>
175171
if (isThenable(returned)) {
176172
return returned.then(
177173
(observable: unknown) => {
178-
instrumentObservable(observable as ObservableLike, afterSpan ?? parentSpan);
174+
if (afterSpan) {
175+
instrumentObservable(observable as ObservableLike, afterSpan);
176+
} else {
177+
// `next.handle()` was never called, so nothing ended the
178+
// before-span (its `handle` proxy never ran); close it here.
179+
beforeSpan.end();
180+
}
179181
return observable;
180182
},
181183
(e: unknown) => {
@@ -186,6 +188,12 @@ function patchInterceptor(target: InjectableTarget, intercept: AnyFn, seenContex
186188
);
187189
}
188190

191+
// Sync interceptor: `next.handle()` (if it was going to be called) has
192+
// already run synchronously, so `afterSpan` is settled.
193+
if (!afterSpan) {
194+
return returned;
195+
}
196+
189197
// sync interceptor: returns an Observable
190198
if (typeof (returned as ObservableLike).subscribe === 'function') {
191199
instrumentObservable(returned as ObservableLike, afterSpan);

packages/server-utils/test/orchestrion/nestjs.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,76 @@ describe('nestjsChannelIntegration: @Injectable (middleware/guard/pipe/intercept
444444
expect(() => teardowns.forEach(fn => fn())).not.toThrow();
445445
});
446446

447+
it('async interceptor that awaits before next.handle(): still instruments the after-span', async () => {
448+
installTestAsyncContextStrategy();
449+
initTestClient();
450+
nestjsChannelIntegration().setupOnce!();
451+
452+
const teardowns: Array<() => void> = [];
453+
const observable = {
454+
subscribe(): { add: (fn: () => void) => void } {
455+
return { add: (fn: () => void) => void teardowns.push(fn) };
456+
},
457+
};
458+
459+
let beforeSpan: ReturnType<typeof getActiveSpan>;
460+
class AsyncInterceptor {
461+
// Awaits *before* calling `next.handle()`, so `intercept` returns a
462+
// pending Promise while the after-span does not yet exist.
463+
public async intercept(_context: unknown, next: { handle: () => unknown }): Promise<unknown> {
464+
beforeSpan = getActiveSpan();
465+
await Promise.resolve();
466+
return next.handle();
467+
}
468+
}
469+
applyInjectable(AsyncInterceptor);
470+
471+
const next = { handle: () => observable };
472+
const returned = (await new AsyncInterceptor().intercept({}, next)) as typeof observable;
473+
474+
expect(returned).toBe(observable);
475+
// before-span ended (when `next.handle()` ran, post-await)
476+
expect(spanToJSON(beforeSpan!).timestamp).toBeDefined();
477+
// after-span was created AND the observable instrumented despite the await
478+
returned.subscribe();
479+
expect(teardowns).toHaveLength(1);
480+
expect(() => teardowns.forEach(fn => fn())).not.toThrow();
481+
});
482+
483+
it('async interceptor that never calls next.handle(): ends the before-span, no after-span', async () => {
484+
installTestAsyncContextStrategy();
485+
initTestClient();
486+
nestjsChannelIntegration().setupOnce!();
487+
488+
const teardowns: Array<() => void> = [];
489+
const observable = {
490+
subscribe(): { add: (fn: () => void) => void } {
491+
return { add: (fn: () => void) => void teardowns.push(fn) };
492+
},
493+
};
494+
495+
let beforeSpan: ReturnType<typeof getActiveSpan>;
496+
class ShortCircuitInterceptor {
497+
public async intercept(_context: unknown, _next: { handle: () => unknown }): Promise<unknown> {
498+
beforeSpan = getActiveSpan();
499+
await Promise.resolve();
500+
return observable; // short-circuits without calling `next.handle()`
501+
}
502+
}
503+
applyInjectable(ShortCircuitInterceptor);
504+
505+
const next = { handle: vi.fn() };
506+
const returned = (await new ShortCircuitInterceptor().intercept({}, next)) as typeof observable;
507+
508+
expect(returned).toBe(observable);
509+
expect(next.handle).not.toHaveBeenCalled();
510+
// before-span is closed even though `next.handle()` (which normally ends it) never ran
511+
expect(spanToJSON(beforeSpan!).timestamp).toBeDefined();
512+
// no after-span, so the observable is left un-instrumented
513+
returned.subscribe();
514+
expect(teardowns).toHaveLength(0);
515+
});
516+
447517
it('skips targets flagged __SENTRY_INTERNAL__', () => {
448518
installTestAsyncContextStrategy();
449519
initTestClient();

0 commit comments

Comments
 (0)