Skip to content

Commit

Permalink
Add synchronous exception workaround for Response requests
Browse files Browse the repository at this point in the history
(cherry picked from commit 02343da)
  • Loading branch information
JakeWharton committed Sep 24, 2019
1 parent 4fe1b2b commit 6713c84
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 2 deletions.
8 changes: 7 additions & 1 deletion retrofit/src/main/java/retrofit2/HttpServiceMethod.java
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,13 @@ static final class SuspendForResponse<ResponseT> extends HttpServiceMethod<Respo
//noinspection unchecked Checked by reflection inside RequestFactory.
Continuation<Response<ResponseT>> continuation =
(Continuation<Response<ResponseT>>) args[args.length - 1];
return KotlinExtensions.awaitResponse(call, continuation);

// See SuspendForBody for explanation about this try/catch.
try {
return KotlinExtensions.awaitResponse(call, continuation);
} catch (Exception e) {
return KotlinExtensions.yieldAndThrow(e, continuation);
}
}
}

Expand Down
24 changes: 23 additions & 1 deletion retrofit/src/test/java/retrofit2/KotlinSuspendTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ class KotlinSuspendTest {
assertThat(body).isEqualTo("HiHiHiHiHi")
}

@Test fun checkedExceptionsAreNotSynchronouslyThrown() = runBlocking {
@Test fun checkedExceptionsAreNotSynchronouslyThrownForBody() = runBlocking {
val retrofit = Retrofit.Builder()
.baseUrl("https://unresolved-host.com/")
.addConverterFactory(ToStringConverterFactory())
Expand All @@ -295,4 +295,26 @@ class KotlinSuspendTest {
}
}
}

@Test fun checkedExceptionsAreNotSynchronouslyThrownForResponse() = runBlocking {
val retrofit = Retrofit.Builder()
.baseUrl("https://unresolved-host.com/")
.addConverterFactory(ToStringConverterFactory())
.build()
val example = retrofit.create(Service::class.java)

server.shutdown()

// The problematic behavior of the UnknownHostException being synchronously thrown is
// probabilistic based on thread preemption. Running a thousand times will almost always
// trigger it, so we run an order of magnitude more to be safe.
repeat(10000) {
try {
example.response()
fail()
} catch (_: IOException) {
// We expect IOException, the bad behavior will wrap this in UndeclaredThrowableException.
}
}
}
}

0 comments on commit 6713c84

Please sign in to comment.