This repository has been archived by the owner on Sep 14, 2020. It is now read-only.
Collect version information in device.runtimeVersions #345
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fractalwrench
changed the title
kattrali
reviewed
Apr 29, 2019
cocoa/BugsnagReactNative.m
Outdated
| [config addBeforeSendBlock:^bool(NSDictionary *_Nonnull rawEventData, | ||
| BugsnagCrashReport *_Nonnull report) { | ||
| report.device = [self addDeviceRuntimeVersion:report.device]; |
There was a problem hiding this comment.
This will append the runtime version of the app when sending the report, which isn't necessarily the same as when capturing the crash.
…ccount for fatal native crashes
kattrali
previously requested changes
May 7, 2019
fractalwrench
dismissed
kattrali’s stale review
Collect version information in device.runtimeVersions (#345)
|
I've updated bugsnag-android and bugsnag-cocoa to the latest release, and also cached the React Native version on iOS as requested. |
kattrali
approved these changes
May 7, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goal
We should collect the React Native version in
device.runtimeVersions.reactNativefor all error reports and sessions.Note: this PR relies on the following Android and Cocoa PRs. The Cocoa changes have been vendored in; the Android changes require installing an artefact to mavenLocal in the usual way.
Changeset
runtime-versions-hooksbranchbeforeSendSessioncallback to both Cocoa/Android which adds the react native version to the session payloadbeforeSendblock/callback which adds the react native version to the error payloadDiscussion
There are a couple of important things to note with this approach:
We use a
beforeSendcallback on React Native rather than abeforeNotifycallback because this is the only callback mechanism that is guaranteed to be invoked when a fatal error occurs in the NDK.Ideally we would specify callbacks in a configuration parameter that is passed into a Bugsnag constructor; however, this is not possible with the current architecture of bugsnag-react-native. Instead we specify callbacks after constructing Bugsnag, which means there is a chance that some sessions could be missing runtime version information. This is a known issue with React Native configuration in general, and is something that we should address as part of a general revamp of the notifier.
Tests
Manually tested that the field was present in the payload for an error/session on both Android/Cocoa.
Mazerunner scenarios will be added in a separate PR to aid review.