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

Coerce non error objects into errors before sending to sentry #1874

Merged
merged 3 commits into from
Oct 13, 2020

Conversation

rickycodes
Copy link
Contributor

It seems like we're passing non errors to sentry

re: https://sentry.io/organizations/metamask/issues/1641747298/?project=2299799&query=is%3Aunresolved&statsPeriod=14d

I realise we should fix this underlying issue, but adding this will allow us to make mistakes without upsetting the Sentry gods
At least, that's my hope 🙏

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

Issue

Resolves #???

@rickycodes rickycodes requested a review from a team as a code owner October 3, 2020 05:47
@rickycodes rickycodes changed the title Coerce non error objects into errors Coerce non error objects into errors before sending to sentry Oct 4, 2020
@rickycodes rickycodes added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Oct 9, 2020
Copy link
Member

@andrepimenta andrepimenta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes a lot of sense to me, really important!
But should we also check if the exception is a string and put that string instead of 'error to capture is not an error instance'?
And also check if the exception is an object and do JSON.stringify and put that instead of 'error to capture is not an error instance'?

I think it would help us further differentiate errors.

It seems like we're passing non errors to sentry

re: https://sentry.io/organizations/metamask/issues/1641747298/?project=2299799&query=is%3Aunresolved&statsPeriod=14d

I realise we should fix this underlying issue, but adding this will allow us to make mistakes without upsetting the Sentry gods
At least, that's my hope 🙏
@rickycodes rickycodes force-pushed the bugfix/fix-sentry-logging-issue branch from d55ae15 to 3ef6780 Compare October 13, 2020 17:20
Copy link
Member

@andrepimenta andrepimenta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍

@rickycodes rickycodes merged commit 7adf92b into develop Oct 13, 2020
@rickycodes rickycodes deleted the bugfix/fix-sentry-logging-issue branch October 13, 2020 19:08
rickycodes added a commit that referenced this pull request Jan 31, 2022
* Coerce non error objects into errors

It seems like we're passing non errors to sentry

re: https://sentry.io/organizations/metamask/issues/1641747298/?project=2299799&query=is%3Aunresolved&statsPeriod=14d

I realise we should fix this underlying issue, but adding this will allow us to make mistakes without upsetting the Sentry gods
At least, that's my hope 🙏
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants