ObjectMapper should not serialize to expected type on 400+ response code

[mdentremont]

I'm using the latest Unirest, with the Jackson object mapper. I'm doing the following:

• POST, using asObject(Response.class) to parse the response
• I'm also using ifFailure(ErrorResponse.class, ...) to handle error responses

Expected:

• On 400+ error responses Unirest should use ErrorResponse.class when trying to parse the response

Actual:

• It first tries to serialize the response to Response.class
  • If that fails, it uses the response body when trying to parse the ErrorResponse.class
  • HOWEVER, if the parsing of a Response.class is successful, Unirest tries to coerce the Response.class to an ErrorResponse.class, and in my case, that leaves me with an empty ErrorResponse.class

We probably should go directly to using the error class on 400+ status codes.

It is also annoying that when using .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);, Jackson treats JSON which has completely different fields than my POJO as a successful de-serialization. I'd like to just instruct JSON to treat all my POJO properties as required by default, but that doesn't appear to be an option.

[ryber]

The problem here is that by the time you get to ifFailure the response has already been parsed. What we would need is some form of .asEither(SuccessType.class, FailType.class) unless we want to take a breaking change. We could possibly also add some config options to the ObjectMapper.

One note,Jackson does have a required property on @JsonProperty, but it only works on @JsonCreator constructor properties.

[mdentremont]

Those options sound good to me. I did see the "required" stuff you mentioned, unfortunately I was hoping to just have all properties be "required" by default, unless they were flagged as "optional", oh well!

[ryber]

yea, I think it's really weird that Jackson hasn't enabled this as even an option

[ryber]

OK, I think I have a good solution that should not be a big breaking change to anyone or a major interface change. We will simply keep the raw body around when the response is >= 400, then it will be available for the .mapError and other post methods.

There are a couple of maybe negatives to this, 1) it still attempts to parse the body in a failure 2) It adds a little memory as we need to keep those strings around. Let me know if this looks reasonable to you https://github.com/Kong/unirest-java/commit/c1eec1ac14e2bbc89fb169f785b2fb352c2bf35e

[mdentremont]

That sounds good to me!

[ryber]

released in 3.4.04

[mdentremont]

Thanks!