search

qcscine / sparrow

https://scine.ethz.ch
BSD 3-Clause "New" or "Revised" License
78 stars 15 forks source link

Zero exit code on failure #12

Closed awvwgk closed 2 years ago

awvwgk commented 2 years ago

After finally getting to test Sparrow, I did find that the error reporting is not reliable, especially in case of wrong command-line options the exit status is not non-zero:

❯ sparrow --structure t.xyz --method pm7
Method PM7 is not available.

Sparrow flies away...
❯ echo $?
0

It would be preferable to signal failure via a non-zero exit status IMO.

nabbelbabbel commented 2 years ago

Staged for the next release. As far as I can tell errors in the input besides the method should result in an exception throw and an appropriate return status.

awvwgk commented 2 years ago

The current error handling with throwing exceptions seems a bit unclean, especially since it causes an ABRT signal.

❯ sparrow --version
terminate called after throwing an instance of 'boost::wrapexcept<boost::program_options::unknown_option>'
  what():  unrecognised option '--version'
zsh: abort (core dumped)  sparrow --version

Preferable would be to catch the exception and present the error message in a useful way to the user. I would usually consider an uncaught exception in an executable as a bug rather than a wanted way of returning an error to the user.

reiher-research-group commented 2 years ago

Release 3.0.1 of Sparrow is out, so this should be fixed.