Auto tracing for expo-image and expo-asset#5718
Merged
Conversation
Contributor
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
Contributor
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
### Features
- Auto tracing for expo-image and expo-asset ([#5718](https://github.com/getsentry/sentry-react-native/pull/5718))If none of the above apply, you can opt out of this check by adding |
alwx
changed the title
alwx
changed the title
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Identical
describeUrlfunction duplicated across two new files- I extracted
describeUrlinto a sharedtracing/describeUrl.tsutility and updated both wrappers to import it.
- I extracted
- ✅ Fixed: Missing try/catch leaves spans unclosed on synchronous throw
- I wrapped each original wrapper call in
try/catchto set error status and end spans on synchronous throws, and added tests for these paths.
- I wrapped each original wrapper call in
Or push these changes by commenting:
@cursor push 9eed63c488
Preview (9eed63c488)
diff --git a/packages/core/src/js/tracing/describeUrl.ts b/packages/core/src/js/tracing/describeUrl.ts
new file mode 100644
--- /dev/null
+++ b/packages/core/src/js/tracing/describeUrl.ts
@@ -1,0 +1,14 @@
+/**
+ * Extracts a human-readable URL identifier for span names.
+ */
+export function describeUrl(url: string): string {
+ try {
+ // Remove query string and fragment
+ const withoutQuery = url.split('?')[0] || url;
+ const withoutFragment = withoutQuery.split('#')[0] || withoutQuery;
+ const filename = withoutFragment.split('/').pop();
+ return filename || url;
+ } catch {
+ return url;
+ }
+}
diff --git a/packages/core/src/js/tracing/expoAsset.ts b/packages/core/src/js/tracing/expoAsset.ts
--- a/packages/core/src/js/tracing/expoAsset.ts
+++ b/packages/core/src/js/tracing/expoAsset.ts
@@ -1,4 +1,5 @@
import { SPAN_STATUS_ERROR, SPAN_STATUS_OK, startInactiveSpan } from '@sentry/core';
+import { describeUrl } from './describeUrl';
import { SPAN_ORIGIN_AUTO_RESOURCE_EXPO_ASSET } from './origin';
/**
@@ -80,17 +81,23 @@
},
});
- return originalLoadAsync(moduleId)
- .then(result => {
- span?.setStatus({ code: SPAN_STATUS_OK });
- span?.end();
- return result;
- })
- .catch((error: unknown) => {
- span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
- span?.end();
- throw error;
- });
+ try {
+ return originalLoadAsync(moduleId)
+ .then(result => {
+ span?.setStatus({ code: SPAN_STATUS_OK });
+ span?.end();
+ return result;
+ })
+ .catch((error: unknown) => {
+ span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
+ span?.end();
+ throw error;
+ });
+ } catch (error) {
+ span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
+ span?.end();
+ throw error;
+ }
}) as T['loadAsync'];
}
@@ -104,15 +111,3 @@
}
return `${moduleIds.length} assets`;
}
-
-function describeUrl(url: string): string {
- try {
- // Remove query string and fragment
- const withoutQuery = url.split('?')[0] || url;
- const withoutFragment = withoutQuery.split('#')[0] || withoutQuery;
- const filename = withoutFragment.split('/').pop();
- return filename || url;
- } catch {
- return url;
- }
-}
diff --git a/packages/core/src/js/tracing/expoImage.ts b/packages/core/src/js/tracing/expoImage.ts
--- a/packages/core/src/js/tracing/expoImage.ts
+++ b/packages/core/src/js/tracing/expoImage.ts
@@ -1,4 +1,5 @@
import { SPAN_STATUS_ERROR, SPAN_STATUS_OK, startInactiveSpan } from '@sentry/core';
+import { describeUrl } from './describeUrl';
import { SPAN_ORIGIN_AUTO_RESOURCE_EXPO_IMAGE } from './origin';
/**
@@ -105,17 +106,23 @@
},
});
- return originalPrefetch(urls, cachePolicyOrOptions)
- .then(result => {
- span?.setStatus({ code: SPAN_STATUS_OK });
- span?.end();
- return result;
- })
- .catch((error: unknown) => {
- span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
- span?.end();
- throw error;
- });
+ try {
+ return originalPrefetch(urls, cachePolicyOrOptions)
+ .then(result => {
+ span?.setStatus({ code: SPAN_STATUS_OK });
+ span?.end();
+ return result;
+ })
+ .catch((error: unknown) => {
+ span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
+ span?.end();
+ throw error;
+ });
+ } catch (error) {
+ span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
+ span?.end();
+ throw error;
+ }
}) as T['prefetch'];
}
@@ -144,32 +151,26 @@
},
});
- return originalLoadAsync(source, options)
- .then(result => {
- span?.setStatus({ code: SPAN_STATUS_OK });
- span?.end();
- return result;
- })
- .catch((error: unknown) => {
- span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
- span?.end();
- throw error;
- });
+ try {
+ return originalLoadAsync(source, options)
+ .then(result => {
+ span?.setStatus({ code: SPAN_STATUS_OK });
+ span?.end();
+ return result;
+ })
+ .catch((error: unknown) => {
+ span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
+ span?.end();
+ throw error;
+ });
+ } catch (error) {
+ span?.setStatus({ code: SPAN_STATUS_ERROR, message: String(error) });
+ span?.end();
+ throw error;
+ }
}) as T['loadAsync'];
}
-function describeUrl(url: string): string {
- try {
- // Remove query string and fragment
- const withoutQuery = url.split('?')[0] || url;
- const withoutFragment = withoutQuery.split('#')[0] || withoutQuery;
- const filename = withoutFragment.split('/').pop();
- return filename || url;
- } catch {
- return url;
- }
-}
-
function describeSource(source: ExpoImageSource | string | number): string {
if (typeof source === 'number') {
return `asset #${source}`;
diff --git a/packages/core/test/tracing/expoAsset.test.ts b/packages/core/test/tracing/expoAsset.test.ts
--- a/packages/core/test/tracing/expoAsset.test.ts
+++ b/packages/core/test/tracing/expoAsset.test.ts
@@ -158,6 +158,26 @@
expect(mockSpan.end).toHaveBeenCalled();
});
+ it('ends the span when loadAsync throws synchronously', () => {
+ const error = new Error('Invalid module id');
+ const mockLoadAsync = jest.fn(() => {
+ throw error;
+ });
+ const assetClass = {
+ loadAsync: mockLoadAsync,
+ fromModule: jest.fn(),
+ } as unknown as ExpoAsset;
+
+ wrapExpoAsset(assetClass);
+
+ expect(() => assetClass.loadAsync(99)).toThrow('Invalid module id');
+ expect(mockSpan.setStatus).toHaveBeenCalledWith({
+ code: SPAN_STATUS_ERROR,
+ message: 'Error: Invalid module id',
+ });
+ expect(mockSpan.end).toHaveBeenCalled();
+ });
+
it('passes the original moduleId argument through', async () => {
const mockLoadAsync = jest.fn().mockResolvedValue([]);
const assetClass = {
diff --git a/packages/core/test/tracing/expoImage.test.ts b/packages/core/test/tracing/expoImage.test.ts
--- a/packages/core/test/tracing/expoImage.test.ts
+++ b/packages/core/test/tracing/expoImage.test.ts
@@ -109,6 +109,23 @@
expect(mockSpan.end).toHaveBeenCalled();
});
+ it('ends the span when prefetch throws synchronously', () => {
+ const error = new Error('Invalid prefetch input');
+ const mockPrefetch = jest.fn(() => {
+ throw error;
+ });
+ const imageClass = { prefetch: mockPrefetch, loadAsync: jest.fn() } as unknown as ExpoImage;
+
+ wrapExpoImage(imageClass);
+
+ expect(() => imageClass.prefetch('https://example.com/image.png')).toThrow('Invalid prefetch input');
+ expect(mockSpan.setStatus).toHaveBeenCalledWith({
+ code: SPAN_STATUS_ERROR,
+ message: 'Error: Invalid prefetch input',
+ });
+ expect(mockSpan.end).toHaveBeenCalled();
+ });
+
it('handles URL without path correctly', async () => {
const mockPrefetch = jest.fn().mockResolvedValue(true);
const imageClass = { prefetch: mockPrefetch, loadAsync: jest.fn() } as unknown as ExpoImage;
@@ -230,6 +247,23 @@
expect(mockSpan.end).toHaveBeenCalled();
});
+ it('ends the span when loadAsync throws synchronously', () => {
+ const error = new Error('Invalid image source');
+ const mockLoadAsync = jest.fn(() => {
+ throw error;
+ });
+ const imageClass = { prefetch: jest.fn(), loadAsync: mockLoadAsync } as unknown as ExpoImage;
+
+ wrapExpoImage(imageClass);
+
+ expect(() => imageClass.loadAsync('https://example.com/broken.png')).toThrow('Invalid image source');
+ expect(mockSpan.setStatus).toHaveBeenCalledWith({
+ code: SPAN_STATUS_ERROR,
+ message: 'Error: Invalid image source',
+ });
+ expect(mockSpan.end).toHaveBeenCalled();
+ });
+
it('passes options through to original loadAsync', async () => {
const mockResult = { width: 100, height: 100, scale: 1, mediaType: null };
const mockLoadAsync = jest.fn().mockResolvedValue(mockResult);
Contributor
Author
|
@antonis @lucas-zimerman this one is ready now! |
antonis
reviewed
Mar 9, 2026
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| export function sanitizeUrl(url: string): string { | ||
| try { | ||
| const withoutQuery = url.split('?')[0] || url; | ||
| return withoutQuery.split('#')[0] || withoutQuery; |
There was a problem hiding this comment.
Unnecessary || fallback defeats URL sanitization for edge-case inputs
Low Severity
sanitizeUrl and describeUrl use || url / || withoutQuery fallbacks after split('?')[0] and split('#')[0]. Since String.split() always returns a non-empty array, [0] is always a string — never null or undefined. The || fallback only activates when the result is an empty string (falsy), which happens when the URL starts with ? or #. In that case, the fallback restores the original unsanitized URL, defeating the purpose of stripping query parameters. Removing the || url / || withoutQuery fallback entirely fixes this — an empty string is the correct result when nothing precedes the delimiter.
Additional Locations (1)
antonis
approved these changes
Mar 10, 2026
Contributor
antonis
left a comment
antonis
left a comment
There was a problem hiding this comment.
LGTM 🚀
Thank you for your work on this and iterating on the feedback 🙇
Contributor
iOS (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| ea3e26e+dirty | 1229.13 ms | 1228.46 ms | -0.67 ms |
| 80e4616+dirty | 1221.32 ms | 1225.64 ms | 4.32 ms |
| 818a608+dirty | 1205.76 ms | 1208.00 ms | 2.24 ms |
| 77061ed+dirty | 1233.16 ms | 1234.88 ms | 1.71 ms |
| bef3709+dirty | 1222.07 ms | 1220.24 ms | -1.83 ms |
| a206511+dirty | 1185.00 ms | 1186.35 ms | 1.35 ms |
| 74979ac+dirty | 1210.49 ms | 1213.31 ms | 2.82 ms |
| a2bb688+dirty | 1223.53 ms | 1232.90 ms | 9.37 ms |
| 8a868fe+dirty | 1221.50 ms | 1230.78 ms | 9.28 ms |
| d590428+dirty | 1211.77 ms | 1220.51 ms | 8.75 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| ea3e26e+dirty | 3.41 MiB | 4.58 MiB | 1.17 MiB |
| 80e4616+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
| 818a608+dirty | 2.63 MiB | 3.91 MiB | 1.28 MiB |
| 77061ed+dirty | 2.63 MiB | 3.98 MiB | 1.34 MiB |
| bef3709+dirty | 3.38 MiB | 4.78 MiB | 1.40 MiB |
| a206511+dirty | 3.41 MiB | 4.67 MiB | 1.25 MiB |
| 74979ac+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
| a2bb688+dirty | 2.63 MiB | 3.99 MiB | 1.36 MiB |
| 8a868fe+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
| d590428+dirty | 3.38 MiB | 4.78 MiB | 1.39 MiB |
Contributor
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 86584b7+dirty | 463.83 ms | 500.31 ms | 36.48 ms |
| 9a81842+dirty | 412.23 ms | 416.56 ms | 4.33 ms |
| c637fc7+dirty | 433.70 ms | 467.76 ms | 34.06 ms |
| d73150f+dirty | 411.21 ms | 465.86 ms | 54.65 ms |
| fa7bb7e+dirty | 350.37 ms | 377.02 ms | 26.65 ms |
| 3bd3f0d+dirty | 447.21 ms | 472.31 ms | 25.10 ms |
| 88890fe+dirty | 350.94 ms | 365.74 ms | 14.80 ms |
| 95aaf8a | 437.89 ms | 419.45 ms | -18.44 ms |
| c0842e7+dirty | 527.76 ms | 566.69 ms | 38.93 ms |
| 1e7a472+dirty | 348.80 ms | 362.55 ms | 13.75 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 86584b7+dirty | 43.75 MiB | 48.08 MiB | 4.33 MiB |
| 9a81842+dirty | 43.75 MiB | 48.08 MiB | 4.33 MiB |
| c637fc7+dirty | 43.75 MiB | 48.40 MiB | 4.64 MiB |
| d73150f+dirty | 43.75 MiB | 48.55 MiB | 4.80 MiB |
| fa7bb7e+dirty | 17.75 MiB | 19.75 MiB | 2.00 MiB |
| 3bd3f0d+dirty | 17.75 MiB | 19.70 MiB | 1.95 MiB |
| 88890fe+dirty | 17.75 MiB | 19.71 MiB | 1.96 MiB |
| 95aaf8a | 17.75 MiB | 19.68 MiB | 1.93 MiB |
| c0842e7+dirty | 43.75 MiB | 48.41 MiB | 4.66 MiB |
| 1e7a472+dirty | 17.75 MiB | 19.70 MiB | 1.96 MiB |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📢 Type of change
📜 Description
Fixes #5427
💡 Motivation and Context
💚 How did you test it?
📝 Checklist
sendDefaultPIIis enabled🔮 Next steps