fix: Prevent Metro serializer crash on non-standard serializer output - #6652
Open
antonis wants to merge 3 commits into
Open
fix: Prevent Metro serializer crash on non-standard serializer output#6652antonis wants to merge 3 commits into
antonis wants to merge 3 commits into
Conversation
The Metro serializer used `'map' in serializerResult` to detect a
`{ code, map }` bundle. For arrays that check is always true because of
`Array.prototype.map`, so an array result (e.g. Expo's static/EAS Update
export) yielded `{ code: undefined }` and crashed in
`determineDebugIdFromBundleSource` with "Cannot read properties of
undefined (reading 'match')".
Detect a bundle with a positive check (object, not array, string `code`)
and pass non-standard output through untouched instead of crashing.
Fixes #6650
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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. |
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
@cursor review |
Contributor
Author
|
@sentry review |
There was a problem hiding this comment.
🟡 Changes recommended
The implementation now supports (and may return) non-standard serializer outputs, but the exported serializer types still only model standard { code, map }/string outputs, leaving behavior and TypeScript API inconsistent.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes a Metro bundler crash in the Sentry Metro serializer when the wrapped serializer returns non-standard outputs (notably arrays from Expo static/EAS Update exports), by tightening bundle-shape detection and safely passing through unsupported result shapes.
Changes:
- Replace the unsound
'map' in resultbundle check with a stricter object-shape check (codemust be a string and result must not be an array). - Pass through non-standard serializer outputs unchanged instead of attempting Debug ID injection and crashing.
- Add regression tests covering array and promise-resolving-to-array serializer outputs; document the fix in the changelog.
File summaries
| File | Description |
|---|---|
| packages/core/src/js/tools/sentryMetroSerializer.ts | Makes serializer result handling robust against non-standard outputs and avoids crashing on array results. |
| packages/core/test/tools/sentryMetroSerializer.test.ts | Adds regression tests for array and async-array serializer outputs to prevent reintroducing the crash. |
| CHANGELOG.md | Notes the fix in the Unreleased section for user visibility. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit c67100a. Configure here.
📲 Install BuildsAndroid
|
Contributor
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 5a010b7+dirty | 425.62 ms | 469.38 ms | 43.76 ms |
| 1a5721e+dirty | 424.07 ms | 482.32 ms | 58.25 ms |
| 7d8c8bd+dirty | 417.45 ms | 462.10 ms | 44.65 ms |
| 27d9693+dirty | 419.08 ms | 469.12 ms | 50.04 ms |
| 6bbd2c7+dirty | 421.43 ms | 436.30 ms | 14.88 ms |
| ef27341+dirty | 412.94 ms | 443.98 ms | 31.04 ms |
| 41d6254+dirty | 424.45 ms | 474.34 ms | 49.89 ms |
| b9bebee+dirty | 438.86 ms | 452.21 ms | 13.35 ms |
| 3d536d1+dirty | 524.34 ms | 547.32 ms | 22.98 ms |
| 580fb5c+dirty | 436.34 ms | 471.63 ms | 35.28 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 5a010b7+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
| 1a5721e+dirty | 49.74 MiB | 55.09 MiB | 5.34 MiB |
| 7d8c8bd+dirty | 48.30 MiB | 53.54 MiB | 5.23 MiB |
| 27d9693+dirty | 49.74 MiB | 55.09 MiB | 5.34 MiB |
| 6bbd2c7+dirty | 49.74 MiB | 55.66 MiB | 5.91 MiB |
| ef27341+dirty | 48.30 MiB | 53.54 MiB | 5.24 MiB |
| 41d6254+dirty | 48.30 MiB | 53.60 MiB | 5.30 MiB |
| b9bebee+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
| 3d536d1+dirty | 49.74 MiB | 55.26 MiB | 5.52 MiB |
| 580fb5c+dirty | 49.74 MiB | 54.79 MiB | 5.05 MiB |
Contributor
iOS (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 68ae91b+dirty | 3834.04 ms | 1216.60 ms | -2617.44 ms |
| 0b5a379+dirty | 3828.91 ms | 1214.12 ms | -2614.79 ms |
| f170ec3+dirty | 3822.26 ms | 1218.33 ms | -2603.93 ms |
| 68672fc+dirty | 3841.58 ms | 1228.89 ms | -2612.69 ms |
| 2c735cc+dirty | 1229.67 ms | 1221.50 ms | -8.17 ms |
| b9bebee+dirty | 3850.15 ms | 1227.51 ms | -2622.64 ms |
| a50b33d+dirty | 1197.74 ms | 1197.17 ms | -0.57 ms |
| 5569641+dirty | 3839.22 ms | 1231.30 ms | -2607.91 ms |
| f9c1ed4+dirty | 3833.98 ms | 1226.30 ms | -2607.68 ms |
| 7ac3378+dirty | 1213.37 ms | 1218.15 ms | 4.78 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 68ae91b+dirty | 4.98 MiB | 6.46 MiB | 1.48 MiB |
| 0b5a379+dirty | 5.15 MiB | 6.70 MiB | 1.54 MiB |
| f170ec3+dirty | 5.15 MiB | 6.69 MiB | 1.53 MiB |
| 68672fc+dirty | 5.15 MiB | 6.71 MiB | 1.55 MiB |
| 2c735cc+dirty | 3.38 MiB | 4.74 MiB | 1.35 MiB |
| b9bebee+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
| a50b33d+dirty | 3.38 MiB | 4.73 MiB | 1.35 MiB |
| 5569641+dirty | 5.15 MiB | 6.67 MiB | 1.51 MiB |
| f9c1ed4+dirty | 4.98 MiB | 6.50 MiB | 1.53 MiB |
| 7ac3378+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
Contributor
Android (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| c151573+dirty | 485.39 ms | 495.18 ms | 9.79 ms |
| 5c1e987+dirty | 444.71 ms | 475.13 ms | 30.42 ms |
| 04207c4+dirty | 395.40 ms | 456.55 ms | 61.15 ms |
| 3ce5254+dirty | 373.90 ms | 427.84 ms | 53.94 ms |
| 6176a94+dirty | 403.58 ms | 446.73 ms | 43.15 ms |
| 9474ead+dirty | 432.18 ms | 481.92 ms | 49.73 ms |
| 09a902f+dirty | 423.02 ms | 472.18 ms | 49.16 ms |
| 882f8ae+dirty | 408.19 ms | 435.86 ms | 27.67 ms |
| 1122a96+dirty | 510.16 ms | 542.00 ms | 31.84 ms |
| 7d6fd3a+dirty | 435.06 ms | 458.78 ms | 23.72 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| c151573+dirty | 48.30 MiB | 53.54 MiB | 5.24 MiB |
| 5c1e987+dirty | 43.94 MiB | 48.94 MiB | 5.00 MiB |
| 04207c4+dirty | 43.94 MiB | 48.98 MiB | 5.04 MiB |
| 3ce5254+dirty | 43.94 MiB | 48.98 MiB | 5.04 MiB |
| 6176a94+dirty | 48.30 MiB | 53.54 MiB | 5.24 MiB |
| 9474ead+dirty | 48.30 MiB | 53.61 MiB | 5.30 MiB |
| 09a902f+dirty | 49.74 MiB | 54.81 MiB | 5.07 MiB |
| 882f8ae+dirty | 48.30 MiB | 53.60 MiB | 5.29 MiB |
| 1122a96+dirty | 48.30 MiB | 53.54 MiB | 5.24 MiB |
| 7d6fd3a+dirty | 43.94 MiB | 49.00 MiB | 5.06 MiB |
Contributor
iOS (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3b6e9f9+dirty | 3822.77 ms | 1208.00 ms | -2614.77 ms |
| 4e0b819+dirty | 3828.96 ms | 1205.64 ms | -2623.32 ms |
| 1e5d96d+dirty | 3845.93 ms | 1222.51 ms | -2623.42 ms |
| 0a9e622+dirty | 3825.35 ms | 1219.04 ms | -2606.31 ms |
| 5ca03f9+dirty | 3873.76 ms | 1236.26 ms | -2637.51 ms |
| 40c9884+dirty | 3826.11 ms | 1217.04 ms | -2609.07 ms |
| 57e0069+dirty | 3842.23 ms | 1210.00 ms | -2632.23 ms |
| 7ac3378+dirty | 1202.35 ms | 1198.31 ms | -4.04 ms |
| 20fbd51+dirty | 3832.52 ms | 1206.13 ms | -2626.39 ms |
| 822d35b+dirty | 3841.52 ms | 1221.75 ms | -2619.77 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3b6e9f9+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
| 4e0b819+dirty | 4.98 MiB | 6.46 MiB | 1.49 MiB |
| 1e5d96d+dirty | 4.98 MiB | 6.46 MiB | 1.49 MiB |
| 0a9e622+dirty | 4.98 MiB | 6.51 MiB | 1.53 MiB |
| 5ca03f9+dirty | 4.98 MiB | 6.53 MiB | 1.55 MiB |
| 40c9884+dirty | 4.98 MiB | 6.51 MiB | 1.53 MiB |
| 57e0069+dirty | 4.98 MiB | 6.50 MiB | 1.52 MiB |
| 7ac3378+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
| 20fbd51+dirty | 4.98 MiB | 6.46 MiB | 1.49 MiB |
| 822d35b+dirty | 4.98 MiB | 6.50 MiB | 1.53 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
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
The Metro serializer used
'map' in serializerResultto decide whether the wrapped serializer returned a{ code, map }bundle. BecauseArray.prototype.mapis inherited,'map' in anyArrayis alwaystrue— so when the wrapped serializer returns an array (as Expo's serializer does for static/EAS Update exports), the code took the bundle branch and returned{ code: undefined, map: undefined }.determineDebugIdFromBundleSource(undefined)then calledundefined.match(...)and crashed the bundler:Changes:
'map' in resultcheck with a positive one: the result is treated as a bundle only when it's a non-array object with a stringcode.{ code, map }bundle, return it untouched instead of crashing. Debug IDs for Expo static exports are handled by Expo's own serializer.💡 Motivation and Context
Fixes #6650. All
withSentryConfigusers doing Expo static / EAS Update (OTA) bundling hit this crash on@sentry/react-native8.23.0 and 8.24.0. The only workaround was disablingwithSentryConfig, which removes Debug ID injection and produces unreadable stack traces in Sentry.💚 How did you test it?
Added two regression tests in
sentryMetroSerializer.test.ts(a serializer returning an array, and one returning a promise that resolves to an array). Both reproduce the exactTypeErroronmainand pass with the fix. Full tools test suite passes (336 tests);yarn build:sdkand lint are clean.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
In the pass-through path the Sentry Debug ID module is still prepended and handed to the wrapped serializer (unchanged behavior), so Expo static bundles may contain that module with an unreplaced
__debug_id_place_holder__. It cannot be mis-detected as a real Debug ID (thesentry-dbid-<uuid>regex rejects it), but a follow-up should confirm against a liveeas updateexport whether that module should be suppressed when the output is non-standard.