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/react-native - Hermes Support #649

Closed
HazAT opened this issue Sep 2, 2019 · 32 comments · Fixed by #749
Closed

@sentry/react-native - Hermes Support #649

HazAT opened this issue Sep 2, 2019 · 32 comments · Fixed by #749

Comments

@HazAT
Copy link
Member

HazAT commented Sep 2, 2019

The current version as of 1.0.1 we have very basic support for Hermes bundles meaning we are able to get function names but not showing the correct line number / source code for the crash.

In order for us to support it fully, we need to do some changes on how we process minified sources and source maps on the server.

Keep an eye on this issue if you are interested in this topic.

1.0.1

image

@HazAT HazAT self-assigned this Sep 2, 2019
@HazAT HazAT changed the title @sentry/hermes - 1.x.x - Hermes Support @sentry/react-native - 1.x.x - Hermes Support Sep 2, 2019
@HazAT HazAT changed the title @sentry/react-native - 1.x.x - Hermes Support @sentry/react-native - Hermes Support Sep 2, 2019
@HazAT HazAT pinned this issue Sep 2, 2019
@motiz88
Copy link

motiz88 commented Sep 2, 2019

@HazAT: Please let me know if there's anything we can do on the React Native side to make this easier.

@HazAT
Copy link
Member Author

HazAT commented Sep 3, 2019

@motiz88 Thanks again :)

Is there any way to output the minified js bundle that hermes uses internally?
The reason we need the minified code is that we try to get the function name from the crashed line and our symbolication code needs the minified bundle in order to do this correctly.

We initially thought the x_facebook_source is used for that but in our examples the value in there is always null.

@motiz88
Copy link

motiz88 commented Sep 3, 2019

x_facebook_sources is indeed designed for this (and is inherently more robust and complete than inspecting the bundled JS) so let's make it work. Can you share a complete example where it doesn't work for you?

@HazAT
Copy link
Member Author

HazAT commented Sep 3, 2019

@motiz88 https://github.com/HazAT/testhermes

@HazAT
Copy link
Member Author

HazAT commented Sep 5, 2019

@motiz88 Let me know if you need anything else.

@motiz88
Copy link

motiz88 commented Sep 9, 2019

@HazAT React Native 0.61 updates Metro (transitively via the RN CLI) to a version that populates x_facebook_sources. Could you test against that? We did not bump the Metro dependency in RN 0.60 because it had unrelated breaking changes.

@HazAT
Copy link
Member Author

HazAT commented Sep 9, 2019

@motiz88 there is no 0.61.0 in the RN CLI yet, but will try again

@HazAT
Copy link
Member Author

HazAT commented Sep 25, 2019

@motiz88 I can confirm with 0.61.0 x_facebook_sources is filled, let's see when we find time implementing this :)

@tobi512
Copy link

tobi512 commented Oct 17, 2019

Hi @HazAT, any timeline updates yet?
Would be great to see this as fast as possible since Hermes will drastically reduce our APK size and improve overall JS performance, two of our main pain points at the moment...

@filipef101
Copy link
Contributor

Will this mean react-native-v8 will be suported?

@haikov
Copy link

haikov commented Nov 5, 2019

Hello @HazAT!

Is there anything the community can help with? I think it's kind of a blocking issue for a lot of RN developers as it's impossible to use Sentry with Hermes now.

Could you please let us know how we can help or is there any timeline on fixing it from Sentry's side?

@HazAT
Copy link
Member Author

HazAT commented Nov 6, 2019

@gaykov Thank you for offering your help 🙏
Sadly adding this is not trivial, it requires mostly changes in sentry-cli which is written in rust.
Of course we gladly accept PRs but even I can’t tell you right now what exactly needs to be implemented to make this work.
What I can tell you; this is for sure on our roadmap but can’t give you any exact dates but I wouldn’t expect it in the upcoming weeks, realistic is Q1 2020.

@mitsuhiko
Copy link
Member

So a quick description of the problem from me (who built a lot of the current version of the source map processing) for why Hermes is different:

Under normal circumstances we use the source map and minified file to look up a location in the source map, then reverse tokenize from that location in the minified source to the last function declaration that matches the function name in the backtrace. Then we use the source map to map this back into the original token at that location. This is described in greater detail here: https://blog.sentry.io/2019/07/16/building-sentry-source-maps-and-their-problems

With Hermes this approach no longer works because Hermes doesn't actually have source maps or minified source code. Instead if has something similar to source maps where the column offset is a bytecode instruction. Since there is no minified source, there is nothing to step back on. Additionally it requires changes to how the filenames are processed in the stack traces because there is again no actual minified file that shows up there.

If someone is interested in helping out on the implementation of this I can go into further detail on discord and explain the current ideas I have for dealing with it.

@HazAT
Copy link
Member Author

HazAT commented Feb 5, 2020

Short update, @Swatinem did an amazing job, we have Hermes internally implemented and tested.
We need to bump dependencies and release everything, we will ship support for Hermes probably next week including using our new Android SDK, stay tuned.

#749

Kudos to @bruno-garcia @marandaneto

image

PR links:
getsentry/sentry-javascript#2406
#752
#754
getsentry/rust-sourcemap#22
getsentry/sentry-cli#646
getsentry/symbolic#187

@HazAT
Copy link
Member Author

HazAT commented Feb 12, 2020

1.3.0 has been released with Hermes support!
There aren't any breaking changes, please check and double-check to delete all caches to make sure you have to latest version.

Thank you all for your patience!

@desmondmc
Copy link

desmondmc commented Feb 14, 2020

@HazAT thanks for the updates! Maybe i'm doing something wrong but i just updated to 1.3.0 and i'm getting the following error when compiling: Do we need to have a minSdkVersion>=26 for this update to work?

> Transform artifact sentry-android-core.aar (io.sentry:sentry-android-core:2.0.0-rc04) with DexingNoClasspathTransform
AGPBI: {"kind":"error","text":"Invoke-customs are only supported starting with Android O (--min-api 26)","sources":[{}],"tool":"D8"}

> Transform artifact sentry-core.jar (io.sentry:sentry-core:2.0.0-rc04) with DexingNoClasspathTransform
AGPBI: {"kind":"error","text":"Default interface methods are only supported starting with Android N (--min-api 24): void io.sentry.core.IHub.addBreadcrumb(io.sentry.core.Breadcrumb)","sources":[{}],"tool":"D8"}

Works fine with "@sentry/react-native": "1.2.2"


Update:
Just needed to add the following to my app/build.gradle and now it works fine:

android {
    compileOptions {
        sourceCompatibility 1.8
        targetCompatibility 1.8
    }
    ...

@mxmzb
Copy link

mxmzb commented Feb 17, 2020

Looks like I have issues with the dependency io.sentry:sentry-android:2.0.0-rc04 here after the update. I'll investigate more in-depth tomorrow, meanwhile here is what Android SDK says:

org.gradle.api.internal.tasks.TaskDependencyResolveException: Could not determine the dependencies of task ':app:preDebugBuild'.
	at org.gradle.api.internal.tasks.CachingTaskDependencyResolveContext.getDependencies(CachingTaskDependencyResolveContext.java:68)
	at org.gradle.execution.plan.TaskDependencyResolver.resolveDependenciesFor(TaskDependencyResolver.java:46)
	at org.gradle.execution.plan.LocalTaskNode.getDependencies(LocalTaskNode.java:106)
	at org.gradle.execution.plan.LocalTaskNode.resolveDependencies(LocalTaskNode.java:79)
	at org.gradle.execution.plan.DefaultExecutionPlan.addEntryTasks(DefaultExecutionPlan.java:175)
	at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph.addEntryTasks(DefaultTaskExecutionGraph.java:139)
	at org.gradle.execution.TaskNameResolvingBuildConfigurationAction.configure(TaskNameResolvingBuildConfigurationAction.java:48)
	at org.gradle.execution.DefaultBuildConfigurationActionExecuter.configure(DefaultBuildConfigurationActionExecuter.java:57)
	at org.gradle.execution.DefaultBuildConfigurationActionExecuter.access$200(DefaultBuildConfigurationActionExecuter.java:26)
	at org.gradle.execution.DefaultBuildConfigurationActionExecuter$2.proceed(DefaultBuildConfigurationActionExecuter.java:63)
	at org.gradle.execution.DefaultTasksBuildExecutionAction.configure(DefaultTasksBuildExecutionAction.java:44)

... HUGE STACKTRACE GOING ON ...

org.gradle.api.internal.tasks.CachingTaskDependencyResolveContext.getDependencies(CachingTaskDependencyResolveContext.java:66)
	... 113 more
Caused by: org.gradle.internal.resolve.ModuleVersionNotFoundException: Could not find io.sentry:sentry-android:2.0.0-rc04.
Searched in the following locations:
  - file:/Users/maxim/.m2/repository/io/sentry/sentry-android/2.0.0-rc04/sentry-android-2.0.0-rc04.pom
  - file:/Users/maxim/.m2/repository/io/sentry/sentry-android/2.0.0-rc04/sentry-android-2.0.0-rc04.jar
  - file:/Volumes/Projects/goodtendency/packages/goodtendency-app/node_modules/react-native/android/io/sentry/sentry-android/2.0.0-rc04/sentry-android-2.0.0-rc04.pom
  - file:/Volumes/Projects/goodtendency/packages/goodtendency-app/node_modules/react-native/android/io/sentry/sentry-android/2.0.0-rc04/sentry-android-2.0.0-rc04.jar
  - file:/Volumes/Projects/goodtendency/packages/goodtendency-app/node_modules/jsc-android/dist/io/sentry/sentry-android/2.0.0-rc04/sentry-android-2.0.0-rc04.pom
  - file:/Volumes/Projects/goodtendency/packages/goodtendency-app/node_modules/jsc-android/dist/io/sentry/sentry-android/2.0.0-rc04/sentry-android-2.0.0-rc04.jar
  - https://dl.google.com/dl/android/maven2/io/sentry/sentry-android/2.0.0-rc04/sentry-android-2.0.0-rc04.pom
  - https://dl.google.com/dl/android/maven2/io/sentry/sentry-android/2.0.0-rc04/sentry-android-2.0.0-rc04.jar
  - https://jcenter.bintray.com/io/sentry/sentry-android/2.0.0-rc04/sentry-android-2.0.0-rc04.pom
  - https://jcenter.bintray.com/io/sentry/sentry-android/2.0.0-rc04/sentry-android-2.0.0-rc04.jar
  - https://jitpack.io/io/sentry/sentry-android/2.0.0-rc04/sentry-android-2.0.0-rc04.pom
  - https://jitpack.io/io/sentry/sentry-android/2.0.0-rc04/sentry-android-2.0.0-rc04.jar
Required by:
    project :app > project :@sentry_react-native
    project :app > project :sentry_react-native

@marandaneto
Copy link
Contributor

@mxmzb hey, we already have an issue for that, please look at #767 there's a workaround over there, thanks.

@bruno-garcia
Copy link
Member

@mxmzb we're merged packages on jcenter and that seems to have unlisted our package.
We're working on a fix. Please follow this thread:
getsentry/sentry-android#270

@marandaneto
Copy link
Contributor

it should be fixed :)

@capJavert
Copy link

capJavert commented Feb 22, 2020

I am still getting the same gibberish even after upgrading and uploading the new release with updated sourcemaps. Any tips if I am missing something? Is there a specific sentry server version needed to support this?

Things I did:

  • updated sentry
  • cleared android caches
  • updated versionCode
  • fresh build + upload to sentry

image

@marandaneto
Copy link
Contributor

@capJavert hey, it might be related to #772

@capJavert
Copy link

capJavert commented Feb 23, 2020

Not really, My sourcemaps are generated and uploaded and look fine but for some reason when erroris logged in Sentry its still like the image above.

@HazAT
Copy link
Member Author

HazAT commented Feb 24, 2020

@capJavert Can you please link to this issue on sentry.io?

@capJavert
Copy link

Nope, we use private instance of sentry inside the company. So the screenshot is from there. That is why I asked if some specific version of sentry server is required for this feature.

@HazAT
Copy link
Member Author

HazAT commented Feb 24, 2020

I am pretty sure we didn't release an official image containing support for Hermes. You need to use the latest master or wait for the new release (I can't tell you when this will be).

@mxmzb
Copy link

mxmzb commented Feb 26, 2020

I am pretty sure we didn't release an official image containing support for Hermes. You need to use the latest master or wait for the new release (I can't tell you when this will be).

But it says so in the release notes: https://github.com/getsentry/sentry-react-native/releases/tag/1.3.0

@bruno-garcia
Copy link
Member

bruno-garcia commented Feb 26, 2020

@HazAT was talking about Sentry. There are lots of changes on the backend to support this.
We've built it and deployed to sentry.io (we deploy multiple times a day to production). If you're running Sentry on-premise, you'll likely using our Docker files. Sentry now builds new images for each commit to master:

https://hub.docker.com/r/getsentry/sentry

You will need to update your on-prem installation to have the latest version of Sentry, and the latest SDK (seems you've got that covered).

@mxmzb
Copy link

mxmzb commented Feb 26, 2020

@bruno-garcia Oh, I see, thanks for clarifying!

@lorenzoangelini
Copy link

Hi @HazAT , Into the next release of react-native also in ios hermes will avaiable, react-native-sentry is ready for this?

@Swatinem
Copy link
Member

If the sourcemaps use a compatible format, its just a matter of tooling to make sure we use it for symbolication. Maybe file a new issue for this?

@getsentry getsentry locked as resolved and limited conversation to collaborators Dec 22, 2020
@jennmueng
Copy link
Member

Created an Issue to investigate and ensure we support this: #1262

@bruno-garcia bruno-garcia unpinned this issue Mar 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.