Conversation
| app_info: dict = client.get(path=app_service_path).json() | ||
| app_info_res: Response = client.get(path=app_service_path) | ||
| if app_info_res.status_code >= 300: | ||
| raise KeyError( |
There was a problem hiding this comment.
I don't know if a KeyError is the best thing to raise, since it usually applies to dictionaries. Maybe ValueError?
There was a problem hiding this comment.
Yes, I agree that the KeyError is probably too specific here. I would suggest to go with a generic RuntimeError.
Further, I think we should be a bit more specific about the kind of status codes that we actually catch. Where is the API for the application-service documented? These are the kind of codes I would expect in this particular context (despite generic 500s etc):
- 200: The application exists and capabilities are returned.
- 404: No application with the provided ID exists (or the application is hidden from the user)
- 403: Missing authorization
I think catching all status codes above a certain error code and then throwing a generic error message is not really helpful to application developers (something I have pointed out before). Issue #38 was originally motivated, because the request resulted in a JSON decode error which is obviously utterly unhelpful.
There was a problem hiding this comment.
The codes currently returned are 200, 404 and 401 for Unauthorized (not ´403´). As for documentation... working on it.
3 participants
Fixing #38