Skip to content

Commit 9eed63c

Browse files
cursoragentcursoragent
committed
fix(tracing): close expo spans on sync throws
1 parent 56eac67 commit 9eed63c

5 files changed

Lines changed: 121 additions & 57 deletions

File tree

‎packages/core/src/js/tracing/describeUrl.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/**
2+
* Extracts a human-readable URL identifier for span names.
3+
*/
4+
export function describeUrl(url: string): string {
5+
try {
6+
// Remove query string and fragment
7+
const withoutQuery = url.split('?')[0] || url;
8+
const withoutFragment = withoutQuery.split('#')[0] || withoutQuery;
9+
const filename = withoutFragment.split('/').pop();
10+
return filename || url;
11+
} catch {
12+
return url;
13+
}
14+
}

‎packages/core/src/js/tracing/expoAsset.ts‎

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { SPAN_STATUS_ERROR, SPAN_STATUS_OK, startInactiveSpan } from '@sentry/core';
2+
import { describeUrl } from './describeUrl';
23
import { SPAN_ORIGIN_AUTO_RESOURCE_EXPO_ASSET } from './origin';
34

45
/**
@@ -80,17 +81,23 @@ function wrapLoadAsync<T extends ExpoAsset>(assetClass: T): void {
8081
},
8182
});
8283

83-
return originalLoadAsync(moduleId)
84-
.then(result => {
85-
span?.setStatus({ code: SPAN_STATUS_OK });
86-
span?.end();
87-
return result;
88-
})
89-
.catch((error: unknown) => {
90-
span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
91-
span?.end();
92-
throw error;
93-
});
84+
try {
85+
return originalLoadAsync(moduleId)
86+
.then(result => {
87+
span?.setStatus({ code: SPAN_STATUS_OK });
88+
span?.end();
89+
return result;
90+
})
91+
.catch((error: unknown) => {
92+
span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
93+
span?.end();
94+
throw error;
95+
});
96+
} catch (error) {
97+
span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
98+
span?.end();
99+
throw error;
100+
}
94101
}) as T['loadAsync'];
95102
}
96103

@@ -104,15 +111,3 @@ function describeModuleIds(moduleIds: (number | string)[]): string {
104111
}
105112
return `${moduleIds.length} assets`;
106113
}
107-
108-
function describeUrl(url: string): string {
109-
try {
110-
// Remove query string and fragment
111-
const withoutQuery = url.split('?')[0] || url;
112-
const withoutFragment = withoutQuery.split('#')[0] || withoutQuery;
113-
const filename = withoutFragment.split('/').pop();
114-
return filename || url;
115-
} catch {
116-
return url;
117-
}
118-
}

‎packages/core/src/js/tracing/expoImage.ts‎

Lines changed: 35 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { SPAN_STATUS_ERROR, SPAN_STATUS_OK, startInactiveSpan } from '@sentry/core';
2+
import { describeUrl } from './describeUrl';
23
import { SPAN_ORIGIN_AUTO_RESOURCE_EXPO_IMAGE } from './origin';
34

45
/**
@@ -105,17 +106,23 @@ function wrapPrefetch<T extends ExpoImage>(imageClass: T): void {
105106
},
106107
});
107108

108-
return originalPrefetch(urls, cachePolicyOrOptions)
109-
.then(result => {
110-
span?.setStatus({ code: SPAN_STATUS_OK });
111-
span?.end();
112-
return result;
113-
})
114-
.catch((error: unknown) => {
115-
span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
116-
span?.end();
117-
throw error;
118-
});
109+
try {
110+
return originalPrefetch(urls, cachePolicyOrOptions)
111+
.then(result => {
112+
span?.setStatus({ code: SPAN_STATUS_OK });
113+
span?.end();
114+
return result;
115+
})
116+
.catch((error: unknown) => {
117+
span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
118+
span?.end();
119+
throw error;
120+
});
121+
} catch (error) {
122+
span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
123+
span?.end();
124+
throw error;
125+
}
119126
}) as T['prefetch'];
120127
}
121128

@@ -144,32 +151,26 @@ function wrapLoadAsync<T extends ExpoImage>(imageClass: T): void {
144151
},
145152
});
146153

147-
return originalLoadAsync(source, options)
148-
.then(result => {
149-
span?.setStatus({ code: SPAN_STATUS_OK });
150-
span?.end();
151-
return result;
152-
})
153-
.catch((error: unknown) => {
154-
span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
155-
span?.end();
156-
throw error;
157-
});
154+
try {
155+
return originalLoadAsync(source, options)
156+
.then(result => {
157+
span?.setStatus({ code: SPAN_STATUS_OK });
158+
span?.end();
159+
return result;
160+
})
161+
.catch((error: unknown) => {
162+
span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
163+
span?.end();
164+
throw error;
165+
});
166+
} catch (error) {
167+
span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
168+
span?.end();
169+
throw error;
170+
}
158171
}) as T['loadAsync'];
159172
}
160173

161-
function describeUrl(url: string): string {
162-
try {
163-
// Remove query string and fragment
164-
const withoutQuery = url.split('?')[0] || url;
165-
const withoutFragment = withoutQuery.split('#')[0] || withoutQuery;
166-
const filename = withoutFragment.split('/').pop();
167-
return filename || url;
168-
} catch {
169-
return url;
170-
}
171-
}
172-
173174
function describeSource(source: ExpoImageSource | string | number): string {
174175
if (typeof source === 'number') {
175176
return `asset #${source}`;

‎packages/core/test/tracing/expoAsset.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,26 @@ describe('wrapExpoAsset', () => {
158158
expect(mockSpan.end).toHaveBeenCalled();
159159
});
160160

161+
it('ends the span when loadAsync throws synchronously', () => {
162+
const error = new Error('Invalid module id');
163+
const mockLoadAsync = jest.fn(() => {
164+
throw error;
165+
});
166+
const assetClass = {
167+
loadAsync: mockLoadAsync,
168+
fromModule: jest.fn(),
169+
} as unknown as ExpoAsset;
170+
171+
wrapExpoAsset(assetClass);
172+
173+
expect(() => assetClass.loadAsync(99)).toThrow('Invalid module id');
174+
expect(mockSpan.setStatus).toHaveBeenCalledWith({
175+
code: SPAN_STATUS_ERROR,
176+
message: 'Error: Invalid module id',
177+
});
178+
expect(mockSpan.end).toHaveBeenCalled();
179+
});
180+
161181
it('passes the original moduleId argument through', async () => {
162182
const mockLoadAsync = jest.fn().mockResolvedValue([]);
163183
const assetClass = {

‎packages/core/test/tracing/expoImage.test.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,23 @@ describe('wrapExpoImage', () => {
109109
expect(mockSpan.end).toHaveBeenCalled();
110110
});
111111

112+
it('ends the span when prefetch throws synchronously', () => {
113+
const error = new Error('Invalid prefetch input');
114+
const mockPrefetch = jest.fn(() => {
115+
throw error;
116+
});
117+
const imageClass = { prefetch: mockPrefetch, loadAsync: jest.fn() } as unknown as ExpoImage;
118+
119+
wrapExpoImage(imageClass);
120+
121+
expect(() => imageClass.prefetch('https://example.com/image.png')).toThrow('Invalid prefetch input');
122+
expect(mockSpan.setStatus).toHaveBeenCalledWith({
123+
code: SPAN_STATUS_ERROR,
124+
message: 'Error: Invalid prefetch input',
125+
});
126+
expect(mockSpan.end).toHaveBeenCalled();
127+
});
128+
112129
it('handles URL without path correctly', async () => {
113130
const mockPrefetch = jest.fn().mockResolvedValue(true);
114131
const imageClass = { prefetch: mockPrefetch, loadAsync: jest.fn() } as unknown as ExpoImage;
@@ -230,6 +247,23 @@ describe('wrapExpoImage', () => {
230247
expect(mockSpan.end).toHaveBeenCalled();
231248
});
232249

250+
it('ends the span when loadAsync throws synchronously', () => {
251+
const error = new Error('Invalid image source');
252+
const mockLoadAsync = jest.fn(() => {
253+
throw error;
254+
});
255+
const imageClass = { prefetch: jest.fn(), loadAsync: mockLoadAsync } as unknown as ExpoImage;
256+
257+
wrapExpoImage(imageClass);
258+
259+
expect(() => imageClass.loadAsync('https://example.com/broken.png')).toThrow('Invalid image source');
260+
expect(mockSpan.setStatus).toHaveBeenCalledWith({
261+
code: SPAN_STATUS_ERROR,
262+
message: 'Error: Invalid image source',
263+
});
264+
expect(mockSpan.end).toHaveBeenCalled();
265+
});
266+
233267
it('passes options through to original loadAsync', async () => {
234268
const mockResult = { width: 100, height: 100, scale: 1, mediaType: null };
235269
const mockLoadAsync = jest.fn().mockResolvedValue(mockResult);

0 commit comments

Comments
 (0)