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

Migrate away from Fabric Crashlytics #1321

Closed
Gudahtt opened this issue Feb 4, 2020 · 1 comment · Fixed by #1376
Closed

Migrate away from Fabric Crashlytics #1321

Gudahtt opened this issue Feb 4, 2020 · 1 comment · Fixed by #1376
Assignees
Labels
needs-qa Any New Features that needs a full manual QA prior to being added to a release.

Comments

@Gudahtt
Copy link
Member

Gudahtt commented Feb 4, 2020

The Fabric Crashlytics platform is shutting down on March 31st, as it has been replaced by/migrated to Firebase Crashlytics. We need to migrate to some other service for crash reporting (e.g. Firebase Crashlytics, Sentry, etc.)

@Gudahtt Gudahtt self-assigned this Feb 4, 2020
@omnat omnat added this to the Sprint Feb10 milestone Feb 10, 2020
Gudahtt added a commit that referenced this issue Feb 18, 2020
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.

Closes #1321
Gudahtt added a commit that referenced this issue Feb 20, 2020
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 `metemask-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
@danjm danjm added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Feb 24, 2020
Gudahtt added a commit that referenced this issue Feb 26, 2020
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 `metemask-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
@brat69
Copy link

brat69 commented Feb 27, 2020

Hi, I’m a newbie here and i need some help how to post or how this network woks I’m confused with this platform...

Gudahtt added a commit that referenced this issue Mar 6, 2020
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 `metemask-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
Gudahtt added a commit that referenced this issue Mar 23, 2020
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 `metemask-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
Gudahtt added a commit that referenced this issue Mar 24, 2020
* 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)
rickycodes pushed a commit that referenced this issue Jan 31, 2022
* 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-qa Any New Features that needs a full manual QA prior to being added to a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants