Close OkHttp response body #945
Conversation
|
how is this change closing the body? can u explain? |
| try (ResponseBody body = okHttpResponse.body()) { | ||
| return new Response(okHttpResponse.code(), okHttpResponse.message(), headersMap, | ||
| body == null ? null : body.byteStream()); | ||
| } | ||
| } |
There was a problem hiding this comment.
shouldn't there be a finally? i don't get how this closes the body.
There was a problem hiding this comment.
This is a try-with-resources. https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html
There was a problem hiding this comment.
Hi, @drei01 , ResponseBody shouldn't be closed here in such way.
it is passed to the com.github.scribejava.core.model.Response.
and is used by it to read body stream later.
Developers should close com.github.scribejava.core.model.Response. And it will close the stream.
Response implements Closeable and have method
@Override
public void close() throws IOException {
if (stream != null) {
stream.close();
}
}
There was a problem hiding this comment.
@kullfar thanks for the clarification. I'll close this PR.
|
But.... You're right, that only byteStream will be closed, not the ResponseBody. I will make some changes soon. |
|
that's it |
Response body must be closed, see square/okhttp#2311