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

chore: let sentry capture panics when failed to verify the data from the storage #1525

Merged

Conversation

@yangby-cryptape
Copy link
Contributor

commented Sep 3, 2019

This week, sentry reported few panics that the data from the storage can not deserialize.
After reviewing the code, I think it should be impossible.
I added the hex string of the bytes to sentry messages. So, next time, we can figure it out.

@yangby-cryptape yangby-cryptape requested a review from nervosnetwork/ckb-code-review as a code owner Sep 3, 2019
@nervos-bot

This comment has been minimized.

Copy link

commented Sep 3, 2019

@TheWaWaR is assigned as the chief reviewer

)
},
);
panic!("verify slice should be ok, but {}", err);

This comment has been minimized.

Copy link
@doitian

doitian Sep 3, 2019

Member

panic! is enough, we don't need to send the sentry message manually.

This comment has been minimized.

Copy link
@yangby-cryptape

yangby-cryptape Sep 3, 2019

Author Contributor

DONE!

…the storage
@yangby-cryptape yangby-cryptape force-pushed the yangby-cryptape:pr/more-details-for-panic branch from 4e24c0b to abe0171 Sep 3, 2019
@doitian
doitian approved these changes Sep 3, 2019
@doitian

This comment has been minimized.

Copy link
Member

commented Sep 3, 2019

bors r+

@doitian doitian added this to 👀 Awaiting review in CKB Pull Requests Sep 3, 2019
@doitian doitian moved this from 👀 Awaiting review to ✅ Reviewer approved in CKB Pull Requests Sep 3, 2019
bors bot added a commit that referenced this pull request Sep 3, 2019
Merge #1525
1525: chore: let sentry capture panics when failed to verify the data from the storage r=doitian a=yangby-cryptape

This week, sentry reported few panics that the data from the storage can not deserialize.
After reviewing the code, I think it should be impossible.
I added the hex string of the bytes to sentry messages. So, next time, we can figure it out.

Co-authored-by: Boyu Yang <yangby@cryptape.com>
@bors

This comment has been minimized.

Copy link
Contributor

commented Sep 3, 2019

Build succeeded

  • continuous-integration/travis-ci/push
@bors bors bot merged commit abe0171 into nervosnetwork:develop Sep 3, 2019
5 checks passed
5 checks passed
Dummy CI CI that does nothing
Details
Travis CI - Pull Request Build Passed
Details
bors Build succeeded
Details
nervosnetwork.ckb Build #20190903.61 succeeded
Details
nervosnetwork.ckb (UnitTest) UnitTest succeeded
Details
CKB Pull Requests automation moved this from ✅ Reviewer approved to Done Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
4 participants
You can’t perform that action at this time.