Preserve original remote error attributes in API exception#159
Preserve original remote error attributes in API exception#159knaperek wants to merge 1 commit intofireblocks:masterfrom
Conversation
What this fixes: - wild "except" statement - non-Pythonic direct type comparison (even worse, with "is") - textual FB response message is parsed and preserved; this may be handy when trying to automatically propagate a meaningful textual error reason somewhere upstream in the code (e.g. operator dashboard panel) - original HTTP status code is now preserved for easier debugging under the http_code attribute - original requests status exception is nicely chained (with the extended raise from statement)
9693da1 to
69e6e65
Compare
| def __init__(self, message="Fireblocks SDK error", error_code=None): | ||
| self.message = message | ||
| def __init__(self, | ||
| reason: str = '', |
| display_error = reason or \ | ||
| (error_message and f"Got an error from fireblocks server: {error_message}") or \ | ||
| "Fireblocks SDK error" | ||
|
|
||
| super().__init__(display_error) |
There was a problem hiding this comment.
please fix without "reason" and make sure it is the exact same string as previous to preserve backward compatibility
check before what was the string if "response.text" was None
| except (AttributeError, ValueError): | ||
| response_data = None |
There was a problem hiding this comment.
what about other types of exceptions?
There was a problem hiding this comment.
Which other types of exceptions do you have in mind that would be appropriate to silence and automatically handle this way?
As it is, the wild except statement would catch all exceptions, including system-related errors that have nothing to do with the Fireblocks library itself (e.g. program could be terminated, out of memory, etc..). In such cases it is more appropriate to propagate the exception rather than silencing it this way.
Wild except statements are very discouraged in general and are considered bad practice in Python.
What this fixes: