Handling errors and warnings in Payu

[jo-basevi]

It would be good to develop a consistent approach to logging, handling warnings and errors in Payu. Currently for errors, there's a range of:

• sys.exit('payu: error: ...')
• Standard exceptions: e.g. raise RuntimeError(...), raise FileNotFoundError(...)
• Custom exceptions: raise PayuBranchError

For warnings:

• print('payu: warning: ...')
• warnings.warn(...)

A benefit of using exceptions is that it is easy for them to be caught and handled. This is a large part why using sys.exit() is discouraged in library code. On the other hand, exceptions with stack traces (and warnings with code line numbers), aren't the most user friendly and it is often showing information that is not required. Though, especially in development, there are cases where it is useful to have stack traces.

An approach could be to catch exceptions in the main CLI entry point code, and then log the error messages and remove the stack trace, and have a clean exit with sys.exit(1). Similarly, the warnings could be configured in the main CLI entry point code to format and log the warnings. We could add a "debug" option flag to the CLI (and/or config.yaml) to include debug logs with stack traces.

A similar approach could be to do above - so keep any global configuration or wrappers around the CLI entry point code, but have custom PayuError type (with more specific sub-classes like PayuConfigError or PayuFileNotFound), then only these exceptions will have a clean error message with no stack trace, and all other exceptions keep their stack trace as before. This will have the benefit that we know the customs exceptions should have a descriptive error message.

Some references:

• An explanation of configuring global logging near the CLI entry point of code: https://realpython.com/ref/best-practices/logging/
• Blog post on error handling which goes into high level catching exceptions with a sys.exit(): https://blog.miguelgrinberg.com/post/the-ultimate-guide-to-error-handling-in-python
• An explanation on why it's difficult to handle system exits: https://stackoverflow.com/a/48571382
• Python docs on when to use logging: https://docs.python.org/3/howto/logging.html#when-to-use-logging

[jo-basevi]

@aidanheerdegen @KAUR1984 @Qian-HuiChen - this is follow-up to the discussions around logging and handling errors in #664, and how to format warnings in #451. Please feel free to add in ideas or questions as it'll be good to have a clear direction of what to do by the next payu session.

[aidanheerdegen]

That proposal seems really good @jo-basevi. It means as long as we're not using the cli.* entry-point in testing we still get meaningful exceptions we can use in unit testing, and we're also giving users the option of full stack-traces with a debug option flag.

It would be good to decouple a debug option from however we set the logging levels, in that we would like to be able to get stacktraces for the standard level of debug output without also requiring DEBUG logging level.

Those links are great BTW. Really helpful to have a framework to think about how to approach exceptions and logging.

[jo-basevi]

It would be good to decouple a debug option from however we set the logging levels, in that we would like to be able to get stacktraces for the standard level of debug output without also requiring DEBUG logging level.

Ok so you mean not to use logger.debug() for stacktraces. Is this to avoid picking up debug logs from other libraries? We could have a --dev-mode command-line flag which enables stack traces, and a PAYU_DEVELOPMENT_MODE environment flag. Or there could be a explicit --stacktrace option - however the --dev-mode or something general seems a good way to add other logs that will be useful in future?

Also were you in favour more of catching all errors or only custom Payu errors? I think I was leaning more towards custom Payu error types as we could guarantee the stand-alone error message would be sufficient.

[aidanheerdegen]

Ok so you mean not to use logger.debug() for stacktraces.

Sorry, I was not clear, and my use of terminology was not good.

I believe the logging module has the ability to configure a higher level of default logging output: DEBUG. This can be turned on by default (unusual), but can be configured "on" to provide more useful debugging information. This is independent of the top level exception catching proposal.

If there is an option to be more verbose when catching errors at the top level, there should be a separate option or method to set the logging level.

Also were you in favour more of catching all errors or only custom Payu errors? I think I was leaning more towards custom Payu error types as we could guarantee the stand-alone error message would be sufficient.

I can see the value in both approaches. There is less work omitting non-payu errors, and there is a limit to how much that can be usefully told to the user if they're just an amorphous blob of "not-payu" errors. On the other hand maybe users get put off with stack-traces and find it more difficult to understand. The user could re-run the command with the debug option to see a full stack-trace, and/or it can be logged to a file in the first instance that we tell the user to see for more info.