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

Don't show stack trace for certain voluntary exit failure scenarios #7554

Merged
merged 4 commits into from
Oct 17, 2020

Conversation

rkapka
Copy link
Contributor

@rkapka rkapka commented Oct 16, 2020

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?

Stack trace will not be displayed if validator could not be exited because of the following non-fatal reasons:

  • validator has already exited
  • validator has not been active long enough to exit

Which issues(s) does this PR fix?

Fixes #7534

Other notes for review

N/A

@rkapka rkapka requested a review from a team as a code owner October 16, 2020 18:05
@rkapka rkapka changed the title handle some scenarios more gracefully Don't show stack trace for certain voluntary exit failure scenarios Oct 16, 2020
@rkapka rkapka added Ready For Review A pull request ready for code review Accounts labels Oct 16, 2020
@@ -171,6 +173,11 @@ this command outputs a deposit data string which is required to become a validat
Action: func(cliCtx *cli.Context) error {
featureconfig.ConfigureValidator(cliCtx)
if err := ExitAccountsCli(cliCtx, os.Stdin); err != nil {
msg := err.Error()
Copy link
Contributor

Choose a reason for hiding this comment

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

@rkapka let's use errors.Is instead of doing string comparisons

Copy link
Contributor Author

@rkapka rkapka Oct 17, 2020

Choose a reason for hiding this comment

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

errors.Is cannot be used here as I am not comparing errors. The exported messages are plain strings.

@codecov
Copy link

codecov bot commented Oct 17, 2020

Codecov Report

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

@@           Coverage Diff           @@
##           master    #7554   +/-   ##
=======================================
  Coverage   61.74%   61.74%           
=======================================
  Files         424      424           
  Lines       29944    29944           
=======================================
  Hits        18488    18488           
  Misses       8488     8488           
  Partials     2968     2968           

@prylabs-bulldozer prylabs-bulldozer bot merged commit f474c4b into master Oct 17, 2020
@delete-merged-branch delete-merged-branch bot deleted the remove-exit-error branch October 17, 2020 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate Exit Throws Error
2 participants