Parsing failure when expecting JSON array but receiving a server error with JSON body

[mihalyr]

Hi,

I ran into one more issue with the code that prevents me from using it. The problem is the following:

1. expected JSON object response type is array, e.g. Item[].class, so I use JsonResponseConsumers.create(objectMapper, Item[].class)
2. server like Dropwizard/Jersey returns an error (e.g. HTTP 404 status) with a JSON body such as {"code":404,"message":"HTTP 404 Not Found"}
3. I expect the FutureCallback#completed to be called with an HttpResponse containing the 404 status code and the reasonPhrase, so I can deal with it.

Now, what happens is, that the JSON parser tries to parse the error message into the response class, but fails and the whole request fails with a JSON parsing exception instead of the 404 that will be lost. This happens for any other error response such as 500.

Caused by: com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot deserialize instance of `[Lorg.example.Item;` out of START_OBJECT token
 at [Source: UNKNOWN; line: -1, column: -1]
    at com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:59)
    at com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1445)
    at com.fasterxml.jackson.databind.DeserializationContext.handleUnexpectedToken(DeserializationContext.java:1219)
    at com.fasterxml.jackson.databind.DeserializationContext.handleUnexpectedToken(DeserializationContext.java:1129)
    at com.fasterxml.jackson.databind.deser.std.ObjectArrayDeserializer.handleNonArray(ObjectArrayDeserializer.java:329)
    at com.fasterxml.jackson.databind.deser.std.ObjectArrayDeserializer.deserialize(ObjectArrayDeserializer.java:175)
    at com.fasterxml.jackson.databind.deser.std.ObjectArrayDeserializer.deserialize(ObjectArrayDeserializer.java:21)
    at com.fasterxml.jackson.databind.ObjectMapper._readValue(ObjectMapper.java:4189)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2545)
    at com.ok2c.hc5.json.http.JsonObjectEntityConsumer.lambda$new$0(JsonObjectEntityConsumer.java:42)
    at com.ok2c.hc5.json.http.JsonObjectEntityConsumer.lambda$createJsonTokenConsumer$1(JsonObjectEntityConsumer.java:58)
    at com.ok2c.hc5.json.TokenBufferAssembler.accept(TokenBufferAssembler.java:47)
    at com.ok2c.hc5.json.JsonAsyncTokenizer.processData(JsonAsyncTokenizer.java:62)
    at com.ok2c.hc5.json.JsonAsyncTokenizer.consume(JsonAsyncTokenizer.java:89)
    at com.ok2c.hc5.json.http.AbstractJsonEntityConsumer.consume(AbstractJsonEntityConsumer.java:75)
    at com.ok2c.hc5.json.http.JsonMessageConsumer.consume(JsonMessageConsumer.java:108)

I think the issue is that the JSON consumer cannot assume that non-2XX responses will adhere to the response class structure.

In my existing naive implementation with SimpleHttpResponse, I parse the response body to the response class only on 2XX status codes, but handle errors differently (currently no parsing the body at all, just setting the failure).

I am not sure what would be the best approach to have a way around this using JsonResponseConsumers.

One option for sure can be to ignore the body and return a null object on non-2XX responses, but that might not fit every use case and in some cases the lost error response bodies could be useful (e.g. Google API telling you what limit was exceeded when returning a 429).

Another way might be to provide a different "error body" class to the JSON consumer, but then the issue comes with non-JSON error bodies or different JSON responses on different errors for example, so maybe returning a JsonNode or String on error responses?

Of course I could just use the JsonNode entity consumer and then do whatever I want with it in the callback, but at that time I could probably just use SimpleHttpRequests to buffer the byte[] and then parse it in one go.

I think the Jersey client for instance would throw an exception on non-2XX (NotFoundException, etc.) and won't try to parse the body into the response class.

[mihalyr]

In Netty I could deal with the http status in a handler and decide whether to pass the response further the chain, I was wondering if some kind of callback approach to decide when to parse would work, but probably would need something that is simple enough as the rest of the API.

Or perhaps instead of returning the generic type "T" we could wrap it in a ResponseBody<T> object that has the parsed object "T" on successful status codes, otherwise it allows to retrieve the unparsed error body as a String or bytes.

[ok2c]

In Netty I could deal with the http status in a handler and decide whether to pass the response further the chain

@mihalyr I see no reason why the same cannot be done with HC. One needs a custom AsyncResponseConsumer that delegates to different AsyncResponseConsumers or AsyncEntityConsumers depending on the response status. I can whip something up in the coming days.

[ok2c]

@mihalyr If you could provide a test case reproducing the issue similar to JsonEntityConsumerTest it would really help.

[mihalyr]

Sure, I'll find some time tomorrow to do it. Thanks for looking into it.

[mihalyr]

@ok2c I've created a test that reproduces this here https://github.com/mihalyr/httpcomponents-jackson/commit/f5cb06399af572f85e4f12d1f58ef61416bbae68

[ok2c]

I've created a test that reproduces this here mihalyr@f5cb063

@mihalyr Awesome! I will be working on a fix for the issue.