exception_hierarchy.py's SafeException confuses Exception.__repr__

[aaaaalbert]

The definition of class SafeException in RepyV2's exception hierarchy assigns the parameters handed to the exception object's constructor to an instance variable value. However, the __str__, __repr__ et al. methods inherited from Python's Exception class rather expect them to be in args. These functions thus don't work correctly, e.g. repr outputs only the constructor name but no arguments. SafeException does redefine __str__ to access that special variable and make str(ASafeExeptionInstance) work.

I found no indication in the history of the offending code as to why this was implemented like that. It might have been a copy-and-paste job from the Python docs on user-defined exceptions.

Therefore, I propose to remove SafeException's overrides and rely on the methods inherited from Exception to construct string or other representations of exception object instances. Pull request follows.

(Note: I found this problem debugging safe.py's handling of unsafe builtins where an error message created using repr was missing RunBuiltinException's parameter.)

[aaaaalbert]

OK, digging more into the history of the problem I found this forerunner of SafeException, in which SafeException was still a subclass of Exception.

Since its move into exception_hierarchy.py, SafeException subclasses RepyException instead. Note that RepyException does not replace the double-underscore methods, but inherits them from Exception currently.

I didn't find an explanation for not allowing SafeException to inherit these methods, so I think we should

• Remove SafeException's overrides and inherit from the base class (which PR #75 already proposed),
• and optionally override the methods in RepyException instead, so that all of its subclasses inherit the overriden methods.

A diff for the latter would look something like this, with some string mangling required to re-implement __repr__ via type:

--- a/exception_hierarchy.py
+++ b/exception_hierarchy.py
@@ -50,7 +50,20 @@ class InternalRepyError (Exception):

 class RepyException (Exception):
   """All Repy Exceptions derive from this exception."""
-  pass
+  def __init__(self, *value):
+    self.value = value
+  def __str__(self):
+    return str(self.value)
+  def __repr__(self):
+    # Using `type` on a Repy exception returns
+    # "<class 'exception_hierarchy." and the Repy exception class name, 
+    # followed by a closing "'>". We are interested only in the actual 
+    # exception class name.
+    # (The `value` parameter on the other hand is a plain tuple.)
+    my_type_as_string = str(type(self))
+    name_prefix = "<class 'exception_hierarchy."
+    exception_name = my_type_as_string[len(name_prefix):-2]
+    return exception_name + str(self.value)

However, it remains to be argued what the override would be good for in the first place. Comments, opinions, flames?

[awwad]

Yup. It doesn't really make sense to me yet, either.

Here's the definition of BaseException::__repr__() from PEP 352:

    def __repr__(self):
        return "%s(*%s)" % (self.__class__.__name__, repr(self.args))

Here's an example indicating the break in the sensible assumptions. (CheckStrException inherits from SafeException.)

>>> import exception_hierarchy as eh

>>> eIRE = eh.InternalRepyError("arg 1", 2, "arg 3")
>>> eIRE
   InternalRepyError('arg 1', 2, 'arg 3')```

>>> eCSE = eh.CheckStrException("arg 1", 2, "arg 3")
>>> repr(eCSE)
   'CheckStrException()'

Additionally, the list of Exceptions in exception_hierarchy.py is a lovely list of passes, interrupted by the strange case of SafeException.

Probably historical and accidental in the way you noted, Albert. Unless there's some desire to avoid seeing error details...?

Er, I'll just chat with you about this.