search

ideafast / middleware-services

Python API containing endpoints for smartphone hub applications and transfer to data portal
0 stars 0 forks source link

Catch Request Exceptions in Consumer #72

Closed jawrainey closed 3 years ago

jawrainey commented 3 years ago

It is possible for an error to be raised in consumer.services.inventory:response such as if the inventory is offline, i.e., all requests.HTTPError. This is because res.raise_for_status() is not implemented and not try/catched, and would be the same in other services, support, UCAM, etc.

We have two options:

  1. Add raise_for_status in a try/catch for HTTPError then log the error.
  2. Catch app HTTPErrors at the router level and log there. That way, we do not need try/catch throughout the services.
jawrainey commented 3 years ago

I've implemented a proof-of-concept for (2) in this branch. The advantage here is that we do not have to use try/catch in all services for all possible requests errors. What's your thoughts on this approach @davidverweij?

To test it:

  1. poetry run consumer
  2. navigate to http://0.0.0.0:8000/inventory/device/history/ABC
  3. modify the url to be something else
  4. Note that in the browser a 404 is returned

This means that when a 500 error occurs (that we do not account for) then we can provide an enveloped reponse to clients. For example, modify the url above to be "https://fake.abc" and that the response is enveloped (data/meta) rather than "internal server error". As long as we log this data we can address any internal problems.

davidverweij commented 3 years ago

Tested it - works well! What do you think about feeding back some expected errors. For example - if a dependency if broken (i.e. the inventory system), it can make sense to just actually throw the HTTP 500 to reflect the API in a more honest manner. Enveloping everything does convolute some things a bit; i.e., depending on who is using it. But due to our API mainly being an (project) internal approach, your solution works well and keeps the API running regardless of dependencies!

So, LGTM

jawrainey commented 3 years ago

We return a status_code of 500 if an error is thrown internally but in a way that allows clients to consumer the API consistently. As we use "meta.success" to outline as source of truth then the client can use either that or the http.status_code if an error is thrown.