Skip to content

Fix path to Hermes build source map on RN 62.2#823

Closed
iagormoraes wants to merge 1 commit intogetsentry:masterfrom
iagormoraes:master
Closed

Fix path to Hermes build source map on RN 62.2#823
iagormoraes wants to merge 1 commit intogetsentry:masterfrom
iagormoraes:master

Conversation

@iagormoraes
Copy link
Copy Markdown

This PR fixes the build that is failing with Hermes enabled on RN 62.2 due to the source map file was renamed and also the generated path.

Issue Related:
#822

@iagormoraes iagormoraes requested a review from HazAT as a code owner April 18, 2020 15:41
@HazAT
Copy link
Copy Markdown
Member

HazAT commented May 12, 2020

Also here @marandaneto can we do a check if this file exists and then fall back to the old version to keep backward compatibility?

@marandaneto
Copy link
Copy Markdown
Contributor

marandaneto commented May 12, 2020

Also here @marandaneto can we do a check if this file exists and then fall back to the old version to keep backward compatibility?

I don't see any changes about that on the react-native repo:
https://github.com/facebook/react-native/commits/v0.62.2/react.gradle

That doesn't mean we don't have a bug though.

The intermediates folder isn't the final bundles, as stated in the comments:
https://github.com/getsentry/sentry-react-native/blob/master/sentry.gradle#L190-L200

I'd really double-check this before merging it, even if this is the right fix, we might need to add some extra code for keeping retro compatibility.

Afaik the tasks are executed at different times, when the path is gotten, the files don't exist yet, for this to be possible, it'd require bigger changes, refactoring.

@bruno-garcia
Copy link
Copy Markdown
Member

Sounds like this shouldn't be merged? Maybe @jennmueng has some input here too?

@jennmueng
Copy link
Copy Markdown
Contributor

Sounds like this shouldn't be merged? Maybe @jennmueng has some input here too?

Yeah I think we should leave it out for now. As @marandaneto said we might need to add something to ensure it works on prior versions.

@iagormoraes
Copy link
Copy Markdown
Author

any update on this situation?

@marandaneto
Copy link
Copy Markdown
Contributor

any update on this situation?

we believe this is not the right fix based on the previous comments, this change breaks other versions.
@jennmueng could you test RN 62.2 using Hermes + latest Sentry and see if you get source maps and stack traces demangled properly on release mode? thanks :)

@iagormoraes
Copy link
Copy Markdown
Author

any update on this situation?

we believe this is not the right fix based on the previous comments, this change breaks other versions.
@jennmueng could you test RN 62.2 using Hermes + latest Sentry and see if you get source maps and stack traces demangled properly on release mode? thanks :)

Without the patch, the build fails:

Execution failed for task ':app:bundleProdMinSdkProdKernelPreJsAndAssets_SentryUpload_2097435'.

error: No such file or directory (os error 2)

Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output.
Please attach the full debug log to all bug reports.

FAILURE: Build failed with an exception.

error: No such file or directory (os error 2)

Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output.
Please attach the full debug log to all bug reports.

FAILURE: Build failed with an exception.

Even using the latest version 1.5.0 I see in releases the errors the source map is there, but the stack trace it seems to not be working well:

Screen Shot 2020-07-02 at 10 12 45

Screen Shot 2020-07-02 at 10 10 51

Also all APK with distribution of 4xxxxxx handles all bundles and sourcemap from the others distributions:

Screen Shot 2020-07-02 at 10 22 16

See distribution 1xxxxxx, it doesn't have the sourcemap attached to it:
Screen Shot 2020-07-02 at 10 24 01

@marandaneto
Copy link
Copy Markdown
Contributor

marandaneto commented Jul 2, 2020

hey @iagormoraes what's about using the latest RN version? I meant, doing this (merging this PR), would break when using older RN version, RN itself and not our SDK.

could you enable --log-level=debug and copypaste the full log?
The same for export SENTRY_LOG_LEVEL=debug

This is a Java/Kotlin exception and source maps have nothing to do with it.

source maps are only for Javascript code.

stack trace it seems to not be working well

what do you mean here? stack traces look good to me, they are not minified, you probably don't have proguard enabled.

@iagormoraes
Copy link
Copy Markdown
Author

hey @iagormoraes what's about using the latest RN version? I meant, doing this (merging this PR), would break when using older RN version, RN itself and not our SDK.

could you enable --log-level=debug and copypaste the full log?
The same for export SENTRY_LOG_LEVEL=debug

This is a Java/Kotlin exception and source maps have nothing to do with it.

source maps are only for Javascript code.

stack trace it seems to not be working well

what do you mean here? stack traces look good to me, they are not minified, you probably don't have proguard enabled.

Hey @marandaneto, we are using RN 0.62.2 is the latest if I'm not mistaken, with hermes enabled, please follow below the debug log:

  DEBUG   2020-07-02 13:12:29.632721 -03:00 sentry-cli version: 1.55.0, platform: "darwin", architecture: "x86_64"
  INFO    2020-07-02 13:12:29.633870 -03:00 sentry-cli was invoked with the following command line: "/Users/iagormoraes/Desktop/projects/react-app/node_modules/@sentry/cli/sentry-cli" "react-native" "gradle" "--bundle" "/Users/iagormoraes/Desktop/projects/react-app/android/app/build/generated/assets/react/prodMinSdkProdKernel/pre/index.android.bundle" "--sourcemap" "/Users/iagormoraes/Desktop/projects/react-app/android/app/build/generated/sourcemaps/react/prodMinSdkProdKernel/pre/index.android.bundle.map" "--release" "com.app.pre@1.5.10-pre+3146011" "--dist" "3146011"
  INFO    2020-07-02 13:12:29.634908 -03:00 Issuing a command for Organization: rebellion Project: app
  INFO    2020-07-02 13:12:29.634941 -03:00   bundle path: /Users/iagormoraes/Desktop/projects/react-app/android/app/build/generated/assets/react/prodMinSdkProdKernel/pre/index.android.bundle
  INFO    2020-07-02 13:12:29.634949 -03:00   sourcemap path: /Users/iagormoraes/Desktop/projects/react-app/android/app/build/generated/sourcemaps/react/prodMinSdkProdKernel/pre/index.android.bundle.map
  DEBUG   2020-07-02 13:12:29.643532 -03:00 error: running update nagger
  DEBUG   2020-07-02 13:12:29.643551 -03:00 skipping update nagger because session is not attended
error: No such file or directory (os error 2)
  DEBUG   2020-07-02 13:12:29.644455 -03:00 client close; no transport to shut down  (from sentry)

Even with that patch, the sourcemap is not added to the correct distribution in releases as you can see below, if you think I have to open another issue let me know.

Screen Shot 2020-07-02 at 13 35 19

Screen Shot 2020-07-02 at 13 35 38

@regalstreak
Copy link
Copy Markdown

regalstreak commented Jul 4, 2020

Atleast compiles fine for me with react-native 0.62.2, android, hermes enabled

@fabiendem
Copy link
Copy Markdown

🤔 Been following this issue, quite weird because we use "@sentry/react-native": "1.4.5" with React native 0.62.2 and Hermes enabled without any problem.
The JS source map gets upload and the stack trace deobfuscated.

How do you apply the the gradle sentry plugin?
We do apply from: "../../node_modules/@sentry/react-native/sentry.gradle"

Client side, you must set the dist and release correctly when setting up Sentry, otherwise it won't pick up the correct source map at deobfuscation time.

@iagormoraes
Copy link
Copy Markdown
Author

iagormoraes commented Jul 6, 2020

🤔 Been following this issue, quite weird because we use "@sentry/react-native": "1.4.5" with React native 0.62.2 and Hermes enabled without any problem.
The JS source map gets upload and the stack trace deobfuscated.

How do you apply the the gradle sentry plugin?
We do apply from: "../../node_modules/@sentry/react-native/sentry.gradle"

Client side, you must set the dist and release correctly when setting up Sentry, otherwise it won't pick up the correct source map at deobfuscation time.

I am closing this PR, as @fabiendem said his setup works fine, maybe my project is with some configuration missing on the migration.

@iagormoraes
Copy link
Copy Markdown
Author

iagormoraes commented Jul 8, 2020

Doing some tests on build settings of hermes, I figured that when you override the hermesFlags it erases the option to output source map, to keep the source map output option add the following:

project.ext.react = [
    enableHermes: true,  // clean and rebuild if changing
    hermesFlagsRelease: ["-w", "-O", "-output-source-map"], // add this,
    hermesFlagsDebug: ["-w", "-O", "-output-source-map"], // add this,
    entryFile: "index.js",
    bundleInPre: true
]

The expected file will be there:

Screen Shot 2020-07-08 at 01 12 43

This will solve the issue.

@marandaneto
Copy link
Copy Markdown
Contributor

@iagormoraes good finding, thanks for sharing.

@regalstreak
Copy link
Copy Markdown

Thanks for sharing! This needs to be put up in the docs

@shiraz27
Copy link
Copy Markdown

shiraz27 commented Jun 12, 2024

@iagormoraes a more recent solution is :
hermesFlags = ["-O", "-output-source-map"]

if hermesFlagsRelease and hermesFlagsDebug do not work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants