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

catch infura/metamask errors before they alert sentry #5113

Merged
merged 3 commits into from
Sep 4, 2019

Conversation

danlipert
Copy link
Contributor

Description

We use a variety of techniques in our shared.js to determine whether the user has web3 access. One of the ways we do it is to attempt to access the coinbase of the connected ETH node - if the coinbase comes back as something other than undefined, then we assume we have a working blockchain connection. However, when calling this synchronous method in browsers that do not have metamask unlocked, we get an error trying to access the coinbase, and use this to show the user the "unlock metamask" alert. Each time this happens however, an alert is sent to sentry.

Refers/Fixes

This PR catches the error.

Testing

Untested

octavioamu
octavioamu previously approved these changes Sep 4, 2019
thelostone-mc
thelostone-mc previously approved these changes Sep 4, 2019
@danlipert danlipert dismissed stale reviews from thelostone-mc and octavioamu via 848cfaf September 4, 2019 14:06
@codecov
Copy link

codecov bot commented Sep 4, 2019

Codecov Report

Merging #5113 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5113   +/-   ##
=======================================
  Coverage   31.03%   31.03%           
=======================================
  Files         218      218           
  Lines       17537    17537           
  Branches     2414     2414           
=======================================
  Hits         5442     5442           
  Misses      11872    11872           
  Partials      223      223

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eefca82...848cfaf. Read the comment docs.

@codecov
Copy link

codecov bot commented Sep 4, 2019

Codecov Report

Merging #5113 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5113   +/-   ##
=======================================
  Coverage   30.87%   30.87%           
=======================================
  Files         219      219           
  Lines       17649    17649           
  Branches     2435     2435           
=======================================
  Hits         5449     5449           
+ Misses      11977    11975    -2     
- Partials      223      225    +2
Impacted Files Coverage Δ
app/dashboard/views.py 13.96% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff37104...f6d6ec4. Read the comment docs.

@thelostone-mc thelostone-mc merged commit 185b0a5 into master Sep 4, 2019
@thelostone-mc thelostone-mc deleted the json-sentry-undefined branch June 27, 2020 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants