Try-except decorator for classmethods

Question

This question is related to the Cactus Text Game Engine.

---

For easy error handling in Cactus, we've put together this small decorator. Essentially, the decorator wraps any class method in a try-except block, which reports errors to a log file if something goes wrong. While it's a small file, I can't help but feel that it's a little monolithic, and slightly ugly as well.

from contextlib import redirect_stdout
from traceback import format_exc
from pprint import pformat
from sys import exit, maxsize
from os import path
from time import strftime
import platform

def _cactus_class_method_exception_handle(function):
    """
    This function provides a general wrapper for
    handling errors with the Cactus engine. Example
    usage:

        @_cactus_class_method_exception_handle
        def my_class_method(self, args):
            code
    """
    def wrapper(self, *args, **kwargs):
        try:
            return function(self, *args, **kwargs)
        except Exception as cactus_game_error:
            date_string = strftime("%H_%M_%S_%b_%d_%y")

            print("\n" + "=" * 50 + "\n")
            print("Something wrong internally happened. (This may or may not be your fault!). This may be an issue with the game itself, or the") 
            print("underlying components. To help fix this issue, please contact the developers of the game with the error report.") 
            print("")
            print("In your error report, please include the file:")
            print(path.abspath("cactus_error_log_" + date_string +".txt"))
            print("\n" + "=" * 45 + "\n")

            with open("cactus_error_log_" + date_string +".txt", "w+") as cactus_error_log:
                with redirect_stdout(cactus_error_log):
                    print("\n" + "=" * 45 + "\n")
                    print("CACTUS GAME ENGINE ERROR REPORT")
                    print("")
                    print("If you think this error is engine-related,")
                    print("please report it to:")
                    print("https://github.com/ShearOfDoom/Cactus/issues/")
                    print("\n" + "=" * 45 + "\n")
                    print("Date/Time: {0}".format(strftime("%c")))
                    print("Operating System: {0}".format(platform.platform()))
                    print("Processor: {0}".format(platform.processor()))
                    print("System [not Python itself] 32 or 64 bit: {0}".format(platform.machine()))
                    print("Is 64-bit Python?: {0}".format(maxsize > 2**32))
                    print("Python: {0} {1} {2} {3}".format(platform.python_implementation(), platform.python_build()[0], platform.python_build()[1], platform.python_compiler()))
                    print("\n" + "=" * 45 + "\n")
                    print("CLASS DATA:", pformat(self.class_data, indent = 2))
                    print("\n" + "=" * 45 + "\n")
                    print("STACK TRACE:", format_exc())
                    print("=" * 45 + "\n")
                    exit(0)
    return wrapper

Here's a simple example of the output this function will produce when it encounters an error of any kind.

=============================================

CACTUS GAME ENGINE ERROR REPORT

If you think this error is engine-related,
please report it to:
https://github.com/ShearOfDoom/Cactus/issues/

=============================================

Date/Time: 06/11/15 18:14:59
Operating System: Windows-8-6.2.9200
Processor: Intel64 Family 6 Model 69 Stepping 1, GenuineIntel
System [not Python itself] 32 or 64 bit: AMD64
Is 64-bit Python?: False
Python: CPython v3.4.3:9b73f1c3e601 Feb 24 2015 22:43:06 MSC v.1600 32 bit (Intel)

=============================================

CLASS DATA: { 'about_text': 'Write about your game here.',
  'allow_help': True,
  'case_sensitive': False,
  'desc': 'Game Description',
  'event_handlers': { 'position.name goes here 1.enter.after': <built-in function exit>,
                      'position.name goes here 2.enter.after': <built-in function exit>},
  'flowchart': <cactus.flowchart.Flowchart object at 0x02A00F30>,
  'invalid_input_msg$': 'Invalid input',
  'name': 'Game Name',
  'prompt': '> '}

=============================================

STACK TRACE: Traceback (most recent call last):
  File "C:\Users\...\Desktop\Git-Projects\Cactus\cactus\errors.py", line
21, in wrapper
    return function(self, *args, **kwargs)
  File "C:\Users\...\Desktop\Git-Projects\Cactus\cactus\game.py", line 54,
in _check_class_data
    raise KeyError("Could not find key {0} in class data.".format(item[0]))
KeyError: 'Could not find key invalid_input_msg in class data.'

=============================================

Answer 1

1. Design

The decorator mechanism seems hard to use, unreliable, and counter-productive. It's hard to use because you have to remember to add it to every method that might raise an exception. It's unreliable because it would be easy to forget one or two. And it's counter-productive because in Python, exceptions are not only used for runtime errors, but for non-local flow of control too, and wrapping methods with this decorator makes that use impossible.

The usual approach to global error handling and logging is to intercept only uncaught errors (that is, ones that would have caused Python to exit anyway) at the top level (where there is no question of the exception being used for control flow).

For example, if your top-level code looks like this:

# game code here, including definition of main

if __name__ == '__main__':
    main()

then you'd separate the game code into its own module, leaving the top-level code looking like this:

if __name__ == '__main__':
    try:
        import game
        game.main()
    except Exception as error:
        # etc.

This is easy to use (having done this once, there's nothing else to do), reliable (you can't forget to add the decorator to a method), and doesn't prevent the rest of the code from using exceptions for control flow.

The only thing that's tricky is getting at self.class_data for the error report. You'll need to use inspect.trace for this. But if you go down this route, then you'll find that you can log all the local variables in the stack frames leading up to the exception, not just self.class_data. Having all the local variables is really helpful when you're trying to debug a problem based on a customer report.

2. Review

1. If you followed the Python style guide (PEP8) recommendation, "Limit all lines to a maximum of 79 characters", then we wouldn't have to scroll the code horizontally to read it here.

2. The text of the error message should also be limited to 79 characters, for the benefit of the user reading it in an 80-column terminal.

3. It's not clear why this is called a "class method" decorator. The decorator signature uses the name self for the first argument, as does the example in the docstring, which suggests an ordinary method and not a class method.

4. I don't like the name _cactus_class_method_exception_handle. The "cactus" part is just a namespace, and in Python modules are a better way to manage namespaces. The "class method" part seems wrong too, as discussed above.

5. There is no need for local variables like cactus_game_error and cactus_error_log to contain the namespace cactus (there's no other game_error or error_log variable, so there's no need to disambiguate).

6. When decorating a function, it's a good idea to use functools.wraps so that the decorated function has the same name and docstring as the original.

7. The code for computing the filename is duplicated:

   "cactus_error_log_" + date_string + ".txt"
   

   It would be easy to accidentally change it in one place and not the other. Better to assign a variable:

   log_name = strftime("cactus_error_log_%H_%M_%S_%b_%d_%y.txt")
   

8. When making filenames that include a timestamp, it's a good idea to use the ISO 8601 date format, because then the filenames sort into time order, making the newest file easy to find (it's the last one in the sorted list), instead of being hidden among other files from similar times of day on different dates. Use something like:

   log_name = strftime("cactus_error_log_%Y-%m-%dT%H:%M:%S.txt")
   

9. Log files are written into the current directory, wherever that happens to be. This is rude: what if the user doesn't want their home directory (or wherever) filling up with log files? The usual place to put this kind of file is in the system temporary directory, so I recommend using tempfile.mkstemp:

   log_file, log_path = tempfile.mkstemp(prefix=log_name, text=True)
   with contextlib.closing(log_file):
       # etc.
   

10. Python's string formatting engine supports named parameters. It's easier to understand something like {date} than {0}.

11. There are 50 equals signs at the top of one message, but 45 equals signs everywhere else.

12. The error message says, "please contact the developers of the game with the error report", but does not say how to do so. By e-mail? If so, to what address? On the web? If so, at what URL?

13. Constant values like the format strings should be global variables. It would be convenient to use Python's triple-quoting to provide one format string for each block of output. This makes the text easier to read and edit, because it's not surrounded by print() and quotes and so on. So instead of:

    print("\n" + "=" * 50 + "\n")
    print("Something wrong internally happened. (This may or may not be your fault!). This may be an issue with the game itself, or the") 
    print("underlying components. To help fix this issue, please contact the developers of the game with the error report.") 
    print("")
    print("In your error report, please include the file:")
    print(path.abspath("cactus_error_log_" + date_string +".txt"))
    print("\n" + "=" * 45 + "\n")
    

    declare the message as a global constant:

    STDOUT_MESSAGE = '''
    ============================================================
    Something wrong internally happened. (This may or may not be
    your fault!). This may be an issue with the game itself, or
    the underlying components. To help fix this issue, please
    contact the developers of the game with the error report.
    
    In your error report, please include the file:
    {filename}
    ============================================================
    '''
    

    and then:

    print(STDOUT_MESSAGE.format(filename=log_path))
    

    Note that this mechanism avoids the need to use use redirect_stdout, because you can easily write:

    msg = LOG_MESSAGE.format(platform=platform.platform(),
                             processor=platform.processor(),
                             bits64=maxsize > 2 ** 32,
                             # etc)
    log_file.write(msg)
    

14. When a program exits due to an error, the exit code should be non-zero.