bybit-exchange / pybit

Official Python3 API connector for Bybit's HTTP and WebSockets APIs.
Other
423 stars 135 forks source link

chore: _WebSocketManager._on_error rework #230

Open radomir9720 opened 5 months ago

radomir9720 commented 5 months ago

Closes #224

All the changes in this PR are backward compatible.

Below are described changes and reasons.

  1. Added skip_utf8_validation argument, and set its value to False by default. Reason is these two lines:

    if op_code == ABNF.OPCODE_TEXT and not skip_utf8_validation:
    data = data.decode("utf-8")

    If skip_utf8_validation is False, the websocket package will try to decode the message. And the main problem is if this decode function will raise an exception, the on_error callback 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_error callback.

  2. Rework on the _on_error callback. 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_exception is True(by default) the behavior will correspond to the old one. If False - exceptions will be only logged, and connection kept open. Also, as a part of these changes, restart_on_error was renamed to restart_on_ws_disconnect. To make it clearer.

  3. 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.

  4. Deprecation utils. I added two deprecation functions - deprecate_class() and deprecate_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.

radomir9720 commented 5 months ago

Realised that skip_utf8_validation should be by default False, because in the current and previous prod versions validation was enabled. And one may handle the exception caused by validation in the on_close callback, and validation disabling will result in an exception which should be handled in the on_error callback. So, for now, better to keep everything as it was. Maybe in the next major release it is worth to switch this arg to True?