Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sentry metro bundle splitting - filename hash is wrong and source code does not point to correct module .js files #4317

Open
ricklove opened this issue Nov 27, 2024 · 9 comments
Assignees
Labels
Expo Issues related to Sentry React Native Expo support Platform: React-Native

Comments

@ricklove
Copy link

ricklove commented Nov 27, 2024

What React Native libraries do you use?

Expo Web, Expo Router

Are you using sentry.io or on-premise?

sentry.io (SaS)

@sentry/react-native SDK Version

6.3.0

How does your development environment look like?

"@sentry/react-native": "~6.3.0",
"expo": "^52.0.11",
"react": "18.3.1",
"react-native": "0.76.3",

Sentry.init()

Sentry.init({
dsn: process.env.EXPO_PUBLIC_APP_SENTRY_DSN,
debug: true,
});

Steps to Reproduce

Use React.lazy to dynamically load a module (that uses a default export):

const DynamicComponent = React.lazy(() => import('.example.tsx'));

This will enable bundle splitting. Now use getSentryExpoConfig instead of getDefaultConfig and the dynamic bundle will fail to import at runtime. (Everything builds and the initial index.js will load, but upon first loading a dynamic module, it fails because the module filename hash is wrong.)

The filenames for the bundles are changed, but internally the module code still tries to import the original paths.

Expected Result

The imported file names should match the new module filenames after sentry injected it's code.

Actual Result

The imported file names do not match the actual file names. The actual filenames saved in the dist folder is changed, but the imports still try to import the original filenames.

For example:

/_expo/static/js/web/icon-85ddfcb05c4f588fb9924ef24ca2aea9.js (This is imported)
changes to
/_expo/static/js/web/icon-1131b4f3e7922204435b95f36bc75f6e.js (This should be imported)

However, the module map still points to: "2125":"/_expo/static/js/web/icon-85ddfcb05c4f588fb9924ef24ca2aea9.js"

Also: the file contents had this prefixed:

var _sentryDebugIds,_sentryDebugIdIdentifier;void 0===_sentryDebugIds&&(_sentryDebugIds={});try{var stack=(new Error).stack;stack&&(_sentryDebugIds[stack]="9769af17-...",_sentryDebugIdIdentifier="sentry-dbid-....")}catch(e){}

Analysis:

Confirmed, commenting out this line in metro fixes this. This line updates filenames after unstable_beforeAssetSerializationPlugin is called.

https://github.com/expo/expo/blob/0d46fc187c55e6b73507f7e44f0d9e19c4893f64/packages/%40expo/metro-config/src/serializer/serializeChunks.ts#L403

@ricklove
Copy link
Author

This seems related, but it is not the same: #3663

@ricklove
Copy link
Author

This appears to be where it goes wrong:

export function unstable_beforeAssetSerializationPlugin({

@ricklove
Copy link
Author

In the latest metro code, it has something similar to this old PR:

https://github.com/expo/expo/pull/29054/files#diff-e51e0a447b1c87524c067bc171823cb544895a690991f436ea3fa1fc96351d38R280

That may be what introduced the bug.

@ricklove
Copy link
Author

@ricklove
Copy link
Author

ricklove commented Nov 27, 2024

This bug was introduced with this pr: expo/expo@a6e59b1

@ricklove
Copy link
Author

expo/expo#30980 @EvanBacon this PR seems to have broken sentry for bundle splitting: the filenames are saved with the new hash, however the internal imports still point to the original filenames.

This is a circular issues since updating the file would cause a cascade of needed updates through all the files following their dependencies.

@krystofwoldrich
Copy link
Member

Hi @ricklove,
thank you for the message and all the research you have done.

This makes is very easy for us to start looking into a solution.

@krystofwoldrich krystofwoldrich added Type: 🪲 Bug Expo Issues related to Sentry React Native Expo support labels Nov 28, 2024
@krystofwoldrich
Copy link
Member

The following change fixes the issue. But before opening a PR the implementation needs more testing.

expo/expo@main...krystofwoldrich:expo:kw/fix-async-import-with-unstable-premodules

@krystofwoldrich
Copy link
Member

Update: We have opened a PR to fix this is Expo.

@krystofwoldrich krystofwoldrich self-assigned this Dec 2, 2024
@krystofwoldrich krystofwoldrich moved this from Needs Discussion to Needs Review in Mobile & Cross Platform SDK Dec 2, 2024
EvanBacon pushed a commit to expo/expo that referenced this issue Dec 5, 2024
…s` from `stableChunkSource` (#33344)

# Why

<!--
Please describe the motivation for this PR, and link to relevant GitHub
issues, forums posts, or feature requests.
-->

Sentry user reported issue with using bundle splitting and
`beforeAssetSerializationPlugins` (used by Sentry Expo Metro Plugin).
The issue is the `stableChunkSource` becomes unstable due to updated
`premodules` by the `beforeAssetSerializationPlugins`

getsentry/sentry-react-native#4317

# How

<!--
How did you build this feature or fix this bug and why?
-->

This PR ensures the updated `premodules` are not affecting the chunk
global `premodules` but only the serialized output.

# Test Plan

<!--
Please describe how you tested this change and how a reviewer could
reproduce your test, especially if this PR does not include automated
tests! If possible, please also provide terminal output and/or
screenshots demonstrating your test/reproduction.
-->

I've added multiple scenarios to the tests to ensure this regression is
avoided in the future.

I also tried running the fix with
https://github.com/getsentry/sentry-react-native/tree/main/samples/expo
and the Web Release build is working with the fix. (Debug is not
affected, as premodules are changed only for release.)

# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [x] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [x] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [x] This diff will work correctly for `npx expo prebuild` & EAS Build
(eg: updated a module plugin).
tsapeta pushed a commit to expo/expo that referenced this issue Dec 5, 2024
…s` from `stableChunkSource` (#33344)

# Why

<!--
Please describe the motivation for this PR, and link to relevant GitHub
issues, forums posts, or feature requests.
-->

Sentry user reported issue with using bundle splitting and
`beforeAssetSerializationPlugins` (used by Sentry Expo Metro Plugin).
The issue is the `stableChunkSource` becomes unstable due to updated
`premodules` by the `beforeAssetSerializationPlugins`

getsentry/sentry-react-native#4317

# How

<!--
How did you build this feature or fix this bug and why?
-->

This PR ensures the updated `premodules` are not affecting the chunk
global `premodules` but only the serialized output.

# Test Plan

<!--
Please describe how you tested this change and how a reviewer could
reproduce your test, especially if this PR does not include automated
tests! If possible, please also provide terminal output and/or
screenshots demonstrating your test/reproduction.
-->

I've added multiple scenarios to the tests to ensure this regression is
avoided in the future.

I also tried running the fix with
https://github.com/getsentry/sentry-react-native/tree/main/samples/expo
and the Web Release build is working with the fix. (Debug is not
affected, as premodules are changed only for release.)

# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [x] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [x] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [x] This diff will work correctly for `npx expo prebuild` & EAS Build
(eg: updated a module plugin).
entiendoNull pushed a commit to entiendoNull/expo that referenced this issue Dec 13, 2024
…s` from `stableChunkSource` (expo#33344)

# Why

<!--
Please describe the motivation for this PR, and link to relevant GitHub
issues, forums posts, or feature requests.
-->

Sentry user reported issue with using bundle splitting and
`beforeAssetSerializationPlugins` (used by Sentry Expo Metro Plugin).
The issue is the `stableChunkSource` becomes unstable due to updated
`premodules` by the `beforeAssetSerializationPlugins`

getsentry/sentry-react-native#4317

# How

<!--
How did you build this feature or fix this bug and why?
-->

This PR ensures the updated `premodules` are not affecting the chunk
global `premodules` but only the serialized output.

# Test Plan

<!--
Please describe how you tested this change and how a reviewer could
reproduce your test, especially if this PR does not include automated
tests! If possible, please also provide terminal output and/or
screenshots demonstrating your test/reproduction.
-->

I've added multiple scenarios to the tests to ensure this regression is
avoided in the future.

I also tried running the fix with
https://github.com/getsentry/sentry-react-native/tree/main/samples/expo
and the Web Release build is working with the fix. (Debug is not
affected, as premodules are changed only for release.)

# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
This is required for changes to Expo modules.
-->

- [x] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [x] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [x] This diff will work correctly for `npx expo prebuild` & EAS Build
(eg: updated a module plugin).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Expo Issues related to Sentry React Native Expo support Platform: React-Native
Projects
Status: No status
Status: Needs Review
Development

No branches or pull requests

2 participants