getsentry / sentry-react-native

Official Sentry SDK for React Native
https://sentry.io
MIT License
1.58k stars 338 forks source link

Fix concurrent runs with Sentry Babel transformer #4121

Open feidaZhang opened 1 month ago

feidaZhang commented 1 month ago

OS:

Platform:

SDK:

SDK version: 5.30.0

react-native version: 0.72.12

Are you using Expo?


I have the following issue:

After integrating withSentryConfig in the metro.config.js, annotateReactComponents is true, we found that when building iOS application through Xcode in debug mode, the .sentry/. defaultBabelTransformerPath file was first created and then deleted before the build was completed. However, when loading the JS Bundle, sentry tried to read the file again and threw an error.

Actual result:

Image

Expected result:

no error

feidaZhang commented 1 month ago

if i comment this line. There is no error but the file can not be deleted automatically.

function withSentryBabelTransformer(config) {
    const defaultBabelTransformerPath = config.transformer && config.transformer.babelTransformerPath;
    utils_1.logger.debug('Default Babel transformer path from `config.transformer`:', defaultBabelTransformerPath);
    if (!defaultBabelTransformerPath) {
        // This has to be console.warn because the options is enabled but won't be used
        // eslint-disable-next-line no-console
        console.warn('`transformer.babelTransformerPath` is undefined.');
        // eslint-disable-next-line no-console
        console.warn('Sentry Babel transformer cannot be used. Not adding it...');
        return config;
    }
    if (defaultBabelTransformerPath) {
        (0, sentryBabelTransformerUtils_1.saveDefaultBabelTransformerPath)(defaultBabelTransformerPath);
        process.on('exit', () => {
             // comment this line
            // (0, sentryBabelTransformerUtils_1.cleanDefaultBabelTransformerPath)();
        });
    }
    return Object.assign(Object.assign({}, config), { transformer: Object.assign(Object.assign({}, config.transformer), { babelTransformerPath: require.resolve('./sentryBabelTransformer') }) });
}
feidaZhang commented 1 month ago

It's my device info. Model Name: MacBook Pro Model Identifier: MacBookPro17,1 Model Number: Z11C000BPCH/A Chip: Apple M1

kahest commented 1 month ago

Thanks @feidaZhang for the report, we'll investigate this and follow up here

antonis commented 1 month ago

Hello @feidaZhang 👋 thank you for your report and the provided information. I debugged the issue with the reported configuration (SDK version: 5.30.0, react-native version: 0.72.12) but wasn't able to reproduce. Could you set the log level to debug: export SENTRY_LOG_LEVEL=debug and provide us with some extra logs from metro E.g.

Sentry Logger [debug]: Default Babel transformer path from `config.transformer`: [TRANSFORMER_PATH]
Sentry Logger [debug]: Saved default Babel transformer path
...
Sentry Logger [debug]: Loading default Babel transformer from [TRANSFORMER_PATH]
Sentry Logger [debug]: Cleaned default Babel transformer path
✨  Done in 7.23s.

Do you see the Cleaned default Babel transformer path before metro termination?

We also noticed that the provided screenshot is from the macOS app. Could you confirm if you are building the iOS or macOS app with XCode? Have you made any other customisations in your Xcode setup?

feidaZhang commented 1 month ago

Hello @feidaZhang 👋 thank you for your report and the provided information. I debugged the issue with the reported configuration (SDK version: 5.30.0, react-native version: 0.72.12) but wasn't able to reproduce. Could you set the log level to debug: export SENTRY_LOG_LEVEL=debug and provide us with some extra logs from metro E.g.

Sentry Logger [debug]: Default Babel transformer path from `config.transformer`: [TRANSFORMER_PATH]
Sentry Logger [debug]: Saved default Babel transformer path
...
Sentry Logger [debug]: Loading default Babel transformer from [TRANSFORMER_PATH]
Sentry Logger [debug]: Cleaned default Babel transformer path
✨  Done in 7.23s.

Do you see the Cleaned default Babel transformer path before metro termination?

We also noticed that the provided screenshot is from the macOS app. Could you confirm if you are building the iOS or macOS app with XCode? Have you made any other customisations in your Xcode setup?

Image hi, there are logs i got. I built the application with Xcode, i don't think i made some customisations on setup. One of my colleagues said he has no issue with Apple Intel. However, it happened on Apple M1.

If i set annotateReactComponents:false on metro.config.js, there is no issue also.

antonis commented 1 month ago

Thank you for sharing the debug logs @feidaZhang 🙇 My understanding from the last log (Cleaned default Babel transformer path) is that that the Metro server has stopped. Could you share more details on how you start the Metro server and if it is running when the error occurs?

feidaZhang commented 1 month ago

@antonis yeah, I also noticed Metro server has stopped. The metro server is still running and show the errors when it happens. I tried to start the metro before building using Xcode, I also tried to building the app using Xcode directly.However, I always get the errors. Did you build app with Debug executable on Xcode? I enabled it.

krystofwoldrich commented 1 month ago

Hi @feidaZhang, from the Xcode logs I see you are building debug version of your application, during debug builds Metro is by default not executed, as it's expected to load bundle from the running development server.

But in your case metro is executed, which causes deletion of the file and consequently the error in the development server running outside of Xcode.

Have you changed this behaviour intentionally, otherwise it might indicate indicate an error in the build phase.

krystofwoldrich commented 1 month ago

The current impl of .sentry/.defaultBabelTransformerPath doesn't work with multiple parallel build/dev server running.

krystofwoldrich commented 1 month ago

We can enable multiple parallel build simply just by scoping the ./sentry using timestamp or unique generated hash.

./sentry/${timestamp/hash}/.defaultBabelTransformerPath

johnf commented 2 weeks ago

I've noticed this directory and file started appearing in the last few weeks. Is it fine to add it to .gitignore?

krystofwoldrich commented 2 weeks ago

@johnf Yes, can be ignored, it's used for tmp files during the JS bundling process.

fediru-gloat commented 4 days ago

Also faced this issue and it's pretty annoying. I just tried to open development build from Expo cli by pressing "i" and it throws an error. It should probably generated in runtime on dev builds if it's missed or if it's missed it shouldn't block app start?