From 033f97fc7dec1b34b85d6fc214a1b34cffce39df Mon Sep 17 00:00:00 2001 From: Tadeusz Dudkiewicz Date: Wed, 9 Jul 2025 00:47:18 +0200 Subject: [PATCH] Fix external cancels propagation in rxjava2 and rxjava3 adapters If OkHttp call is cancelled (e.g. because of a timeout) then error is not passed to downstream resulting in a stuck rxjava invocation. Fix it by using `disposed` instead of reusing OkHttp call cancelled state. Separate state was added in 3f111b7b2c8c0e2ce0c4aa8f27bb94dbb6d7385f, for the same reason but it was not used in `CallEnqueueObservable` `onFailure` while it should be (just like it was used in try-catch in `CallExecuteObservable`). --- CHANGELOG.md | 2 +- .../adapter/rxjava2/CallEnqueueObservable.java | 2 +- .../adapter/rxjava2/CancelDisposeTest.java | 15 +++++++++++++++ .../adapter/rxjava3/CallEnqueueObservable.java | 2 +- .../adapter/rxjava3/CancelDisposeTest.java | 15 +++++++++++++++ 5 files changed, 33 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7915b598cf..33f9232ba9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ **Fixed** - - Nothing yet! + - Propagate timeouts and other external cancels in retrofit-adapter for rxjava2 and rxjava3 ## [3.0.0] - 2025-05-15 diff --git a/retrofit-adapters/rxjava2/src/main/java/retrofit2/adapter/rxjava2/CallEnqueueObservable.java b/retrofit-adapters/rxjava2/src/main/java/retrofit2/adapter/rxjava2/CallEnqueueObservable.java index a6bac4e8b0..73974bfe53 100644 --- a/retrofit-adapters/rxjava2/src/main/java/retrofit2/adapter/rxjava2/CallEnqueueObservable.java +++ b/retrofit-adapters/rxjava2/src/main/java/retrofit2/adapter/rxjava2/CallEnqueueObservable.java @@ -82,7 +82,7 @@ public void onResponse(Call call, Response response) { @Override public void onFailure(Call call, Throwable t) { - if (call.isCanceled()) return; + if (disposed) return; try { observer.onError(t); diff --git a/retrofit-adapters/rxjava2/src/test/java/retrofit2/adapter/rxjava2/CancelDisposeTest.java b/retrofit-adapters/rxjava2/src/test/java/retrofit2/adapter/rxjava2/CancelDisposeTest.java index 031c219510..696816c4d7 100644 --- a/retrofit-adapters/rxjava2/src/test/java/retrofit2/adapter/rxjava2/CancelDisposeTest.java +++ b/retrofit-adapters/rxjava2/src/test/java/retrofit2/adapter/rxjava2/CancelDisposeTest.java @@ -21,7 +21,12 @@ import io.reactivex.Observable; import io.reactivex.disposables.Disposable; +import io.reactivex.observers.TestObserver; + +import java.io.IOException; import java.util.List; +import java.util.concurrent.TimeUnit; + import okhttp3.Call; import okhttp3.OkHttpClient; import okhttp3.mockwebserver.MockWebServer; @@ -79,4 +84,14 @@ public void cancelDoesNotDispose() { calls.get(0).cancel(); assertFalse(disposable.isDisposed()); } + + @Test + public void cancelSetsError() throws InterruptedException { + TestObserver testObserver = service.go().test(); + List calls = client.dispatcher().runningCalls(); + assertEquals(1, calls.size()); + calls.get(0).cancel(); + testObserver.await(10, TimeUnit.SECONDS); + testObserver.assertError(e -> e instanceof IOException && "Canceled".equals(e.getMessage())); + } } diff --git a/retrofit-adapters/rxjava3/src/main/java/retrofit2/adapter/rxjava3/CallEnqueueObservable.java b/retrofit-adapters/rxjava3/src/main/java/retrofit2/adapter/rxjava3/CallEnqueueObservable.java index 1152187487..c2f28e01c0 100644 --- a/retrofit-adapters/rxjava3/src/main/java/retrofit2/adapter/rxjava3/CallEnqueueObservable.java +++ b/retrofit-adapters/rxjava3/src/main/java/retrofit2/adapter/rxjava3/CallEnqueueObservable.java @@ -82,7 +82,7 @@ public void onResponse(Call call, Response response) { @Override public void onFailure(Call call, Throwable t) { - if (call.isCanceled()) return; + if (disposed) return; try { observer.onError(t); diff --git a/retrofit-adapters/rxjava3/src/test/java/retrofit2/adapter/rxjava3/CancelDisposeTest.java b/retrofit-adapters/rxjava3/src/test/java/retrofit2/adapter/rxjava3/CancelDisposeTest.java index 072035d312..0ede1553e2 100644 --- a/retrofit-adapters/rxjava3/src/test/java/retrofit2/adapter/rxjava3/CancelDisposeTest.java +++ b/retrofit-adapters/rxjava3/src/test/java/retrofit2/adapter/rxjava3/CancelDisposeTest.java @@ -21,7 +21,12 @@ import io.reactivex.rxjava3.core.Observable; import io.reactivex.rxjava3.disposables.Disposable; +import io.reactivex.rxjava3.observers.TestObserver; + +import java.io.IOException; import java.util.List; +import java.util.concurrent.TimeUnit; + import okhttp3.Call; import okhttp3.OkHttpClient; import okhttp3.mockwebserver.MockWebServer; @@ -79,4 +84,14 @@ public void cancelDoesNotDispose() { calls.get(0).cancel(); assertFalse(disposable.isDisposed()); } + + @Test + public void cancelSetsError() throws InterruptedException { + TestObserver testObserver = service.go().test(); + List calls = client.dispatcher().runningCalls(); + assertEquals(1, calls.size()); + calls.get(0).cancel(); + testObserver.await(10, TimeUnit.SECONDS); + testObserver.assertError(e -> e instanceof IOException && "Canceled".equals(e.getMessage())); + } }