How to up/download huge files with Feign?

[dibog]

I have to up/download huge files, bigger then fits into memory.
For the download I could use the Response object as it gives me the
(hopefully) unbuffered input stream. But how could I send big data
within a request.

I would like to avoid multipart files or similar. I would like to push the
binary data direct behind the request object.

Ideally, I would like to be able to stream data from a response object to a request object without
the need to write it into the file system.

Currently, I got stuck with the upload part. The encoder expects that I write the data either as string or byte[]
into the RequestTemplate. Is there no way to attach there an output stream?

Thanks,
Dieter

[velo]

Hrmm, that is a serious problem... from long time, feign has cache the date into memory using a byte[].

It would be a massive improvement for feign 11 to add the ability to stream data. And breaking some of the compatibility for the next major to include this functionality would be ok

[kdavisk6]

We could consider adding support for a StreamingResponse implementation. Something we would need to design more. @dibog we are open to suggestions. Any thoughts on how you would like this to work?

[dibog]

I had the impression that the Reponse object already allows me to stream the content of the response body without that it was buffered in memory or in the file system.

So what is IMHO missing is a way to fill the request body without it being cached. If I use an Encoder I have to fill the Request.Body object with a byte[]. I would like to store there a reference to an InputStream. That way I'm free to deliver an InputStream implementation of my choice, that could be something in memory, or a FileInputStream, or stream which I got from anywhere else.

[velo]

You are right @dibog , the request loads the contents into memory, being a problem for large payloads.

[dibog]

But the question: Is there a good reason to do that? To compute the Content-Length you would require it,
but you can indicate that you don't know the Content-Length. It would be great if the Request mechanism does not expect
to get an byte[] but an stream. The system could check if the body is from an bufferable resource (like byte[] or File/Path) or if it is not bufferable.

[velo]

Yeah, we would need to change the encoder to be more a streaming resource instead of accumulating all in a byte[]

[MrSoftwareEngineer]

I was impressed when I opened Request.Body implementation. There is no a workaround excepting different clients using for lightweight request and upload huge files

[lsubbare]

@dibog @velo @kdavisk6 Is the response.body().asInputStream loads whole file into memory? I thought it just streams.

[k-boyle]

Any update on this?

[velo]

Any update on this?

It's still waiting on a community member to raise a PR with a fix.

[trumpetinc]

I've been doing some experimentation on this issue, and I observe that Response does indeed return the original stream from the HTTP connect - as long as nothing (like request loggers, or a decoder) has converted it into a ByteArrayInputStream.

If we return Response, then the Jackson decoder doesn't interfere, and we can adjust the feign.Logger logAndRebufferResponse() method so it only does response body logging for specific content types.

So unless I'm missing something, I think that for downloads there is definitely a way to retrieve large files with Feign without clobbering memory - like this:

			// only log body if response is json
			Set<String> loggedContentTypes = new HashSet<>(Arrays.asList("application/json", "text/html"));
			String contentType = response.headers().get("content-type").stream().findFirst().orElse("UNKNOWN");
			if (!loggedContentTypes.contains(contentType)) {
 			        String contentLength = response.headers().get("content-length").stream().findFirst().orElse("UNKNOWN");
				logDebug(configKey, "<--- END HTTP (Non Logged Body, %s-bytes)", contentLength);
				return response;
			}

If I make the above change, and I use the Response return type, I am able to get an input stream of type HttpURLConnection$HttpInputStream, which I'm pretty sure is what we want for non-memory intensive downloads.

Does anyone see any problem with this? Ideally, we'd have an InputStreamDecoder, but that would require some changes in AsyncResponseHandler.handleResponse (basically, if the response type is Closable, the response body itself should probably not be auto-closed).

I did a quick glance at the code for sending, and making that stream friendly would be more work (Body has a byte[] representation hard coded into it) - but the underlying Client$Default is stream oriented, so there may be a clean way to introduce a stream representation into the Body - but it would definitely require some surgery.

One big problem with making the upload streamable is that it kills the ability to replay. I think there are ways to make this coexist with the existing replay behavior - but it would require some discussion with the maintainers...

[trumpetinc]

@kdavisk6 I wanted to tag you to see if this issue is still on your radar. I've done a bit of digging, and I think that I could create a pull request that would accommodate streaming for uploads and downloads, but I am uncertain about the process. I don't want to fork and spend a ton of time creating a pull request that won't be accepted.

[trumpetinc]

FYI - I have a proof of concept that I think will mesh nicely into the existing library - but it will require breaking changes to the API (there's just no getting around the current Request.Body implementation). My proof of concept involved creating my own Client implementation (not compatible with the current Client interface).

Ironically, I was able to use Response.Body as-is - even for Requests. Someone clearly had their eye on this as a possibility. So I think there is a clear path to having a unified Body implementation that isn't specific to requests or responses.

Really hoping to get a discussion going about whether a pull request will be considered - and if so, discuss tradeoffs of various approaches.

[trumpetinc]

Note for myself:

if we integrate this back into Feign proper, SynchronousMethodHandler.invoke() has retry logic. This will somehow need to know whether the request is retryable or not.

[kdavisk6]

@trumpetinc Thanks for your offering your help. I like the idea of an InputStreamDecoder over making breaking changes to SynchronousMethodHandler. It fits more with our intention of extension and could provide an simple way to validate your ideas. Feel free to fork the repository and submit a PR for review.