Exception handling for InputStream

[lightZebra]

Hi there,

I have a problem returning InputStream as body using latest Ring(1.8.2) and Jetty. I noticed that if InputStream returned a few bytes and then an Exception was thrown(could happen in my case) then http-client received partial data but thinking that he received all the data.

Here is test/example of this issue https://gist.github.com/lightZebra/56bc2f21218dbec52214fb0af9adb2b4

If I understood correctly this happened because of with-open inside StreamableResponseBody for InputStream. Exception was thrown and ring closed OutputStream like streaming finished successfully and jetty couldn't finish it correctly I didn't find in Jetty's documentation information that Handler should close OutputStream, so I tried extend-protocol and removed with-open and client received an error, like it should

I'm kinda new to ring and jetty, am I doing something wrong or it is a bug?

[weavejester]

Sorry, could you explain what "client received an error" means? If you've begun writing the response body, that implies you've already sent the response status, unless the body is being buffered.

[lightZebra]

Sorry, I described this not good enough By "client received an error" I mean status:500(ExceptionInfo clj-http: status 500), this is the case with small stream which could be buffered

But the case with bigger stream which couldn't be buffered more interesting For example if in headers we will provide "Transfer-Encoding" "chunked" and provide huge InputStream which couldn't be buffered then current behaviour is next:

1. InputStream will provide some bytes
2. Client will receive status:200 and a few chunks (it's ok)
3. InputStream will throw an Exception
4. Because of with-open Jetty's OutputStream will be close and because of that Jetty will send last terminating chunk(which is indication that server sent all the data and stream is finished correctly) but we didn't send all the data

At step 4 we have a problem, server shouldn't send terminating chunk because stream is not finished correctly. If we will remove with-open and leave only io/copy then server will not send terminating chunk on Exception and connection will closed(which is indication for the client that stream finished incorrectly and all received data should be discarded)

I tried to remove with-open and for chunked stream clj-http.client throw Exception as expected MalformedChunkCodingException CRLF expected at end of chunk org.apache.http.impl.io.ChunkedInputStream.getChunkSize (ChunkedInputStream.java:253)

Here is test/example for long streaming with exception https://gist.github.com/lightZebra/f0734fe58b6cff1caecc2de5d376fa34

[weavejester]

But an error response still isn't guaranteed, correct? Even if the stream isn't closed, there's still a chance that the exception will occur on a chunk boundary.

[lightZebra]

If I understood you correctly then, yes, error response still isn't guaranteed But that's not a problem because without last CRLF(terminating chunk) http-client will discard body during decoding

In case with MalformedChunkCodingException CRLF expected at end of chunk I saw that status:200 was received by client and Exception was thrown during decoding

[weavejester]

So are you proposing replacing with-open with a .close?

[lightZebra]

We might leave only body inside with-open and use .close for output-stream in all the cases. Is it sounds good enough?

[weavejester]

Yes, I think so. We might have to test how it fares on various different third-party adapters, but they should be able to handle exceptions gracefully.

[lightZebra]

Is it okay if I will provide PR for this issue?

[weavejester]

Yep, go ahead. Remember to check through the contributing guidelines.