Upgrade SteamAuthenticatorError to have the eresult available where available. #361
Conversation
| @@ -54,7 +54,7 @@ | |||
| from time import time | |||
| from steam import webapi | |||
| from steam.enums import ETwoFactorTokenType | |||
| from steam.steamid import SteamID | |||
There was a problem hiding this comment.
Well, unless documentation for the android generate_device_id method actually needs that...
There was a problem hiding this comment.
Best to leave unrelated changes out of this PR
| def __init__(self, message, eresult=None): | ||
| Exception.__init__(self, message, eresult) | ||
| self.message = message | ||
| self.eresult = EResult(eresult) if eresult is not None else None #: :class:`.EResult`|:class:`None` |
There was a problem hiding this comment.
We have plenty cases where there's no EResult available.
There was a problem hiding this comment.
All these is not needed. Simply inherit from SteamError. Don't set eresult to None where there is not result. Just leave the default.
| def __init__(self, message, eresult=None): | ||
| Exception.__init__(self, message, eresult) | ||
| self.message = message | ||
| self.eresult = EResult(eresult) if eresult is not None else None #: :class:`.EResult`|:class:`None` |
There was a problem hiding this comment.
All these is not needed. Simply inherit from SteamError. Don't set eresult to None where there is not result. Just leave the default.
| @@ -54,7 +54,7 @@ | |||
| from time import time | |||
| from steam import webapi | |||
| from steam.enums import ETwoFactorTokenType | |||
| from steam.steamid import SteamID | |||
There was a problem hiding this comment.
Best to leave unrelated changes out of this PR
| raise SteamAuthenticatorError("MobileWebAuth instance not logged in") | ||
| raise SteamAuthenticatorError( | ||
| message="MobileWebAuth instance not logged in", | ||
| eresult=None, |
There was a problem hiding this comment.
Where eresult is not available, simply don't set it. Default is EResult.Fail.
|
Hi not really much like to fake an API response just for the sake of having that default there. |
|
I see it as extending and reusing the concepts established by Steam. Some of these errors are generated by the library itself and it doesn't make sense to have different set of errors given the Steam ones are already detailed enough. Generally, any failure in Steam results in a Additionally, it makes this change from minor enhancement to breaking change. Any code expecting to only get numerical value of |
|
That is an issue, I see that. How about adding a separate EResult code, like Or I can simply drop the subclassing and then having None is no issue at all. |
Any library specific errors can simply be handled defining more exceptions, which is the native way of doing it. |
See #357.