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

Check for actual http response codes #38

Closed
bramstroker opened this issue Oct 10, 2021 · 7 comments · Fixed by #39
Closed

Check for actual http response codes #38

bramstroker opened this issue Oct 10, 2021 · 7 comments · Fixed by #39
Assignees
Labels
enhancement New feature or request

Comments

@bramstroker
Copy link

Currently the wrapper always assumes the request was succesfull (ie. 20x statuscodes), and tries to decode the resulting json.
It would be nice if the api client would actually check for the error codes and throws a custom exception when this is not the case.

I experience this issue when retrieving state for a non existing entity.
HA rest api returns a 404 (not found) in this case, which is logical but this is not handled by the API wrapper.
It tries to decode the json in a State object resulting in the following error:

  File "/usr/local/lib/python3.9/site-packages/homeassistant_api/models/states.py", line 11, in __init__
    raise ParameterMissingError('"entity_id" attribute is a required parameter')
homeassistant_api.errors.ParameterMissingError: "entity_id" attribute is a required parameter

Would be nice if a EntityNotFoundError was raised or something similar.

Currently I have no way to handle this cases cleanly.

@bramstroker bramstroker added the enhancement New feature or request label Oct 10, 2021
@GrandMoff100
Copy link
Owner

That is a great idea! I think I have some ideas but it's going to take a lot of restructuring under the requesting hood and the exception handling hood. But it should be fun. I'll get right on it.

@bramstroker
Copy link
Author

Wow that's fast ;-) keep up the good job on this library!

@bramstroker
Copy link
Author

Btw regarding the rest API docs the following error status codes can be returned:

  • 400 (Bad Request)
  • 401 (Unauthorized)
  • 404 (Not Found)
  • 405 (Method not allowed)

I think most of them can be handled globally just after the request has been done.

@GrandMoff100
Copy link
Owner

Thank you so much for your support! It means a lot. :)

Concerning those HTTP codes. I think I'll just have different handlers registered for different http codes. I'm thinking about adding another layer of processing between the request and the json parsing that'll raise reception based on http codes and then only 200 range codes get to continue onto to a mimetype parser then to be parsed into json format if the mimetype matches. (To handle images and other binary files which is rare but I'd like to include the functional to be inclusive of all reasonable use cases.)

Basically I'm going to add a couple layers of parsing and processing under the hood, maybe an entire request Processing class which make things a little bit more organized rather than scattered throughout the Client classes.

Overall, thank you for raising this issue! I'll try my best to keep up the good work :) It's nice to know my work is appreciated.

@GrandMoff100
Copy link
Owner

Either today or tomorrow I'll open a PR with my changes and I'll link it to this issue. :D

It should be a lot of fun to work on this. I'm really excited to add some new features!

@bramstroker
Copy link
Author

Awesome job. Clean solution. Thanks for implementing.

@GrandMoff100
Copy link
Owner

No problem man.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants