Skip to content

Commit

Permalink
Replace Fabric Crashlytics with Sentry (#1376)
Browse files Browse the repository at this point in the history
* Remove Fabric and Crashlytics

The Logger now logs to `console.log` exclusively.

* Add Sentry reporting

Errors are reported using `captureException`, and non-errors are added
as breadcrumbs. This means the non-error events won't be directly sent
to Sentry, but they will be included as context for any errors that do
occur.

The sentry configuration is mainly in the `sentry.debug.properties`
file. The only config outside of that file is the DSN used for
reporting errors, which is hard-coded in the `setupSentry.js` module.

There are three separate sets of config: default, debug, and release.
The default config is missing the auth token, so it can't be used to
publish source maps. However, builds that use the default config will
still correctly report errors to the `test-metamask-mobile` Sentry
project.

The debug config is identical to the default config except that it
includes the auth token. The debug config is for publishing any
non-production builds (e.g. testing, pre-releases).

The release configuration is to be used for production builds only.
This is the only config that sends errors to the `metamask-mobile`
Sentry project.

The debug and release config files have not been added to the repo.
They are automatically instantiated from the example files if the
`MM_SENTRY_AUTH_TOKEN` environment variable is set. Setting this
environment variable in CI should allow publishing from CI.

Closes #1321

* Use absolute path for Sentry file

The SENTRY_PROPERTIES environment variable is used at different
directory levels in the Android and iOS builds respectively; it's
used one level down with iOS, but two levels down for Android. This
means the properties path used is incorrect on iOS at the moment.

Instead an absolute path is now used, to prevent any such errors in the
future.

The error message for the missing auth token has been improved as well.

* Improve handling of missing auth token

The properties file is now checked to ensure the auth token is present,
so the build can fail early rather than partway through.

The logic for handling the auth token has been moved to a separate
function as well, so it can be shared between the release and
prerelease builds.

* Force setting METAMASK_ENVIRONMENT explicitly for release builds

The Sentry environment is set to `local` by default (e.g. for dev
builds), but the build script now requires it to be explicitly set for
release builds. This is to ensure it isn't missed in the production
builds, which are done manually.

* Ensure only Errors are thrown/rejected

Whenever using the `throw` keyword or calling `reject` in a Promise,
the value passed should be an Error. Various tools expect thrown
"errors" to be errors; this is the convention. Using Error types is
also more useful for debugging, as they have stack traces.

* Update `Logger.error` function signature

`Logger.error` now expects an actual error object as the first
argument. This allows us to submit proper Errors with stack traces to
Sentry. The second parameter is an optional set of extra data, which
gets attached to the Error and is viewable on Sentry in the "Additional
Data" section of the error report.

Unfortunately any messages added to contextualize errors had to be
submitted as extra data instead of as a wrapper Error. Wrapper error
types provide for a richer debugging experience in general, because
you get additional context for the error plus an additional stack trace
to help trace the flow of the error. Sentry even has an Integration
meant to facilitate this pattern, the `LinkedErrors` integration.
But unfortunately that integration doesn't seem to work with React
Native, so that option was off the table.

* Update `@sentry` packages to latest

The most recent Sentry packages have better support for capturing
native crashes on Android. I had held off on this update at first
because I thought it required React Native >= 0.60, but apparently
not. It still works on 0.59, just not as well (no sourcemaps)
  • Loading branch information
Gudahtt authored Mar 24, 2020
1 parent 201cd7e commit 2375f80
Show file tree
Hide file tree
Showing 64 changed files with 484 additions and 1,361 deletions.
1 change: 0 additions & 1 deletion .android.env.example
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
export MM_FOX_CODE=
export MM_FABRIC_API_KEY=
export MM_BRANCH_KEY_TEST=
export MM_BRANCH_KEY_LIVE=
export MM_MIXPANEL_TOKEN=
4 changes: 2 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ jobs:
name: build:pre-release
command:
|
yarn build:android:pre-release:bundle
METAMASK_ENVIRONMENT='prerelease' yarn build:android:pre-release:bundle
- store_artifacts:
path: android/app/build/outputs/bundle/release
destination: bundle
Expand All @@ -147,7 +147,7 @@ jobs:
at: .
- run:
name: pre-release
command: yarn build:ios:pre-release
command: METAMASK_ENVIRONMENT='prerelease' yarn build:ios:pre-release
- store_artifacts:
path: sourcemaps/ios
destination: sourcemaps-ios
Expand Down
6 changes: 4 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ app/bin
.gradle
local.properties
*.iml
android/app/src/main/assets/crashlytics-build.properties
android/app/src/main/res/values/com_crashlytics_export_strings.xml
android/.project
android/app/.project
android/app/bin/
Expand Down Expand Up @@ -65,5 +63,9 @@ coverage
.ios.env
.android.env

# Sentry
/sentry.debug.properties
/sentry.release.properties

# editor
.vscode
1 change: 0 additions & 1 deletion .ios.env.example
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
MM_FOX_CODE =
MM_FABRIC_API_KEY =
MM_BRANCH_KEY_TEST =
MM_BRANCH_KEY_LIVE =
6 changes: 3 additions & 3 deletions RELEASE.MD
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

2 - Bump the version number in `info.plist` and commit the change

3 - Run `npm run release:ios`
3 - Run `METAMASK_ENVIRONMENT='production' npm run release:ios`

4 - Wait for the appstore email that the build has completed processing (10 min - 1 hour)

Expand All @@ -26,7 +26,7 @@

2 - Save and commit

3 - Run `npm run release:android`
3 - Run `METAMASK_ENVIRONMENT='production' npm run release:android`

4 - Go to the playstore: https://play.google.com/apps/publish/?account=9089866037123936197

Expand All @@ -46,7 +46,7 @@


### Once you're done with both stores:
- Submit a PR with the changes
- Submit a PR with the changes
- Once it's merged create a tag on master for that version
- Go to the release pages and create a new release for that tag, including the changelog

54 changes: 8 additions & 46 deletions android/app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -24,31 +24,8 @@ def getPassword(String currentUser, String keyChain) {
stdout.toString().trim()
}

buildscript {
repositories {
maven { url 'https://maven.fabric.io/public' }
}

dependencies {
// These docs use an open ended version so that our plugin
// can be updated quickly in response to Android tooling updates

// We recommend changing it to the latest version from our changelog:
// https://docs.fabric.io/android/changelog.html#fabric-gradle-plugin
classpath 'io.fabric.tools:gradle:1.+'
}
}

apply plugin: "com.android.application"

apply plugin: 'io.fabric'


repositories {
maven { url 'https://maven.fabric.io/public' }
jcenter()
}

import com.android.build.OutputFile

/**
Expand Down Expand Up @@ -127,6 +104,13 @@ project.ext.react = [

apply from: "../../node_modules/react-native/react.gradle"

project.ext.sentryCli = [
logLevel: "debug",
sentryProperties: System.getenv('SENTRY_PROPERTIES') ? System.getenv('SENTRY_PROPERTIES') : '../../sentry.properties'
]

apply from: "../../node_modules/@sentry/react-native/sentry.gradle"

/**
* Set this to true to create two separate APKs instead of one:
* - An APK that only works on ARM devices
Expand All @@ -142,27 +126,8 @@ def enableSeparateBuildPerCPUArchitecture = false
*/
def enableProguardInReleaseBuilds = false

/**
*
* override fabric properties file if MM_FABRIC_API_KEY is set
*/
def buildFabricPropertiesIfNeeded() {
def FABRIC_API_KEY = System.getenv('MM_FABRIC_API_KEY')
if (FABRIC_API_KEY) {
def commentMessage = "AUTOGEN FABRIC PROPERTIES"
ant.propertyfile(file: "fabric.properties", comment: commentMessage) {
entry(key: "apiKey", value: FABRIC_API_KEY)
}
}
}

android {


afterEvaluate {
buildFabricPropertiesIfNeeded()
}

compileSdkVersion rootProject.ext.compileSdkVersion

compileOptions {
Expand Down Expand Up @@ -250,10 +215,10 @@ android {
}

dependencies {
implementation project(':@sentry_react-native')
implementation project(':react-native-sensors')
implementation project(':react-native-reanimated')
implementation project(':react-native-webview')
implementation project(':react-native-fabric')
implementation project(':@react-native-community_netinfo')
implementation project(':react-native-view-shot')
implementation project(':lottie-react-native')
Expand Down Expand Up @@ -285,9 +250,6 @@ dependencies {
implementation project(':react-native-vector-icons')
implementation 'com.mixpanel.android:mixpanel-android:5.+'

implementation('com.crashlytics.sdk.android:crashlytics:2.9.4@aar') {
transitive = true;
}
androidTestImplementation('com.wix:detox:+') { transitive = true }
androidTestImplementation 'junit:junit:4.12'
}
Expand Down
3 changes: 0 additions & 3 deletions android/app/fabric.properties

This file was deleted.

58 changes: 29 additions & 29 deletions android/app/src/main/java/io/metamask/MainApplication.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package io.metamask;

import com.facebook.react.ReactApplication;
import io.sentry.RNSentryPackage;
import com.sensors.RNSensorsPackage;
import com.swmansion.reanimated.ReanimatedPackage;
import com.reactnativecommunity.webview.RNCWebViewPackage;
import com.smixx.fabric.FabricPackage;
import com.reactnativecommunity.netinfo.NetInfoPackage;
import fr.greweb.reactnativeviewshot.RNViewShotPackage;
import com.airbnb.android.react.lottie.LottiePackage;
Expand Down Expand Up @@ -48,34 +48,34 @@ public boolean getUseDeveloperSupport() {

@Override
protected List<ReactPackage> getPackages() {
return Arrays.<ReactPackage>asList(
new MainReactPackage(),
new RNSensorsPackage(),
new ReanimatedPackage(),
new RNCWebViewPackage(),
new FabricPackage(),
new NetInfoPackage(),
new RNViewShotPackage(),
new LottiePackage(),
new AsyncStoragePackage(),
new ReactNativePushNotificationPackage(),
new BackgroundTimerPackage(),
new RNDeviceInfo(),
new SvgPackage(),
new RNGestureHandlerPackage(),
new RNScreensPackage(),
new RNBranchPackage(),
new KeychainPackage(),
new RandomBytesPackage(),
new RCTAesPackage(),
new RNCameraPackage(),
new RNFSPackage(),
new RNI18nPackage(),
new RNOSModule(),
new RNSharePackage(),
new VectorIconsPackage(),
new RCTAnalyticsPackage()
);
return Arrays.<ReactPackage>asList(
new MainReactPackage(),
new RNSentryPackage(),
new RNSensorsPackage(),
new ReanimatedPackage(),
new RNCWebViewPackage(),
new NetInfoPackage(),
new RNViewShotPackage(),
new LottiePackage(),
new AsyncStoragePackage(),
new ReactNativePushNotificationPackage(),
new BackgroundTimerPackage(),
new RNDeviceInfo(),
new SvgPackage(),
new RNGestureHandlerPackage(),
new RNScreensPackage(),
new RNBranchPackage(),
new KeychainPackage(),
new RandomBytesPackage(),
new RCTAesPackage(),
new RNCameraPackage(),
new RNFSPackage(),
new RNI18nPackage(),
new RNOSModule(),
new RNSharePackage(),
new VectorIconsPackage(),
new RCTAnalyticsPackage()
);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
import android.content.pm.PackageManager;
import android.util.Log;

import com.crashlytics.android.Crashlytics;
import com.crashlytics.android.core.CrashlyticsCore;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.ReactContextBaseJavaModule;
import com.facebook.react.bridge.ReactMethod;
Expand All @@ -23,8 +21,6 @@
import java.util.ArrayList;
import java.util.HashMap;

import io.fabric.sdk.android.Fabric;

public class RCTAnalytics extends ReactContextBaseJavaModule {

MixpanelAPI mixpanel;
Expand Down Expand Up @@ -63,8 +59,6 @@ public void optIn(boolean val) {

if(val){
this.mixpanel.optInTracking();
Fabric.with(this.getReactApplicationContext(), new Crashlytics());

}else{
this.mixpanel.optOutTracking();
}
Expand Down
14 changes: 0 additions & 14 deletions android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,3 @@ allprojects {
}
}
}

subprojects {project ->
if (project.name.contains('react-native-fabric')) {
buildscript {
repositories {
google()
jcenter()
maven {
url = 'https://dl.bintray.com/android/android-tools/'
}
}
}
}
}
4 changes: 2 additions & 2 deletions android/settings.gradle
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
rootProject.name = 'MetaMask'
include ':@sentry_react-native'
project(':@sentry_react-native').projectDir = new File(rootProject.projectDir, '../node_modules/@sentry/react-native/android')
include ':react-native-sensors'
project(':react-native-sensors').projectDir = new File(rootProject.projectDir, '../node_modules/react-native-sensors/android')
include ':react-native-reanimated'
project(':react-native-reanimated').projectDir = new File(rootProject.projectDir, '../node_modules/react-native-reanimated/android')
include ':react-native-webview'
project(':react-native-webview').projectDir = new File(rootProject.projectDir, '../node_modules/react-native-webview/android')
include ':react-native-fabric'
project(':react-native-fabric').projectDir = new File(rootProject.projectDir, '../node_modules/react-native-fabric/android')
include ':@react-native-community_netinfo'
project(':@react-native-community_netinfo').projectDir = new File(rootProject.projectDir, '../node_modules/@react-native-community/netinfo/android')
include ':react-native-view-shot'
Expand Down
4 changes: 2 additions & 2 deletions app/components/UI/AccountList/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ class AccountList extends PureComponent {
} catch (e) {
// Restore to the previous index in case anything goes wrong
this.mounted && this.setState({ selectedAccountIndex: previousIndex });
Logger.error('error while trying change the selected account', e); // eslint-disable-line
Logger.error(e, 'error while trying change the selected account'); // eslint-disable-line
}
InteractionManager.runAfterInteractions(() => {
setTimeout(() => {
Expand Down Expand Up @@ -217,7 +217,7 @@ class AccountList extends PureComponent {
this.mounted && this.setState({ orderedAccounts });
} catch (e) {
// Restore to the previous index in case anything goes wrong
Logger.error('error while trying to add a new account', e); // eslint-disable-line
Logger.error(e, 'error while trying to add a new account'); // eslint-disable-line
this.mounted && this.setState({ loading: false });
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ class TransactionDetails extends PureComponent {
}
} catch (e) {
// eslint-disable-next-line no-console
Logger.error(`can't get a block explorer link for network `, networkID, e);
Logger.error(e, { message: `can't get a block explorer link for network `, networkID });
}
};

Expand Down
7 changes: 1 addition & 6 deletions app/components/UI/UrlAutocomplete/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { connect } from 'react-redux';
import WebsiteIcon from '../WebsiteIcon';
import { colors, fontStyles } from '../../../styles/common';
import { getHost } from '../../../util/browser';
import Logger from '../../../util/Logger';

const styles = StyleSheet.create({
wrapper: {
Expand Down Expand Up @@ -130,11 +129,7 @@ class UrlAutocomplete extends PureComponent {
}

updateResults(results) {
try {
this.mounted && this.setState({ results });
} catch (e) {
Logger.error('Autocomplete crash', results);
}
this.mounted && this.setState({ results });
}

onSubmitInput = () => this.props.onSubmit(this.props.input);
Expand Down
2 changes: 1 addition & 1 deletion app/components/Views/Browser/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ class Browser extends PureComponent {
resolve(true);
},
error => {
Logger.error(`Error saving tab ${url}`, error);
Logger.error(error, `Error saving tab ${url}`);
reject(error);
}
);
Expand Down
Loading

0 comments on commit 2375f80

Please sign in to comment.