Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception that calls super().__init__ breaks copy.copy #125782

Open
jakkdl opened this issue Oct 21, 2024 · 7 comments
Open

Exception that calls super().__init__ breaks copy.copy #125782

jakkdl opened this issue Oct 21, 2024 · 7 comments
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@jakkdl
Copy link

jakkdl commented Oct 21, 2024

Bug report

Bug description:

For some reason calling Exception.__init__ breaks copy.copys ability to detect args. The following code

import copy

class MyException(Exception):
    def __init__(self, x):
        super().__init__()
        self.x = x

my_exc = MyException(5)
copy.copy(my_exc)

gives

Traceback (most recent call last):
  File "./foo.py", line 9, in <module>
    copy.copy(my_exc)
  File "/usr/lib/python3.12/copy.py", line 97, in copy
    return _reconstruct(x, None, *rv)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/copy.py", line 253, in _reconstruct
    y = func(*args)
        ^^^^^^^^^^^
TypeError: MyException.__init__() missing 1 required positional argument: 'x'

Removing the super().__init__() line makes everything work as expected, but given that it's generally seen as good practice to call super().__init__() in child classes, and you may be working with a more complex class structure, this seems like pretty bad behavior.
I haven't dug deep enough into copy.copy or BaseException.__init__ to decide which one deserves the blame.

For the reason why I want to copy exceptions, see python-trio/flake8-async#298

CPython versions tested on:

3.9, 3.10, 3.11, 3.12, 3.13, 3.14

Operating systems tested on:

Linux

@jakkdl jakkdl added the type-bug An unexpected behavior, bug, or error label Oct 21, 2024
@jakkdl jakkdl changed the title Exception that calls __init__ breaks copy.copy Exception that calls super().__init__ breaks copy.copy Oct 21, 2024
@catmeowjiao
Copy link

catmeowjiao commented Oct 21, 2024

Your code would have copy.copy() execute MyException() directly without passing in x and therefore report an error, which is not a Python problem.

@jakkdl
Copy link
Author

jakkdl commented Oct 21, 2024

Your code would have copy.copy() execute MyException() directly without passing in x and therefore report an error, which is not a Python problem.

no? copy.copy() is perfectly capable of passing attributes to __init__. This works:

import copy

class MyException(Exception):
    def __init__(self, x):
        # no call to super().__init__()
        self.x = x

my_exc = MyException(5)
my_copy = copy.copy(my_exc)
assert my_copy.x == 5

and it's not just copying the attributes. If you add a print to __init__ it gets printed twice.

@ZeroIntensity
Copy link
Member

Yeah, this looks like a bug. Confirmed it on the main branch. Thank you for the report!

@ZeroIntensity ZeroIntensity added stdlib Python modules in the Lib dir 3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes labels Oct 21, 2024
@ZeroIntensity
Copy link
Member

I suspect this is an issue with copy rather than Exception itself, so I'm marking this as a standard library problem for now. Would you like to try and fix this, @jakkdl?

@oremanj
Copy link

oremanj commented Oct 21, 2024

copy uses the pickle protocol to reduce the object to a state tuple and then create a new object from that state tuple.

>>> class MyException(Exception):
...     def __init__(self, x):
...         super().__init__()
...         self.x = x
...
>>> MyException(5).__reduce_ex__(4)
(<class '__main__.MyException'>, (), {'x': 5})

Based on https://docs.python.org/3/library/pickle.html#object.__reduce__, this is saying: "call MyException with no arguments, then pass {"x": 5} to its __setstate__ method". Which produces the error shown above. Unpickling produces the same error:

>>> pickle.loads(pickle.dumps(MyException(5)))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: MyException.__init__() missing 1 required positional argument: 'x'

So I don't think this is copy.copy's fault. BaseException assumes that an exception object can be constructed from its args tuple. That's consistent with how an exception repr is printed out; MyException(5) reprs as MyException(). You will have a better time if you respect the args convention where all arguments to __init__ become self.args. But I think having exception objects not roundtrip through copy/pickle correctly when this convention is violated is a bug that ought to be fixed.

It looks like BaseException accepts args in both __new__ and __init__. So maybe one possible solution direction here is to let exceptions reduce more like any other object: instead of making a new instance via a call to the type object, it should be created by calling __new__ with the exception args, followed by __setstate__.

Unfortunately it seems some exceptions such as StopIteration and SystemExit have additional slots which are set only in __init__. Using __new__ to reconstruct them would create some quite insidious regressions:

Python 3.12.0 (v3.12.0:0fb18b02c8, Oct  2 2023, 09:45:56) [Clang 13.0.0 (clang-1300.0.29.30)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> ex = SystemExit(42)
>>> ex.code
42
>>> ex.args
(42,)
>>> ex.__reduce_ex__(4)
(<class 'SystemExit'>, (42,))
>>> ex2 = SystemExit.__new__(SystemExit, 42)
>>> ex2
SystemExit(42)
>>> ex2.code
>>> raise ex2
shell% echo $?
0

Even if CPython were to fix all examples of this internally, there's no way to know how much user code might be relying on the fact that copying/unpickling an exception calls __init__. And pickling exceptions is quite common in certain workflows, such as multiprocessing. So we may be stuck with the current behavior, in which case this would turn into a documentation issue.

@oremanj
Copy link

oremanj commented Oct 21, 2024

copy.copy() is perfectly capable of passing attributes to init. This works:

That's a manifestation of the fact that BaseException saves the args on both __new__ and __init__. If you don't override them by calling __init__ with no arguments, then you get self.args == (5,) based on what was passed to the original object's __new__.

@catmeowjiao
Copy link

I'm sorry for my mistake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
Status: No status
Development

No branches or pull requests

4 participants