Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure closing the response body after it was parsed #79

Merged
merged 4 commits into from
Mar 2, 2017

Conversation

lbalmaceda
Copy link
Contributor

Body should be closed after we finish parsing the response. There are some methods like body.string() that close the body internally, but if the call throws an exception we would need to call body.close() manually.

Related: square/okhttp#2311

@@ -136,6 +138,11 @@ protected U parseUnsuccessfulResponse(Response response) {
} catch (IOException e) {
final Auth0Exception auth0Exception = new Auth0Exception("Error parsing the server response", e);
return errorBuilder.from("Request to " + url.toString() + " failed", auth0Exception);
} finally {
try {
body.close();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can body be null?

} finally {
try {
body.close();
} catch (Exception ignored) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we only catch IOException instead all of them so we know when we fail to proper manage the errors?

@lbalmaceda lbalmaceda force-pushed the fixed-close-response-body branch from 61c8b90 to d6eaf7e Compare February 20, 2017 18:47
@hzalaz hzalaz modified the milestone: v1-Next Feb 20, 2017
@lbalmaceda lbalmaceda force-pushed the fixed-close-response-body branch from d6eaf7e to 57fd185 Compare February 20, 2017 19:11
@lbalmaceda lbalmaceda force-pushed the fixed-close-response-body branch from 760ea08 to 43fc54e Compare March 1, 2017 13:22
@hzalaz hzalaz merged commit 3c37a6d into master Mar 2, 2017
@hzalaz hzalaz deleted the fixed-close-response-body branch March 2, 2017 00:07
@lbalmaceda lbalmaceda modified the milestones: 1-Next, 1.6.0 Mar 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants