chore: _WebSocketManager._on_error rework #230
Conversation
…urposes * chore: log "exception" level instead of "error" in on_error() method, so that trace is also logged * docs: add docstrings for public methods * perf: use % formatting for logger messages * style: format file following linter rules
* chore: made all exceptions inherit base exception * chore: deprecated *Error exceptions, and replaced them with *Exception exceptions
…on_ws_disconnect") * feat: added "disconnect_on_exception" argument * chore: replaced base "Exception" exceptions with "PybitException" inherited ones * chore: rework on _on_error callback. Won't raise the exception when "disconnect_on_exception" is False
| on_pong=lambda ws, *args: self._on_pong(), | ||
| on_message=self._on_message, | ||
| on_close=self._on_close, | ||
| on_open=self._on_open, |
There was a problem hiding this comment.
Lambdas were replaced with named functions because of logging purposes.
So now we'll have:
2024-06-20 19:20:54,291 - websocket - ERROR - error from callback <bound method _WebSocketManager._on_message of <pybit.unified_trading.WebSocket object at 0x7fc0a983fc40>>: 'kline.1.FOGEUSDT'
2024-06-20 19:20:54,482 - websocket - ERROR - error from callback <bound method _WebSocketManager._on_error of <pybit.unified_trading.WebSocket object at 0x7fc0a983fc40>>: 'kline.1.FOGEUSDT'
instead of:
2024-06-20 09:50:05,306 - websocket - ERROR - error from callback <function _WebSocketManager._connect.<locals>.<lambda> at 0x7f7b3854a8c0>: Expecting ':' delimiter: line 1 column 35 (char 34)
2024-06-20 09:50:05,503 - websocket - ERROR - error from callback <function _WebSocketManager._connect.<locals>.<lambda> at 0x7f7b38429a20>: Expecting ':' delimiter: line 1 column 35 (char 34)
We'll se the function the exception was raised from, instead of <locals>.<lambda>
| """ | ||
| Exit on errors and raise exception, or attempt reconnect. | ||
| """ | ||
| if type(error).__name__ not in [ | ||
| "WebSocketConnectionClosedException", | ||
| "ConnectionResetError", |
There was a problem hiding this comment.
"ConnectionResetError" cannot be thrown from the websocket package. Perhaps, this exception is from websockets package. I assume it was used back in the days, but then switched to websocket, but this exception remained here. This is one reason why it's not a very good practice to check the type this way.
| ) | ||
|
|
||
| logger.info(f"WebSocket {self.ws_name} connected") | ||
| logger.info("WebSocket %s connected", self.ws_name) |
There was a problem hiding this comment.
Is this just a style change or is this a better way? Personally prefer {}...
There was a problem hiding this comment.
Forgot to comment this change. Yes, it is not about the style, it's about performance. f-strings will format the message even if it's not logged. In case of %s strings and arguments passed through *args, the string will be formatted only when the message is logged(and, therefore, will not be formatted if the message isn't logged). So we save the resources.
…ard compatibility
|
Realised that |
Closes #224
All the changes in this PR are backward compatible.
Below are described changes and reasons.
skip_utf8_validationargument, and set its value toFalseby default.Reason is these two lines:
If
skip_utf8_validationisFalse, thewebsocketpackage will try to decode the message. And the main problem is if this decode function will raise an exception, theon_errorcallback will not be called. That means when we'll receive a broken message - the connection will be closed. In my opinion, this is an unwanted behavior. On the other hand, yes, we can't do anything with the broken message, but, at least, we can handle the exception, caused by the broken message, using the_on_errorcallback.Rework on the
_on_errorcallback.Old behavior: handle connection error. Other exceptions - disconnect and raise.
New behavior: handle connection error. Other exceptions - gives the choice to the user. If
disconnect_on_exceptionisTrue(by default) the behavior will correspond to the old one. IfFalse- exceptions will be only logged, and connection kept open.Also, as a part of these changes,
restart_on_errorwas renamed torestart_on_ws_disconnect. To make it clearer.Rework on exceptions
Now every exception inherits from
PybitException- base class for exceptions.Also, for consistency purposes, existing *Error exceptions were deprecated. New exceptions named *Exception were created. I had replaced old exceptions with new ones everywhere in the code, because i preserved the backward compatibility(inherited new exceptions from the old one), but then realised that one may not use the best practices of checking the exception type(for example, checking the exception type by its name -
type(error).__name__ == "Error"). Then it will break his code. So i reverted the changes. So, replacing the old exceptions with new ones task remains for the the new major release.Deprecation utils.
I added two deprecation functions -
deprecate_class()anddeprecate_function_arguments().A warning will be printed whenever a deprecated class will be initialized, or a deprecated function argument will be passed as a key-value to function.
Also, i added tests, that won't pass if the current version is greater or equal to the deprecation version. So if some deprecated members should be removed in the current version, tests will tell about that.