Skip to content

Commit 89b5720

Browse files
authored
fix(tracing): Account for case where startTransaction returns undefined (#7566)
1 parent 4f34b5a commit 89b5720

File tree

2 files changed

+28
-6
lines changed

2 files changed

+28
-6
lines changed

packages/core/src/tracing/trace.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,17 @@ import type { Span } from './span';
77
/**
88
* Wraps a function with a transaction/span and finishes the span after the function is done.
99
*
10+
* Note that if you have not enabled tracing extensions via `addTracingExtensions`, this function
11+
* will not generate spans, and the `span` returned from the callback may be undefined.
12+
*
1013
* This function is meant to be used internally and may break at any time. Use at your own risk.
1114
*
1215
* @internal
1316
* @private
1417
*/
1518
export function trace<T>(
1619
context: TransactionContext,
17-
callback: (span: Span) => T,
20+
callback: (span?: Span) => T,
1821
// eslint-disable-next-line @typescript-eslint/no-empty-function
1922
onError: (error: unknown) => void = () => {},
2023
): T {
@@ -32,15 +35,15 @@ export function trace<T>(
3235
scope.setSpan(activeSpan);
3336

3437
function finishAndSetSpan(): void {
35-
activeSpan.finish();
38+
activeSpan && activeSpan.finish();
3639
hub.getScope().setSpan(parentSpan);
3740
}
3841

3942
let maybePromiseResult: T;
4043
try {
4144
maybePromiseResult = callback(activeSpan);
4245
} catch (e) {
43-
activeSpan.setStatus('internal_error');
46+
activeSpan && activeSpan.setStatus('internal_error');
4447
onError(e);
4548
finishAndSetSpan();
4649
throw e;
@@ -52,7 +55,7 @@ export function trace<T>(
5255
finishAndSetSpan();
5356
},
5457
e => {
55-
activeSpan.setStatus('internal_error');
58+
activeSpan && activeSpan.setStatus('internal_error');
5659
onError(e);
5760
finishAndSetSpan();
5861
},

packages/core/test/lib/tracing/trace.test.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,21 @@ describe('trace', () => {
4747
}
4848
});
4949

50+
it('should return the same value as the callback if transactions are undefined', async () => {
51+
// @ts-ignore we are force overriding the transaction return to be undefined
52+
// The `startTransaction` types are actually wrong - it can return undefined
53+
// if tracingExtensions are not enabled
54+
jest.spyOn(hub, 'startTransaction').mockReturnValue(undefined);
55+
try {
56+
const result = await trace({ name: 'GET users/[id]' }, () => {
57+
return callback();
58+
});
59+
expect(result).toEqual(expected);
60+
} catch (e) {
61+
expect(e).toEqual(expected);
62+
}
63+
});
64+
5065
it('creates a transaction', async () => {
5166
let ref: any = undefined;
5267
client.on('finishTransaction', transaction => {
@@ -99,7 +114,9 @@ describe('trace', () => {
99114
});
100115
try {
101116
await trace({ name: 'GET users/[id]' }, span => {
102-
span.op = 'http.server';
117+
if (span) {
118+
span.op = 'http.server';
119+
}
103120
return callback();
104121
});
105122
} catch (e) {
@@ -138,7 +155,9 @@ describe('trace', () => {
138155
try {
139156
await trace({ name: 'GET users/[id]', parentSampled: true }, () => {
140157
return trace({ name: 'SELECT * from users' }, childSpan => {
141-
childSpan.op = 'db.query';
158+
if (childSpan) {
159+
childSpan.op = 'db.query';
160+
}
142161
return callback();
143162
});
144163
});

0 commit comments

Comments
 (0)