feign icon indicating copy to clipboard operation
feign copied to clipboard

Supports force disabling closeAfterDecode for configured types

Open quaff opened this issue 4 years ago • 1 comments

Currently Feign supports configure closeAfterDecode globally and enabled by default, It make sense for most use case, except for streaming types. for example, I'm using spring-cloud-openfeign

@GetMapping
public org.springframework.core.io.Resource download();
@GetMapping
public org.springframework.core.io.InputStreamResource streamingDownload();

If the return type is Resource the underlying InputStream is ByteArrayInputStream, response is drained and closed, it's not streaming and not suitable for large stream, then we should use InputStreamResource for real streaming, but the stream is closed by feign.

Caused by: java.io.IOException: closed
	at okio.RealBufferedSource$1.read(RealBufferedSource.java:443)
	at java.io.FilterInputStream.read(FilterInputStream.java:133)
	at java.io.PushbackInputStream.read(PushbackInputStream.java:186)
	at sun.nio.cs.StreamDecoder.readBytes(StreamDecoder.java:284)
	at sun.nio.cs.StreamDecoder.implRead(StreamDecoder.java:326)
	at sun.nio.cs.StreamDecoder.read(StreamDecoder.java:178)
	at java.io.InputStreamReader.read(InputStreamReader.java:184)
	at java.io.BufferedReader.fill(BufferedReader.java:161)
	at java.io.BufferedReader.readLine(BufferedReader.java:324)
	at java.io.BufferedReader.readLine(BufferedReader.java:389)
	at java.io.BufferedReader$1.hasNext(BufferedReader.java:571)
	... 75 more

It would be nice if feign provide method doNotCloseFor like doNotCloseAfterDecode

    public Builder doNotCloseFor(Class<?>... types) {
      // TODO
      return this;
    }

    public Builder doNotCloseAfterDecode() {
      this.closeAfterDecode = false;
      return this;
    }

quaff avatar Jul 27 '21 10:07 quaff

Currently we are bypassing this issue by setting closeOnDecode=false (to keep all streams open).

However, this would lead to connections and streams not being closed accordingly when dealing with normal JSON POJOs and Jackson. Therefore we wrote our own Feign Decoder, which basically just checks if the type is not a stream and closes the stream using the feign.Util.ensureClosed(response) method. As this is the only way of altering the behavior of feign.AsyncResponseHandler.handleResponse(...), as the class is not accessible from outside Feign.

public class AutoClosingResponseEntityDecoder extends ResponseEntityDecoder {

    public AutoClosingResponseEntityDecoder(Decoder decoder) {
        super(decoder);
    }

    @Override
    public Object decode(Response response, Type type) throws IOException {
        Object result = super.decode(response, type);
        if (shouldBeClosedBecauseItsNotAStream(type)) {
            feign.Util.ensureClosed(response);
        }
        return result;
    }
}

Of course this would all not be necessary if the above would be added in the future, I guess more people would appreciate that. Therefore: +1

PatrickRi avatar May 04 '22 15:05 PatrickRi