From 3d7653415c83b78ff5000dd01bc09f1c92d502a8 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Fri, 16 Feb 2018 09:49:18 +0100 Subject: [PATCH 1/3] Revert "fix: Fixing uncatchable, crashing, exception on Android (#355)" This reverts commit 453fe1ccc36a10b6c9c30fb48d749864aa4ab49a. --- .../main/java/io/sentry/RNSentryModule.java | 13 +------ lib/NativeClient.js | 37 +++++++------------ lib/Sentry.js | 3 +- 3 files changed, 17 insertions(+), 36 deletions(-) diff --git a/android/src/main/java/io/sentry/RNSentryModule.java b/android/src/main/java/io/sentry/RNSentryModule.java index 3dfbeb2a41..2aa66489cf 100644 --- a/android/src/main/java/io/sentry/RNSentryModule.java +++ b/android/src/main/java/io/sentry/RNSentryModule.java @@ -7,7 +7,6 @@ import com.facebook.react.ReactApplication; import com.facebook.react.bridge.Arguments; -import com.facebook.react.bridge.Callback; import com.facebook.react.bridge.Promise; import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.ReactContextBaseJavaModule; @@ -91,20 +90,13 @@ public Map getConstants() { } @ReactMethod - public void startWithDsnString(String dsnString, final ReadableMap options, Callback successCallback, Callback errorCallback) { + public void startWithDsnString(String dsnString, final ReadableMap options) { if (sentryClient != null) { logger.info(String.format("Already started, use existing client '%s'", dsnString)); return; } - try { - sentryClient = Sentry.init(dsnString, new AndroidSentryClientFactory(this.getReactApplicationContext())); - } catch (Exception e) { - logger.info(String.format("Catching on startWithDsnString, calling callback" + e.getMessage())); - errorCallback.invoke(e.getMessage()); - return; - } - + sentryClient = Sentry.init(dsnString, new AndroidSentryClientFactory(this.getReactApplicationContext())); androidHelper = new AndroidEventBuilderHelper(this.getReactApplicationContext()); sentryClient.addEventSendCallback(new EventSendCallback() { @Override @@ -151,7 +143,6 @@ public boolean shouldSend(Event event) { } }); logger.info(String.format("startWithDsnString '%s'", dsnString)); - successCallback.invoke(); } @ReactMethod diff --git a/lib/NativeClient.js b/lib/NativeClient.js index ac2213f983..a848317c3f 100644 --- a/lib/NativeClient.js +++ b/lib/NativeClient.js @@ -54,31 +54,22 @@ export class NativeClient { deactivateStacktraceMerging: true }; Object.assign(this.options, options); - } - async customInit() { - return new Promise((resolve, reject) => { - RNSentry.startWithDsnString(this._dsn, this.options, () => { - resolve(); - }, (err) => { - reject(err); + RNSentry.startWithDsnString(this._dsn, this.options); + if (this.options.deactivateStacktraceMerging === false) { + this._activateStacktraceMerging(); + const eventEmitter = new NativeEventEmitter(RNSentryEventEmitter); + eventEmitter.addListener(RNSentryEventEmitter.MODULE_TABLE, moduleTable => { + try { + this._updateIgnoredModules(JSON.parse(moduleTable.payload)); + } catch (e) { + // https://github.com/getsentry/react-native-sentry/issues/241 + // under some circumstances the the JSON is not valid + // the reason for this is yet to be found + } }); - - if (this.options.deactivateStacktraceMerging === false) { - this._activateStacktraceMerging(); - const eventEmitter = new NativeEventEmitter(RNSentryEventEmitter); - eventEmitter.addListener(RNSentryEventEmitter.MODULE_TABLE, moduleTable => { - try { - this._updateIgnoredModules(JSON.parse(moduleTable.payload)); - } catch (e) { - // https://github.com/getsentry/react-native-sentry/issues/241 - // under some circumstances the the JSON is not valid - // the reason for this is yet to be found - } - }); - } - RNSentry.setLogLevel(options.logLevel); - }); + } + RNSentry.setLogLevel(options.logLevel); } nativeCrash() { diff --git a/lib/Sentry.js b/lib/Sentry.js index f61e56de6a..683198135f 100644 --- a/lib/Sentry.js +++ b/lib/Sentry.js @@ -21,7 +21,7 @@ export const SentryLog = { }; export const Sentry = { - async install() { + install() { // We have to first setup raven otherwise react-native will freeze the options // and some properties like ignoreErrors can not be mutated by raven-js Sentry._ravenClient = new RavenClient(Sentry._dsn, Sentry.options); @@ -31,7 +31,6 @@ export const Sentry = { Sentry.options.disableNativeIntegration === false ) { Sentry._nativeClient = new NativeClient(Sentry._dsn, Sentry.options); - await Sentry._nativeClient.customInit(); Sentry.eventEmitter = new NativeEventEmitter(RNSentryEventEmitter); Sentry.eventEmitter.addListener( RNSentryEventEmitter.EVENT_SENT_SUCCESSFULLY, From 0e0300dee3c1ae73064f780d98385184dcc99a71 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Fri, 16 Feb 2018 10:13:24 +0100 Subject: [PATCH 2/3] fix: Use promises instead of error callback for install --- .../main/java/io/sentry/RNSentryModule.java | 12 +++++-- examples | 2 +- ios/RNSentry.m | 8 ++++- lib/NativeClient.js | 33 ++++++++++--------- lib/Sentry.js | 17 ++++++++-- 5 files changed, 50 insertions(+), 22 deletions(-) diff --git a/android/src/main/java/io/sentry/RNSentryModule.java b/android/src/main/java/io/sentry/RNSentryModule.java index 2aa66489cf..3903a1c2b7 100644 --- a/android/src/main/java/io/sentry/RNSentryModule.java +++ b/android/src/main/java/io/sentry/RNSentryModule.java @@ -90,13 +90,20 @@ public Map getConstants() { } @ReactMethod - public void startWithDsnString(String dsnString, final ReadableMap options) { + public void startWithDsnString(String dsnString, final ReadableMap options, Promise promise) { if (sentryClient != null) { logger.info(String.format("Already started, use existing client '%s'", dsnString)); return; } - sentryClient = Sentry.init(dsnString, new AndroidSentryClientFactory(this.getReactApplicationContext())); + try { + sentryClient = Sentry.init(dsnString, new AndroidSentryClientFactory(this.getReactApplicationContext())); + } catch (Exception e) { + logger.info(String.format("Catching on startWithDsnString, calling callback" + e.getMessage())); + promise.reject("SentryError", "Error during init", e); + return; + } + androidHelper = new AndroidEventBuilderHelper(this.getReactApplicationContext()); sentryClient.addEventSendCallback(new EventSendCallback() { @Override @@ -143,6 +150,7 @@ public boolean shouldSend(Event event) { } }); logger.info(String.format("startWithDsnString '%s'", dsnString)); + promise.resolve(true); } @ReactMethod diff --git a/examples b/examples index d6c2dd55d4..4c2b7b1192 160000 --- a/examples +++ b/examples @@ -1 +1 @@ -Subproject commit d6c2dd55d4822d1219b2c0abad7e5d59124ff597 +Subproject commit 4c2b7b1192566f7c1d9c6ce52a825a4116ba460e diff --git a/ios/RNSentry.m b/ios/RNSentry.m index a694992e0d..8d7a139963 100644 --- a/ios/RNSentry.m +++ b/ios/RNSentry.m @@ -125,7 +125,10 @@ - (void)setReleaseVersionDist:(SentryEvent *)event { resolve(crashedLastLaunch); } -RCT_EXPORT_METHOD(startWithDsnString:(NSString * _Nonnull)dsnString options:(NSDictionary *_Nonnull)options) +RCT_EXPORT_METHOD(startWithDsnString:(NSString * _Nonnull)dsnString + options:(NSDictionary *_Nonnull)options + resolve:(RCTPromiseResolveBlock)resolve + rejecter:(RCTPromiseRejectBlock)reject) { NSError *error = nil; self.moduleMapping = [[NSMutableDictionary alloc] init]; @@ -152,7 +155,10 @@ - (void)setReleaseVersionDist:(SentryEvent *)event { [SentryClient.sharedClient startCrashHandlerWithError:&error]; if (error) { [NSException raise:@"SentryReactNative" format:@"%@", error.localizedDescription]; + reject(@"SentryReactNative", error.localizedDescription, error); + return; } + resolve(@YES); } RCT_EXPORT_METHOD(activateStacktraceMerging:(RCTPromiseResolveBlock)resolve diff --git a/lib/NativeClient.js b/lib/NativeClient.js index a848317c3f..39e508aee0 100644 --- a/lib/NativeClient.js +++ b/lib/NativeClient.js @@ -54,22 +54,25 @@ export class NativeClient { deactivateStacktraceMerging: true }; Object.assign(this.options, options); + } - RNSentry.startWithDsnString(this._dsn, this.options); - if (this.options.deactivateStacktraceMerging === false) { - this._activateStacktraceMerging(); - const eventEmitter = new NativeEventEmitter(RNSentryEventEmitter); - eventEmitter.addListener(RNSentryEventEmitter.MODULE_TABLE, moduleTable => { - try { - this._updateIgnoredModules(JSON.parse(moduleTable.payload)); - } catch (e) { - // https://github.com/getsentry/react-native-sentry/issues/241 - // under some circumstances the the JSON is not valid - // the reason for this is yet to be found - } - }); - } - RNSentry.setLogLevel(options.logLevel); + async install() { + return RNSentry.startWithDsnString(this._dsn, this.options).then(() => { + if (this.options.deactivateStacktraceMerging === false) { + this._activateStacktraceMerging(); + const eventEmitter = new NativeEventEmitter(RNSentryEventEmitter); + eventEmitter.addListener(RNSentryEventEmitter.MODULE_TABLE, moduleTable => { + try { + this._updateIgnoredModules(JSON.parse(moduleTable.payload)); + } catch (e) { + // https://github.com/getsentry/react-native-sentry/issues/241 + // under some circumstances the the JSON is not valid + // the reason for this is yet to be found + } + }); + } + RNSentry.setLogLevel(this.options.logLevel); + }); } nativeCrash() { diff --git a/lib/Sentry.js b/lib/Sentry.js index 683198135f..a72aeae0ea 100644 --- a/lib/Sentry.js +++ b/lib/Sentry.js @@ -43,9 +43,20 @@ export const Sentry = { if (Sentry._internalEventStored) Sentry._internalEventStored(); }); } - // We need to call install here since this add the callback for sending events - // over the native bridge - Sentry._ravenClient.install(); + if (Sentry._nativeClient) { + Sentry._nativeClient + .install() + .then(() => { + Sentry._ravenClient.install(); + }) + .catch(error => { + throw error; + }); + } else { + // We need to call install here since this add the callback for sending events + // over the native bridge + Sentry._ravenClient.install(); + } }, config(dsn, options) { From e23ac61af5bddca6b05e6b5286e900dd530cd4ed Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Fri, 16 Feb 2018 10:51:57 +0100 Subject: [PATCH 3/3] fix: Clone obj before sending it over bridge Fixes #347 --- lib/NativeClient.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/NativeClient.js b/lib/NativeClient.js index 39e508aee0..c792536154 100644 --- a/lib/NativeClient.js +++ b/lib/NativeClient.js @@ -75,24 +75,28 @@ export class NativeClient { }); } + _cloneObject(obj) { + return JSON.parse(JSON.stringify(obj)); + } + nativeCrash() { RNSentry.crash(); } captureEvent(event) { - RNSentry.captureEvent(event); + RNSentry.captureEvent(this._cloneObject(event)); } setUserContext(user) { - RNSentry.setUser(user); + RNSentry.setUser(this._cloneObject(user)); } setTagsContext(tags) { - RNSentry.setTags(tags); + RNSentry.setTags(this._cloneObject(tags)); } setExtraContext(extra) { - RNSentry.setExtra(extra); + RNSentry.setExtra(this._cloneObject(extra)); } addExtraContext(key, value) { @@ -100,7 +104,7 @@ export class NativeClient { } captureBreadcrumb(breadcrumb) { - RNSentry.captureBreadcrumb(breadcrumb); + RNSentry.captureBreadcrumb(this._cloneObject(breadcrumb)); } clearContext() {