palantir / palantir/conjure-java

Errors not handled properly when streaming responses

Open
#16 2 comments 1 reaction 1 assignee View on GitHub

@iamdanfox is already working on this.

Since Jun 6, 2018.

Dominant language
Java
Stars
39
Forks
49
Avg merge
8h 22m
Merged PRs (30d)
32

Description

Currently, when an endpoint returns binary we render a StreamingOutput in the jersey interface. Because the Content-Length is unknown, Jetty properly responds using chunked transfer encoding which allows the client to determine whether the stream has been fully received or failed part way through. There's an issue in Jersey, though, which actually closes the stream when an exception is thrown after some bytes have been written to the client which in turn causes Jetty to close the stream as if it were completed successfully: https://en.wikipedia.org/wiki/Chunked_transfer_encoding

There's not much we can do about streaming back things of unknown size, but we should be able to support streams where the size is known. I'd propose that we add a BinaryStream or Binary or something which takes the place of StreamingOutput in conjure generated interfaces and has two methods:

of(StreamingOutput)
of(StreamingOutput, long contentLength)

think it may be reasonable to stick this in remoting, but sounds like these two projects are converging and we're going to need to change the conjure code gen at some point.

This change also doesn't affect clients of these endpoints because they must use the Retrofit client when consuming streaming endpoints.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.