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 gas estimate errors #9025

Merged
merged 1 commit into from
Jul 16, 2020
Merged

Catch gas estimate errors #9025

merged 1 commit into from
Jul 16, 2020

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Jul 16, 2020

There were two cases where bad gas estimate data was resulting in crashes. These two places have been wrapped in a try ... catch to handle the absence of gas estimate data.

The errors are still reported to Sentry so that we can track down the root cause of this corrupted gas estimate data at some point in the future. We plan on adding additional context to Sentry soon that should help with this.

Fixes #8992

There were two cases where bad gas estimate data was resulting in
crashes. These two places have been wrapped in a `try ... catch` to
handle the absence of gas estimate data.

The errors are still reported to Sentry so that we can track down the
root cause of this corrupted gas estimate data at some point in the
future. We plan on adding additional context to Sentry soon that should
help with this.

Fixes #8992
@whymarrh
Copy link
Contributor

Is there any extra context we want to add/should add?

@whymarrh
Copy link
Contributor

I'm wondering if there's anything we could add that would help us diagnose this further

@Gudahtt
Copy link
Member Author

Gudahtt commented Jul 16, 2020

I was planning on separately re-adding the current Redux state to the Sentry context, minus anything big or identifying. That would include the gas and price estimates that are almost certainly responsible for this error.

@metamaskbot
Copy link
Collaborator

Builds ready [97484d6]
Page Load Metrics (606 ± 9 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint297737136
domContentLoaded577651605199
load578654606199
domInteractive576651604199

@Gudahtt Gudahtt marked this pull request as ready for review July 16, 2020 23:23
@Gudahtt Gudahtt requested a review from a team as a code owner July 16, 2020 23:23
@Gudahtt Gudahtt merged commit fd2e022 into develop Jul 16, 2020
@Gudahtt Gudahtt deleted the catch-time-estimate-errors branch July 16, 2020 23:29
@metamaskbot metamaskbot mentioned this pull request Jul 17, 2020
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.

v8 Speed up in queued tx
3 participants