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

Doesn't Catch Native Exceptions before the Javascript Runs #542

Closed
2 of 5 tasks
hhff opened this issue Feb 7, 2019 · 18 comments
Closed
2 of 5 tasks

Doesn't Catch Native Exceptions before the Javascript Runs #542

hhff opened this issue Feb 7, 2019 · 18 comments

Comments

@hhff
Copy link

hhff commented Feb 7, 2019

OS:

  • Windows
  • MacOS
  • Linux

Platform:

  • iOS
  • Android

Output of node -v && npm -v && npm ls --prod --depth=0

v10.8.0
6.7.0
[email protected] /Users/hhff/dev/light-two/LightOS
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── UNMET PEER DEPENDENCY [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected] invalid
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected] invalid
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
└── [email protected]

npm ERR! peer dep missing: react-native@^0.51.0, required by [email protected]
npm ERR! peer dep missing: react-native@^0.51, required by [email protected]
npm ERR! invalid: [email protected] /Users/hhff/dev/light-two/LightOS/node_modules/react-native-fs
npm ERR! extraneous: [email protected] /Users/hhff/dev/light-two/LightOS/node_modules/base-64
npm ERR! extraneous: [email protected] /Users/hhff/dev/light-two/LightOS/node_modules/utf8
npm ERR! invalid: [email protected] /Users/hhff/dev/light-two/LightOS/node_modules/react-native-telephony

Config:

Sentry.config('https://[email protected]/...', {
....
}).install()

I have following issue:

I'd like to use Sentry to capture information in early Android hooks like MainActivity#onCreate. Unfortunately, because this lib requires the JS to initialize the Native client over the bridge, those early hooks will no-op because it hasn't initialized yet.

Steps to reproduce:

  • Use this Lib
  • Add Sentry.capture("Hello!") to MainActivity#onCreate

Actual result:

  • The message doesn't make it to Sentry

Expected result:

  • The message should make it to Sentry

FWIW, my current work around is to init the client early in the MainApplication#onCreate. Then my JS runs, and re-inits the Sentry client through the Native Module API. Seems OK, but not ideal!

Thanks for all the hard work!

@HazAT
Copy link
Member

HazAT commented Feb 11, 2019

Yeah, this is correct.
The problem is that we need the DSN from JS in order to init the SDK on the native side.
So I am not sure if we are ever able to fix this.

@HazAT HazAT self-assigned this Feb 11, 2019
@hhff
Copy link
Author

hhff commented Feb 11, 2019

I was thinking that the DSN could be a native variable rather than a JS string, and that var could be passed back to the JS side if necessary as a const (React Native supports constants over the bridge).

IMO if your app is crashing from native exceptions before the JS runs, but not reporting them to Sentry, thats a big deal!

Is there any problem with the approach above? ie - instantiating the native client twice? Kinda feels like something this lib should do by default.

@HazAT
Copy link
Member

HazAT commented Feb 12, 2019

That could be an approach we could take, problem with this is where to set the DSN natively?!

I'll also be honest with you, this will probably not be changed in the near future.
We are currently discussing rewriting the whole SDK to be unified with our other SDKs https://github.com/getsentry/sentry-javascript

but it's not been decided yet. If we do, we'll think about how to handle this properly.

But in general I agree with what you are saying (how it should/could work).

@hhff
Copy link
Author

hhff commented Feb 12, 2019

Understood, thank you.

One last Q - Is there any problem with the approach above? ie - instantiating the native client twice?

Just wanna be sure that’s not going to cause some type of collision, etc.

@SebRenon
Copy link

Hi @HazAT
Can you maybe expose the Sentry-Android dependency so on Android we can instantiate Sentry before React Native is loaded?

// file: @sentry/react-native/android/build.gradle
dependencies {
    implementation 'com.facebook.react:react-native:+'
-   implementation 'io.sentry:sentry-android:1.7.23'
+   api 'io.sentry:sentry-android:1.7.23'
}

Then in our Android MainApplication.java we can do:

Sentry.init(sentryConfig.dsn, AndroidSentryClientFactory(applicationContext))

We are using @sentry/react-native v 1.0.5

@xwartz
Copy link

xwartz commented Feb 19, 2020

Any update on this?

@punksta
Copy link

punksta commented May 26, 2020

I am using

io.sentry:sentry-android-core:2.1.3
io.sentry:sentry-android-ndk:2.1.3
io.sentry:sentry-android:2.1.3

And faced with this problem. Sometimes an app crashes within java or binary code before js initialization.

I moved initialization to native level by adding the following:

AndroidManifest.xml
(inside application tag)

        <meta-data android:name="io.sentry.dsn" android:value="${SENTRY_DSN}" />
        <meta-data android:name="io.sentry.auto-init" android:value="true" tools:node="replace"/>
        <meta-data android:name="io.sentry.ndk.enable" android:value="true" />

in app/build.gradle

 defaultConfig {
        manifestPlaceholders = [
                SENTRY_DSN: "https://..."
        ]
     //....
   }

it is catching java errors even from Application::onCreate method.

I still keep init in js

Sentry.init({
  enabled: true,
  enableAutoSessionTracking: true,
  dsn: SENTRY_DSN,
});

which is probably initializing sdk second time. But looks like it's working fine.

Will post updates.

@sagarshakya
Copy link

@punksta Any updates on this? Did you stumble upon any issues when using this approach of initializing the SDK twice?

@punksta
Copy link

punksta commented Jul 10, 2020

@thesagarshakya I haven't noticed any major issues. The only issue I noticed is crashes from binary(c++) code are not linked to a release in the dashboard, but I guess it's a different issue.

@xwartz
Copy link

xwartz commented Oct 20, 2020

@HazAT Any updates on this? Another 1 year have gone by..

@jennmueng
Copy link
Member

We've been discussing possible solutions internally, this is a pretty complex and difficult issue and if we find a good solution we'll make it happen. But for now as a workaround you can initialize the SDKs twice, by this I mean calling .init on the Andrid/Cocoa SDK in your own code and it should work fine.

@bruno-garcia
Copy link
Member

We're planning to tackle this soon. Sorry for the delay

@jennmueng
Copy link
Member

With this PR #1259 merged, it's possible now to catch native errors before the JS code runs. You would have to manually initialize the SDK on the native layer yourself, and pass shouldInitializeNativeSdk: false to the options in JS.

@marandaneto
Copy link
Contributor

@jennmueng do you know how the RN engine gets started on Android? is it thru ContentProvider?

wondering if getsentry/sentry-dart#265 (comment) is also an issue for RN

@jennmueng
Copy link
Member

I'm not fully sure myself as I've not really worked with it, but I think it should started from the Application class.

@marandaneto
Copy link
Contributor

yeah just looked up inside of an old demo, RN creates an Application class that hosts a ReactNativeHost.
Also, MainActivity gets created extending a ReactActivity, so all the magic is happening there + native libs being loaded by SoLoader which is also a Facebook lib.

so yeah a few things could be caught by the SDK, but RN also has ContentProvider, See this https://github.com/facebook/react-native/blob/6e6443afd04a847ef23fb6254a84e48c70b45896/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobProvider.java

so what I've guessed is partly true.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 5, 2021

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@marandaneto
Copy link
Contributor

you can achieve that by manually initing the Native SDKs https://docs.sentry.io/platforms/react-native/manual-setup/native-init/

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

No branches or pull requests

9 participants