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

Exit early on error (#4220) #4658

Merged
merged 6 commits into from Sep 29, 2022
Merged

Exit early on error (#4220) #4658

merged 6 commits into from Sep 29, 2022

Conversation

smartprogrammer93
Copy link
Contributor

Created from #4220

src/Nethermind/Nethermind.Init/Steps/InitializePlugins.cs Outdated Show resolved Hide resolved
}
catch (Exception)
{
Environment.ExitCode = -1;
Copy link
Member

Choose a reason for hiding this comment

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

Don't we want to have more complex exit code differentiation here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I sure don't want to add complexity. I'd recommend adding them when you need.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that recent change when calling Environment.Exit hangs. Added an IExceptionWithExitCode.

Copy link
Member

Choose a reason for hiding this comment

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

Isn't Exception.HResult potentially enough?

Copy link
Contributor

Choose a reason for hiding this comment

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

Did not realize that exist. it seems the range of that value is much largher. Probably not good for this use case.

Copy link
Member

Choose a reason for hiding this comment

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

Yes it might get trimmed, but when we control it it should be enough?

Copy link
Contributor

Choose a reason for hiding this comment

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

Dont know the semantic of that code.

src/Nethermind/Nethermind.Runner/Program.cs Outdated Show resolved Hide resolved
@dceleda
Copy link
Contributor

dceleda commented Sep 28, 2022

@asdacap please check Lukasz comments.

@dceleda
Copy link
Contributor

dceleda commented Sep 28, 2022

@jmederosalvarado this issue was raised some time ago by Lion. Do we want to include in the Gnosis branch or after the Gnosis merge?

@asdacap asdacap force-pushed the Exit-early-on-error-(#4220) branch 2 times, most recently from cb62a5a to a0d2fde Compare September 29, 2022 13:12
@asdacap asdacap merged commit cecab40 into master Sep 29, 2022
@asdacap asdacap deleted the Exit-early-on-error-(#4220) branch September 29, 2022 14:16
Andrew-Pohl pushed a commit to fuseio/nethermind that referenced this pull request Oct 7, 2022
* Exit early on error (NethermindEth#4220)

Exit early on error in steps except for step with "mustInitialize" set to false.

* Fix hanging on exit

* Fix exitcode not forwarded

* Rollback unintended commit

* Trying to invoke build

* Fix build

Co-authored-by: Amirul Ashraf <asdacap@gmail.com>
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.

None yet

4 participants