averbis / averbis-python-api

Conveniently access the REST API of Averbis products using Python
Apache License 2.0
12 stars 4 forks source link

Improve error message when an endpoint is called that points to an invalid URL #60

Closed DavidHuebner closed 3 years ago

DavidHuebner commented 3 years ago

Is your feature request related to a problem? Please describe. In the case of an endpoint that has changed (#59 ), the error message is

~/.local/lib/python3.8/site-packages/averbis/core/_rest_client.py in __request(self, method, endpoint, **kwargs)
    807 
    808         response = raw_response.json()
--> 809         self.__handle_error(response)
    810         return response
    811 

~/.local/lib/python3.8/site-packages/averbis/core/_rest_client.py in __handle_error(response)
   1268     @staticmethod
   1269     def __handle_error(response):
-> 1270         if response["errorMessages"] is None or len(response["errorMessages"]) == 0:
   1271             return
   1272 

KeyError: 'errorMessages'

Describe the solution you'd like This is not helpful at all. We should raise a specific Error, such as an EndpointDoesNotExistError.

DavidHuebner commented 3 years ago

I investigated this issue. There are three kind of possible errors:

  1. General errors with the request that are potentially outside of the scope of our platform or where no access is granted. Sometimes raw_response.json() does not even return a json. In that scenario, we want to just print the status_code to indicate what went wrong.

    Example: Exception("Client request failed with status code 401.")

  2. Errors within the domain of our platform, but when a missing endpoint (see initial example in this issue) is called. In that case we want to return the status_code and what is given in the "message" of the json response. This is what initially caused a problem because the error message was in "message" and not in "errorMessages".

json={
          "servlet": "Platform",
          "message": "Not Found",
          "url": "my_url",
          "status": "404",
      },

Example: Exception("Client request failed with status code 404.\nError message is: Not Found")

  1. We call an existing endpoint, but there is a problem. For this, we already had some logic, but we should additionally return the status_code as it can be informative:

    Examples: Exception("Client request failed with status code 400.\nError message is: A project with name 'test' already exists.")

reckart commented 3 years ago

In case 1, we probably want to print not only the status code but also the actual response which may contain additional information. Cf. https://github.com/averbis/averbis-python-api/commit/41b5f8940141cb289de5aaa40418f5714f9fd6e0

Also, case 3 should be case 1 or 2: if we call an endpoint and there is a problem, then there should either be a JSON response with the error message or there should be a informative message in the response body. Better the first. If this is not the case, the platform needs to be changed to provide a better message

However, there is a case 4 which is we either not talking to a platform or the platform is offline - in which case we should tell the user that there is actually no platform or that the connection could not be established at all.

DavidHuebner commented 3 years ago

Thanks for the discussion point. I agree that we can reduce all kinds of errors into two kinds: Either there is information in the raw_response (in the field status_code and reason) which could by text, bytes or something different OR there is error information in the response when the response is a json object. Then we assume that it is located either in the field "message" or in the field "errorMessages". I included this logic in my latest commit. Please have a look.