Skip to content

Commit 2a3aefb

Browse files
fix: route SessionPoolMap.apply op throws to failed futures
`apply` already converted lookup throws to failed futures, but a sync throw from `op.apply(v)` itself (NPE on malformed input, REE during shutdown, any RuntimeException from the wrapped call chain) propagated to the caller — contradicting the docstring's contract that "callers consistently observe failures through the future surface." Wrap the op invocation in try/catch with the same pattern as the lookup arm.
1 parent 3508eb3 commit 2a3aefb

2 files changed

Lines changed: 33 additions & 4 deletions

File tree

java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/compat/ops/SessionPoolMap.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,10 @@ public V get(K key) {
5454
}
5555

5656
/**
57-
* Looks up the cached value and applies {@code op}. If the lookup throws (e.g. {@link
58-
* IllegalStateException} from a closed Client), the throw is converted to a failed future so
59-
* callers consistently observe failures through the future surface.
57+
* Looks up the cached value and applies {@code op}. Both the lookup (e.g. {@link
58+
* IllegalStateException} from a closed Client) and a synchronous throw from {@code op.apply}
59+
* (e.g. NPE on malformed input, RejectedExecutionException during shutdown) are converted to a
60+
* failed future so callers consistently observe failures through the future surface.
6061
*/
6162
public <R> CompletableFuture<R> apply(K key, Function<V, CompletableFuture<R>> op) {
6263
V v;
@@ -67,7 +68,13 @@ public <R> CompletableFuture<R> apply(K key, Function<V, CompletableFuture<R>> o
6768
f.completeExceptionally(e);
6869
return f;
6970
}
70-
return op.apply(v);
71+
try {
72+
return op.apply(v);
73+
} catch (Throwable t) {
74+
CompletableFuture<R> f = new CompletableFuture<>();
75+
f.completeExceptionally(t);
76+
return f;
77+
}
7178
}
7279

7380
/** Evicts all entries, triggering Closeable.close on each via the cache's removal listener. */

java-bigtable/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/compat/ops/SessionPoolMapTest.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,28 @@ void apply_convertsLoaderThrowToFailedFuture() throws Exception {
7272
assertThat(ee).hasCauseThat().hasMessageThat().isEqualTo("Client is closed");
7373
}
7474

75+
@Test
76+
void apply_convertsOpSyncThrowToFailedFuture() throws Exception {
77+
// op.apply may throw synchronously (NPE on malformed input, REE during shutdown, any
78+
// RuntimeException from the wrapped call chain). The async surface must not propagate
79+
// the throw — same contract as the loader-throw path above.
80+
CountingHandle handle = new CountingHandle();
81+
SessionPoolMap<String, CountingHandle> map = new SessionPoolMap<>(key -> handle);
82+
83+
CompletableFuture<String> result =
84+
map.apply(
85+
"k",
86+
v -> {
87+
throw new IllegalStateException("op blew up");
88+
});
89+
90+
assertThat(result.isCompletedExceptionally()).isTrue();
91+
ExecutionException ee =
92+
assertThrows(ExecutionException.class, () -> result.get(1, TimeUnit.SECONDS));
93+
assertThat(ee).hasCauseThat().isInstanceOf(IllegalStateException.class);
94+
assertThat(ee).hasCauseThat().hasMessageThat().isEqualTo("op blew up");
95+
}
96+
7597
@Test
7698
void apply_happyPathInvokesOp() throws Exception {
7799
CountingHandle handle = new CountingHandle();

0 commit comments

Comments
 (0)