Okhttp: When Authenticator.authenticate() throws an exception the StreamAllocator isn't released

Created on 20 Feb 2018  路  7Comments  路  Source: square/okhttp

First of all sorry for not providing a failing test here, but I struggled to do that for this case.

I was just implementing an Authenticator when I found that when Authenticator.authenticate() throws an exception (it has a catched IOException in its signature, so that seems to be an expected case) the RetryAndFollowUpInterceptor will never release its StreamAllocation - as opposed to when returning null in authenticate().

Here are the according lines in the RetryAndFollowUpInterceptor:

Request followUp = followUpRequest(response, streamAllocation.route());

if (followUp == null) {
    if (!forWebSocket) {
      streamAllocation.release();
    }
    return response;
}

Shouldn't there be a try catch around the call to followUpRequest() to handle that case and and then re-throw the exception?

bug

All 7 comments

I am new to open source community. Can I work it ?

@CMahesh Before any PR lands there is a contributor agreement to submit, http://square.github.io/okhttp/#contributing

As it's not in scope for 3.10, it seems like a great starter task

@yschimke Thank you.. 馃憤 I have submitted the contributor agreement. I will clone the repo and push the changes.

okhttp3.internal.http.RetryAndFollowUpInterceptor

Request followUp = null;
try{
followUp=followUpRequest(response, streamAllocation.route());
}
catch(IOException e){
streamAllocation.release();
throw e;
}

I have made the changes after downloading the repo.
@yschimke What is the procedure to commit the changes ??

There are a number of tutorials you can read

https://help.github.com/articles/creating-a-pull-request/

The goal is to get your changes showing up here https://github.com/square/okhttp/pulls

https://github.com/square/okhttp/pull/3885

changes commited and pull request created.

@yschimke as per #3971 this seems to be fixed and released. I guess this can be closed.

Was this page helpful?
0 / 5 - 0 ratings