RuntimeException thrown in interceptor never notifies Call

[robfletcher]

If an okhttp3.Interceptor throws an exception, the suspend function in a Retrofit interface never completes (neither returning a value or throwing an exception).

A sample project demonstrating the issue with a simple JUnit test can be found here: https://github.com/robfletcher/retrofit-coroutines-hang

The real situation when I've encountered this is when an SSL handshake fails.

FWIW the same behavior is seen using https://github.com/JakeWharton/retrofit2-kotlin-coroutines-adapter with Retrofit 2.5.0

[swankjesse]

When an SSL handshake fails that should throw an IOException, which should be recovered gracefully.

This is an IllegalStateException, which typically results in an application crash.

[robfletcher]

It does the same thing if the interceptor throws IOException.

[JakeWharton]

This should be trivial to test. It's likely we already have coverage for IOException subtypes but probably worth doing both.

…

On Thu, May 30, 2019 at 10:17 PM Jesse Wilson ***@***.***> wrote: When an SSL handshake fails that should throw an IOException, which should be recovered gracefully. This is an IllegalStateException, which typically results in an application crash. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#3110?email_source=notifications&email_token=AAAQIELH4PZJ4S6OKGIKFHDPYCDENA5CNFSM4HROX6D2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWUAIOQ#issuecomment-497550394>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAQIEM4VTTTZOCCCVMXRELPYCDENANCNFSM4HROX6DQ> .

[JakeWharton]

This test shows IOException being propagated properly

https://github.com/square/retrofit/blob/master/retrofit/src/test/java/retrofit2/KotlinSuspendTest.kt#L85-L99

[robfletcher]

Sorry, away from computer right now but does the exception in that case get raised from an interceptor? I’ve seen exceptions propagate successfully in other cases. The issue I’m hitting seems to specifically pertain to interceptors.

I’ll update my example when I get to work in a couple hours.

[JakeWharton]

OkHttp is implemented as a chain of interceptors. The last interceptor just talks to a Socket instead of calling chain.proceed. So yes, but beyond that, Retrofit doesn't care about from where the exception originates in OkHttp since it only talks to OkHttp through one API: Call.enqueue(Callback).

…

On Fri, May 31, 2019 at 9:36 AM Rob Fletcher ***@***.***> wrote: Sorry, away from computer right now but does the exception in that case get raised from an interceptor? I’ve seen exceptions propagate successfully in other cases. The issue I’m hitting seems to specifically pertain to interceptors. I’ll update my example when I get to work in a couple hours. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3110?email_source=notifications&email_token=AAAQIEMFW4X5HATNXRPTOQLPYESXTA5CNFSM4HROX6D2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWVHRXA#issuecomment-497711324>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAQIEPF5526BNR4RCRRJALPYESXTANCNFSM4HROX6DQ> .

[robfletcher]

Ok, my mistake. I could swear I had tested this with IOException but when I updated my example it no longer fails. We must be doing something that ends up throwing another type of exception in our SSL interceptor (integrating with Netflix's metatron cert auth system) or very possibly just doing something fundamentally dumb with coroutines.

Even so, I'm inclined to think that hanging in the case of a RuntimeException is potentially very difficult for people to debug.

Edit: yeah, I tested it with IOException in a more involved integration test within our app, but there must be something else we're doing wrong.

Further edit: found the problem in some (internal) library code we use that wraps checked exceptions up in RuntimeException.

[JakeWharton]

Still probably worth a test

…

On Fri, May 31, 2019 at 12:55 PM Rob Fletcher ***@***.***> wrote: Ok, my mistake. I could swear I had tested this with IOException but when I updated my example it no longer fails. We must be doing something that ends up throwing another type of exception in our SSL interceptor (integrating with Netflix's metatron cert auth system). — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3110?email_source=notifications&email_token=AAAQIEJ3X5JOOVSW333UKFDPYFKA3A5CNFSM4HROX6D2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWVZGBY#issuecomment-497783559>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAQIEOGSGNLPG3C44VTPDTPYFKA3ANCNFSM4HROX6DQ> .

[JakeWharton]

So this is a severe design flaw in OkHttp it turns out. Non-IOExceptions thrown in an enqueue'd request result in no callback whatsoever.

[robfletcher]

Glad I wasn't wasting your time!