The problem with this is that programs often exit with a non-zero exit code even when they exit successfully. Some programs expect to be terminated by Ctrl-C. I think throwing errors in all these cases might produce too much noise.

We used to have a config setting for throwing errors on non-zero exit code, but I kept it off because it was too annoying.

We used to have a config setting for throwing errors on non-zero exit code, but I kept it off because it was too annoying.

I just stumbled across this PR, and I'd be curious what exactly made this annoying.
I assume the problem is that a ShellError would be reported like the image below, which would be annoying in the interactive use case as described in Sophie's post.

I think for exit codes > 0, you want to stop execution of a script, but don´t want a scary/annoying error message for your interactive use case every time you cancel git log.

I think the best way to get out of this is do define a specific Error which is only used when external commands return with an exit code > 0. That Error will not be reported in the interactive shell. In the script, nothing needs to change, it's easy to ignore errors with do

I don't remember the details, but the Sophia's article highlights the point well. E.g., after running git diff of a longer diff, you'd get a shell error reported in the REPL, that's annoying. There are also some tools that when you run them without any args, they print the help message to stderr and non-zero exit code (for example git diff outside a git repository). While technically this could be interpreted as an error, sometimes I run commands like that just to show the help message, and the error would be annoying.

Although, now it seems that code stops executing when encountering a non-zero exit code, in which case you probably do want to have an error, so the suggestion of having a separate Error for this case that gets ignored in REPL seems reasonable.

so the suggestion of having a separate Error for this case that gets ignored in REPL seems reasonable.

@IanManske is this something you'd be interested in working on?

so the suggestion of having a separate Error for this case that gets ignored in REPL seems reasonable.

@IanManske is this something you'd be interested in working on?

Yes, this is what I was thinking as well. I think it should not show by default in the REPL, but I would still like it to be configurable. So, I was wondering how to best structure the config, but I think I have a decent idea now. I'll have time on the weekend to hopefully knock this PR out.

Making the error showing / not showing in a REPL configurable sounds OK to me, but I wouldn't make stopping the execution in a script configurable because semantics shouldn't depend on the config. This might not be honored in 100% of the cases currently, but IMO we should strive to keep Nushell's execution as deterministic as possible.

One thing to consider is whether we want different behavior for the trailing pipeline position vs. for a pipeline element? Is the assumption correct that when you want to pipe things you want to probably handle the exit code vs at the last part of a statement where it may be a result in of itself?

@sholderbach Maybe others in this thread feel differently, but I struggle a bit to understand the situation you are describing. Could you maybe give one or two examples with code? :)

One thing to consider is whether we want different behavior for the trailing pipeline position vs. for a pipeline element?

Our current code structure makes it impossible to check the exit codes of pipeline elements besides the last, so this PR only checks the trailing element's exit code. One exception is if a command has to collect it's input, and the previous command was an external command. In that case, the exit code is checked as well.

Could you maybe give one or two examples with code? :)

My gut feeling was that when comparing a) or b)

# a)
foo | may-fail
# b)
foo | may-fail | bar

that the example b) encodes more of an expectation that things are designed to work together explicitly, so an unhandled exit code presents a true error even in the repl. While a) may present an error in the context of a script the semantics are less clear cut for the repl.

But as @IanManske mentioned that is not the primary focus of the current logic

Our current code structure makes it impossible to check the exit codes of pipeline elements besides the last

In that sense for b) the handling is inverse that only erroring when explicitly asked for through collect.

One exception is if a command has to collect it's input, and the previous command was an external command. In that case, the exit code is checked as well.

Thanks for the explanation @sholderbach

In that sense for b) the handling is inverse that only erroring when explicitly asked for through collect.

That is a bit unfortunate. At least for me that will mean that many errors will go unnoticed. But nothing that can be done about this in the short term I guess.

How does this PR work on non-zero exit codes that are not errors like mentioned here https://www.sophiajt.com/exit-codes/?

This PR makes it so that nonzero exit codes are treated as errors, so an error would be triggered in that example. In interactive mode / the REPL, no error message will be displayed by default though. If a non-zero exit code needs to be ignored/caught in, e.g., a script, then complete or try can be used (this is the same as in nushell today).

I'm ok with this PR as long as I don't get an error every time I run git log and similar commands that return non-zero exit codes when there isn't an error.

Ok, let's see how this goes.

This PR makes it so that nonzero exit codes are treated as errors, so an error would be triggered in that example. In interactive mode / the REPL, no error message will be displayed by default though. If a non-zero exit code needs to be ignored/caught in, e.g., a script, then complete or try can be used (this is the same as in nushell today).

@IanManske Is this still the case? I'm currently using nushell 0.98.0 in a terminal, and sure enough, git log followed by hitting q to quit it causes it to error:

Error: nu::shell::non_zero_exit_code

  × External command had a non-zero exit code
   ╭─[entry #25:1:1]
 1 │ git log
   · ─┬─
   ·  ╰── exited with code 128
   ╰────

Is there a config setting that I'm missing?

Edit: disregard, just read the changelog. There are new config settings $env.config.display_errors.exit_code and $env.config.display_errors.termination_signal outlined here: https://www.nushell.sh/blog/2024-09-17-nushell_0_98_0.html#non-zero-exit-codes-are-now-errors-toc

@wthueb Glad you found the setting. I just made a PR to highlight this in the release notes: nushell/nushell.github.io#1562

This option MUST be false without config value
It is very annoying to update nushell and stuff breaking apart from every minor PR like this
I do not have time to read every PR to see how you break stuff

This option MUST be false without config value It is very annoying to update nushell and stuff breaking apart from every minor PR like this I do not have time to read every PR to see how you break stuff

That's what the release notes are for. Nushell is still pre 1.0, so it is expected for there to be occansional breaking changes.

Anyways, I wouldn't be against making false the default value for REPL sessions (without config).

#13873 needs to be cleaned up for false to happen in the next release.