-
Notifications
You must be signed in to change notification settings - Fork 16
Stop hiding cause of last exception #224
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
base: main
Are you sure you want to change the base?
Conversation
2999255
to
9985364
Compare
9985364
to
56d9b6e
Compare
def __str__(self) -> str: | ||
return str(self.message) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since message
is passed in super.__init__(message)
and we removed the custom __str__
implementation below, this is already the default implementation.
In Python, __str__
isn't helpful. If you need to know the class name, then __repr__
is required, and our __repr__
implementation contains all information about the current exception, including errors
.
>>> e = ValueError("abandon ship!")
>>> str(e)
'abandon ship!'
>>> repr(e)
"ValueError('abandon ship!')"
def __str__(self) -> str: | ||
if self.errors: | ||
return f"Connection error caused by: {self.errors[0].__class__.__name__}({self.errors[0]})" | ||
return "Connection error" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This __str__
implementation tries to be helpful but hides the current exception to focus on errors
, which are past attempts. This comment applies to the other subclasses as well.
def exception_to_dict(exc: TransportError) -> dict: | ||
return { | ||
"type": exc.__class__.__name__, | ||
"message": exc.message, | ||
"errors": [exception_to_dict(e) for e in exc.errors], | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is all the information exposed by __repr__
, but in a more structured way which is nicer to test for.
Closes #223
While this does not change any types, it does change the values returned by
str(e)
andrepr(e)
, so I would like to include this in 9.0 only, without backporting to 8.x.