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 notifications@github.com 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 https://github.com/square/retrofit/issues/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 notifications@github.com 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 https://github.com/square/retrofit/issues/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 notifications@github.com 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 https://github.com/square/retrofit/issues/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!

[JakeWharton]

Blocked by https://github.com/square/okhttp/issues/5151

[CharlyLafon37]

Hi, are there any updates on this issue ? There's a workaround where we can catch any exception thrown in Interceptors and rethrow them as IOException but it would be nice to have a fix soon :)

[Hodkinson]

Just adding a workaround here in case it helps. Actually in my case even IOExceptions are not propagating back. Retrofit 2.6.2, okhttp 4.2.0

I wanted to deal with this at point of call rather than add an UncaughtExceptionHandler.

So the workaround was to add an interceptor which catches any IOExceptions, and returns a blank response with a magic status code.

private val errorInterceptor = object : Interceptor {
        override fun intercept(chain: Interceptor.Chain): okhttp3.Response {
            return try {
                chain.proceed(chain.request())
            } catch (e: IOException) {
                okhttp3.Response.Builder()
                    .code(418)
                    .request(chain.request())
                    .body(object: ResponseBody() {
                        override fun contentLength() = 0L
                        override fun contentType(): MediaType? = null
                        override fun source(): BufferedSource = Buffer()
                    })
                    .message(e.message ?: e.toString())
                    .protocol(Protocol.HTTP_1_1)
                    .build()
            }
        }
    }

This could be combined with a Result object like in the answer here: https://stackoverflow.com/questions/57625272/retrofit-2-6-0-custom-coroutines-calladapterfactory where the status code or message is checked and a NetworkError returned.

[Nghicv]

Hi @robfletcher, are there any updates on this issue?