slumber icon indicating copy to clipboard operation
slumber copied to clipboard

slumber.Resource._process_response

Open diefans opened this issue 10 years ago • 2 comments

I am just checking out your code and stumbled upon slumber.Resource._process_response which seems to throw away and ignore responses =! 200..299.

    def _process_response(self, resp):
        # TODO: something to expose headers and status

        if 200 <= resp.status_code <= 299:
            return self._try_to_serialize_response(resp)
        else:
            return  # @@@ We should probably do some sort of error here? (Is this even possible?)

I think there are a lot of REST APIs which supply meaningful information beyond 2xx and the API wrapper should never try to make assumptions about the API it is wrapping.

diefans avatar May 25 '15 16:05 diefans

Agreed, although it's hard to know what the right thing to do is really. I only see one option, but this is also making some assumptions. Maybe it's the best this API can do:

Throw an exception pertaining to the error code range: 4xx, 5xx. With this however, how do we handle the body content, if any, that might provide meaningful information (an assumption unfortunately), e.g. The API might return some helpful information as part of a 410 gone response for example. Maybe we could follow something like algorithm:

if not (200 <= resp.status_code <= 299):
    body = None
    try:
        body = self._try_to_serialize_response(resp)
    except:
        pass
    raise HTTPClientError(resp.status_code, body)

Thoughts?

samgiles avatar May 29 '15 10:05 samgiles

Every HTTP REST API delivers its response by a tuple of (status code, encoded body). Slumber is basically focusing on deserializing the body into a python structure but ignores the status code. From my point of view all status codes should be handled in the same way - no assumption if this and that are considered an error. The status code must be passed to the application on every case, since the application is implementing the wrapped API and not slumber, so in the end the application must turn (status code, encoded body) into an exception or an error value. In that way Slumber would be a transparent tool without assumptions.

So I think of something like this:

    def _process_response(self, resp):
        result = self._try_to_serialize_response(resp)
        result.status_code = resp.status_code
        return result

diefans avatar May 29 '15 11:05 diefans