fix loss of context/cause on exceptions raised inside open_websocket #192
Conversation
|
shoutout to @graingert for the clean no-copy solution that avoids the issues mentioned in python-trio/flake8-async#298 and python/cpython#125782 |
| context = exc.__context__ | ||
| try: | ||
| raise exc | ||
| finally: | ||
| exc.__context__ = context | ||
| del exc, context |
There was a problem hiding this comment.
just remembered an issue with this:
python/cpython#99856
python-attrs/attrs#1081
| context = exc.__context__ | |
| try: | |
| raise exc | |
| finally: | |
| exc.__context__ = context | |
| del exc, context | |
| context = exc.__context__ | |
| try: | |
| raise exc | |
| finally: | |
| object.__setattr__(exc, "__context__", context) | |
| del exc, context |
There was a problem hiding this comment.
You'll probably need a test with a frozen dataclass and attrs class Exception
There was a problem hiding this comment.
I can fix them... but open_websocket is decorated with @asynccontextmanager so it just kicks the can onto failing in the same way as python/cpython#99856
For attrs compatibility I kind of feel like we can require users with frozen exception to require trio_websocket<=0.11.1 or attrs>24.2.0 (both to be released) - or change their exceptions. It's not a heavy workaround for us, but I still don't really think it's worth it.
| "the internal exception(s)" + eg_substr + "." | ||
| ) | ||
| user_error.__context__ = BaseExceptionGroup(eg_str, exceptions) | ||
| user_error.__suppress_context__ = False |
There was a problem hiding this comment.
this is broken for frozen user_errors on attrs python-attrs/attrs#1365
fixes #191
I'm not 100% sure that
copy_excis 100% robust (given that everything seems ever so fickle with copying exceptions), so it may be a good idea to wrap it in atry: exceptthat gives up on copying if anything fails.