sys.exit(1) vs raise SystemExit vs raise

[cs at zip.com.au]

On 12Apr2016 18:20, Ganesh Pal <ganesh1pal at gmail.com> wrote:
>I m on python 2.7 and Linux ,  I have a simple code  need suggestion if  I
> I could replace sys.exit(1) with raise  SystemExit .
>
>==Actual code==
>
>def main():
>    try:
>        create_logdir()
>        create_dataset()
>        unittest.main()
>    except Exception as e:
>        logging.exception(e)
>        sys.exit(EXIT_STATUS_ERROR)
>
>if __name__ == '__main__':
>    main()
>
>==Changed Code==
>
>
>def main():
>    try:
>        create_logdir()
>        create_dataset()
>        unittest.main()
>    except Exception as e:
>        logging.exception(e)
>        raise SystemExit
>
>if __name__ == '__main__':
>    main()

I am against both of these personally. My preferred pattern is like this:

  def main(argv):
    try:
      ...
    except Exception as e:
      logging.exception(e)
      return 1

  if __name__ == '__main__':
    sys.exit(main(sys.argv))

Notice that main() is back to being a normal function with normal returns.

Also, most of us would avoid the "except Exception" and just let a top level 
except bubble out: that way you get a stack backtrace for debugging. I agree it 
prevents logging the exception and makes for uglier console output, but I think 
it is a win. And if you _do_ want to log the exception there is always this:

  try:
    ...
  except Exception as e:
    logging.exception(e)
    raise

to recite the exception into the log and still let it bubble out normally.

The problem with the "except Exception" pattern is that it catches and _hides_ 
_every_ exception, not merely the narrow set of specific exceptions that you 
understand.

Finally, it is frowned upon to raise a bare Exception class. In  python 3 I 
believe it is actually forbidden, so it is nonportable anyway. But even In 
Python to it is best to supply an Exception instance, not the class:

  raise SystemExit(1)

>2. All the functions in try block have exception bubbled out using raise
>
>   Example for create_logdir() here is the function definition
>
>def create_logdir():
>
>    try:
>        os.makedirs(LOG_DIR)
>    except OSError as e:
>        sys.stderr.write("Failed to create log directory...Exiting !!!")
>        raise
>    print "log file: " + corrupt_log
>    return True
>
>def main():
>    try:
>        create_logdir()
>    except Exception as e:
>        logging.exception(e)
>        raise SystemExit
>
>(a) In case if create_logdir() fails we will get the below error ,is
>this fine or do I need to improve this code.
>
>Failed to create log directory...Exiting !!!ERROR:root:[Errno 17] File
>exists: '/var/log/dummy'
>
>Traceback (most recent call last):
>  File "corrupt_test.py", line 245, in main
>    create_logdir()
>  File "corrupt_test.py", line 53, in create_logdir
>    os.makedirs(LOG_DIR)
>  File "/usr/local/lib/python2.7/os.py", line 157, in makedirs
>OSError: [Errno 17] File exists: '/var/log/dummy'

I prefer the bubble out approach, perhap with a log or warning messages as you 
have done, eg:

  logging.exception("create_logdir failed: makedirs(%r): %s" % (LOG_DIR, e))
  raise

(Also not that that log message records more context: context is very useful 
when debugging problems.)

For very small scripts sys.stderr.write is ok, but in general any of your 
functions that turned out to be generally useful might migrate into a library 
in order to be reused; consider that stderr is not always the place for 
messages; instead reading for the logging module with error() or wanr() or 
exception() as appropriate. There is more scope for configuring where the 
output goes that way without wiring it into your inner functions.

>3. Can I have just raise , instead of SystemExit or sys.exit(1) . This
>looks wrong to me
>
>  def main():
>
>    try:
>        create_logdir()
>    except Exception as e
>        logging.exception(e)
>        raise

This is what I would do, myself.

Think: has the exception been "handled", meaning has the situation been dealt 
with because it was expected? If not, let the exception bubble out so that the 
user knows that something _not_ understood by the program has occurred.

Finally, it is generally bad to SystemExit or sys.exit() from inside anything 
other than the outermost main() function. And I resist it even there; the main 
function, if written well, may often be called from somewhere else usefully, 
and that makes it effectively a library function (it has been reused). Such a 
function should not unilaterally abort the program. How rude! Instead, let the 
exception bubble out: perhaps the _caller_ of main() expects it and can handle 
it. By aborting and not "raise"ing, you have deprived the caller of the chance 
to do something appropriate, even though you yourself (i.e. "main") do not know 
enough context to handle the exception.

So I am for "raise" myself. And then only because you want to log the error. If 
you didn't want to log the exception you could avoid the try/except _entirely_ 
and have simpler code: let the caller worry about unhandled exceptions!

Cheers,
Cameron Simpson <cs at zip.com.au>