Skip to content
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

Fix error response detection #9

Merged
merged 1 commit into from
Nov 25, 2023
Merged

Fix error response detection #9

merged 1 commit into from
Nov 25, 2023

Conversation

jhansche
Copy link
Owner

The check for is dict we were trying to use previously was wrong, and would have needed to be type(data) is dict. But that is not the recommended way to check for a dictionary anyway, so this fixes it to use isinstance() instead.

This also adds a check for a different way that TeslaFi can report an error: specifically, if the API key is not recognized, the response is completely different compared to when it is recognized but the API has been disabled; or the particular feature of the API is not enabled (such as unlocking). We were already checking for the latter, but not the former, so the error in this case is confusing.

The check for `is dict` we were trying to use previously was wrong,
and would have needed to be `type(data) is dict`. But that is not
the recommended way to check for a dictionary anyway, so this fixes
it to use `isinstance()` instead.

This also adds a check for a different way that TeslaFi can report
an error: specifically, if the API key is not recognized, the response
is completely different compared to when it is recognized but the
API has been disabled; or the particular feature of the API is not
enabled (such as unlocking). We were already checking for the latter,
but not the former, so the error in this case is confusing.
@jhansche jhansche merged commit a9edbca into main Nov 25, 2023
4 checks passed
@jhansche jhansche deleted the pr/check-more-errors branch November 25, 2023 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant