ok2c / httpcomponents-jackson

JSON message asynchronous producers and consumers for Apache HttpComponents 5.0 based on Jackson JSON library
https://ok2c.github.io/httpcomponents-jackson
Apache License 2.0
7 stars 6 forks source link

AbstractJsonEntityConsumer prevents setting the failure cause on the result future #3

Closed mihalyr closed 3 years ago

mihalyr commented 3 years ago

Hi Oleg,

thank you for this library, it saved me time and maintenance. I was just beginning to write the usual buffer/serialize code when I stumbled upon your code and was really happy that I could simplify things with it.

While I was testing the code, I came across the below issue:

Send a JSON to a server and make the server respond with corrupted JSON but still setting the content-type header to application/json. I just make the server send back the string "bad json". The expected result is that the future fails with the details of the parsing error, however this is not the case, instead the future is cancelled and the original failure is lost.

Here is the stack from IDEA:

releaseResources:100, AbstractJsonEntityConsumer (com.ok2c.hc5.json.http)
releaseResources:135, JsonMessageConsumer (com.ok2c.hc5.json.http)
failed:121, HttpAsyncMainClientExec$1 (org.apache.hc.client5.http.impl.async)
failed:296, ClientHttp1StreamHandler (org.apache.hc.core5.http.impl.nio)
terminate:184, ClientHttp1StreamDuplexer (org.apache.hc.core5.http.impl.nio)
shutdownSession:156, AbstractHttp1StreamDuplexer (org.apache.hc.core5.http.impl.nio)
onException:385, AbstractHttp1StreamDuplexer (org.apache.hc.core5.http.impl.nio)
exception:90, AbstractHttp1IOEventHandler (org.apache.hc.core5.http.impl.nio)
exception:39, ClientHttp1IOEventHandler (org.apache.hc.core5.http.impl.nio)
onException:162, InternalDataChannel (org.apache.hc.core5.reactor)
handleIOEvent:55, InternalChannel (org.apache.hc.core5.reactor)
processEvents:179, SingleCoreIOReactor (org.apache.hc.core5.reactor)
doExecute:128, SingleCoreIOReactor (org.apache.hc.core5.reactor)
execute:82, AbstractSingleCoreIOReactor (org.apache.hc.core5.reactor)
run:44, IOReactorWorker (org.apache.hc.core5.reactor)
run:834, Thread (java.lang)

It seems that when failed is called, then HttpAsyncClient will make a call to releaseResources() before it sets the failure cause on the future. The problem is that AbstractJsonEntityConsumer calls cancel on the future from releaseResources() and then the future gets completed with a Cancellation preventing the real cause of the failure to bet set. So instead of the parsing error, I end up with a not too useful cancellation.

What is the reason for calling cancel from releaseResources()?

ok2c commented 3 years ago

@mihalyr No reason. Please let me know if 1a9e6bac6871167d5d6998a33cbd2053cfff4740 fixes the problem for you.

mihalyr commented 3 years ago

@ok2c This was super fast! It works perfectly :+1: Thanks a lot!