Skip to content

Commit 461f7d6

Browse files
google-genai-botcopybara-github
authored andcommitted
fix: close the Gemini response stream after a read error
PiperOrigin-RevId: 1008122641
1 parent 367b402 commit 461f7d6

3 files changed

Lines changed: 54 additions & 12 deletions

File tree

‎core/src/main/java/com/google/adk/models/Gemini.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -330,26 +330,26 @@ private static GenerateContentConfig withTrackingHeaders(@Nullable GenerateConte
330330
}
331331

332332
/**
333-
* Reads the stream and, when the subscriber cancels, closes it on {@link Schedulers#io()} so a
334-
* partial read doesn't leave the HTTP response open. The close waits for the stream to open and
335-
* can block behind a {@code readLine()} in progress. An iterator call that returns after the
336-
* cancel closes the stream again, covering a continuation request opened during that call.
333+
* Reads the stream and closes it on {@link Schedulers#io()} unless it was read to the end, so a
334+
* cancel or a read error doesn't leave the HTTP response open. The close waits for the stream to
335+
* open and can block behind a {@code readLine()} in progress. An iterator call that returns after
336+
* the cancel closes the stream again, covering a continuation request opened during that call.
337337
*/
338338
@VisibleForTesting
339339
static <T, S extends Iterable<T> & AutoCloseable> Flowable<T> readStream(
340340
CompletableFuture<S> streamFuture) {
341341
return Flowable.defer(
342342
() -> {
343343
AtomicBoolean cancelled = new AtomicBoolean();
344-
AtomicBoolean terminated = new AtomicBoolean();
344+
AtomicBoolean completed = new AtomicBoolean();
345345
return Flowable.using(
346346
() -> streamFuture,
347347
future ->
348348
Flowable.fromFuture(future)
349349
.flatMapIterable(stream -> closingAfterCancel(stream, cancelled))
350-
.doOnTerminate(() -> terminated.set(true)),
350+
.doOnComplete(() -> completed.set(true)),
351351
future -> {
352-
if (!terminated.get()) {
352+
if (!completed.get()) {
353353
cancelled.set(true);
354354
var unused = future.thenAccept(Gemini::closeAsync);
355355
}

‎core/src/test/java/com/google/adk/flows/llmflows/BaseLlmFlowCancelTest.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import java.util.Map;
5252
import java.util.Optional;
5353
import java.util.concurrent.CountDownLatch;
54+
import java.util.concurrent.atomic.AtomicInteger;
5455
import org.junit.Test;
5556
import org.junit.runner.RunWith;
5657
import org.junit.runners.JUnit4;
@@ -112,7 +113,7 @@ public void run_disposedBeforeFunctionCallArrives_doesNotRunTool() throws Except
112113
responses.onNext(createLlmResponse(FUNCTION_CALL));
113114
responses.onComplete();
114115

115-
assertThat(tool.ran.getCount()).isEqualTo(1);
116+
assertThat(tool.runCount.get()).isEqualTo(0);
116117
assertThat(testLlm.getRequests()).hasSize(1);
117118
}
118119

@@ -129,7 +130,7 @@ public void run_consumedToCompletion_runsToolAndEmitsEveryEvent() throws Excepti
129130

130131
List<Event> events = createBaseLlmFlow().run(invocationContext).toList().blockingGet();
131132

132-
assertThat(tool.ran.getCount()).isEqualTo(0);
133+
assertThat(tool.runCount.get()).isEqualTo(1);
133134
assertThat(events).hasSize(3);
134135
assertThat(events.get(2).content()).hasValue(TEXT);
135136
assertThat(testLlm.getRequests()).hasSize(2);
@@ -283,7 +284,7 @@ protected Flowable<Event> runLiveImpl(InvocationContext invocationContext) {
283284
}
284285

285286
private static final class RecordingTool extends BaseTool {
286-
final CountDownLatch ran = new CountDownLatch(1);
287+
final AtomicInteger runCount = new AtomicInteger();
287288

288289
RecordingTool() {
289290
super("my_function", "tool description for my_function");
@@ -296,7 +297,7 @@ public Optional<FunctionDeclaration> declaration() {
296297

297298
@Override
298299
public Single<Map<String, Object>> runAsync(Map<String, Object> args, ToolContext toolContext) {
299-
ran.countDown();
300+
runCount.incrementAndGet();
300301
return Single.just(ImmutableMap.of("response", "response for my_function"));
301302
}
302303
}

‎core/src/test/java/com/google/adk/models/GeminiTest.java‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2234,6 +2234,18 @@ public FakeStream get() throws InterruptedException, ExecutionException {
22342234
subscriber.assertNoValues();
22352235
}
22362236

2237+
@Test
2238+
public void readStream_errorMidStream_closesStream() throws Exception {
2239+
IllegalStateException failure = new IllegalStateException("stream failed");
2240+
FakeStream stream = new FailingStream(2, failure);
2241+
2242+
Gemini.readStream(CompletableFuture.completedFuture(stream))
2243+
.test()
2244+
.assertValues("chunk-1", "chunk-2")
2245+
.assertError(failure);
2246+
assertThat(stream.closed.await(5, SECONDS)).isTrue();
2247+
}
2248+
22372249
@Test
22382250
public void readStream_readsToEnd_emitsEveryResponse() {
22392251
FakeStream stream = new FakeStream(3);
@@ -2245,7 +2257,7 @@ public void readStream_readsToEnd_emitsEveryResponse() {
22452257
}
22462258

22472259
/** A stream of "chunk-N" strings that records when it is closed; unbounded by default. */
2248-
private static final class FakeStream implements Iterable<String>, AutoCloseable {
2260+
private static class FakeStream implements Iterable<String>, AutoCloseable {
22492261
final CountDownLatch closed = new CountDownLatch(1);
22502262
private final int size;
22512263

@@ -2267,4 +2279,33 @@ public void close() {
22672279
closed.countDown();
22682280
}
22692281
}
2282+
2283+
/** A {@link FakeStream} of {@code size} chunks whose iterator then throws {@code failure}. */
2284+
private static final class FailingStream extends FakeStream {
2285+
private final RuntimeException failure;
2286+
2287+
FailingStream(int size, RuntimeException failure) {
2288+
super(size);
2289+
this.failure = failure;
2290+
}
2291+
2292+
@Override
2293+
public Iterator<String> iterator() {
2294+
Iterator<String> chunks = super.iterator();
2295+
return new Iterator<String>() {
2296+
@Override
2297+
public boolean hasNext() {
2298+
if (!chunks.hasNext()) {
2299+
throw failure;
2300+
}
2301+
return true;
2302+
}
2303+
2304+
@Override
2305+
public String next() {
2306+
return chunks.next();
2307+
}
2308+
};
2309+
}
2310+
}
22702311
}

0 commit comments

Comments
 (0)