Skip to content

fix(model/openai): wrap streaming branch in Flux.defer to make retry re-issue HTTP requests#2079

Open
birdie7761 wants to merge 4 commits into
agentscope-ai:mainfrom
birdie7761:fix/openai-streaming-retry
Open

fix(model/openai): wrap streaming branch in Flux.defer to make retry re-issue HTTP requests#2079
birdie7761 wants to merge 4 commits into
agentscope-ai:mainfrom
birdie7761:fix/openai-streaming-retry

Conversation

@birdie7761

Copy link
Copy Markdown
Contributor

Description

Fixes a bug where streaming retry in OpenAIChatModel did not re-issue HTTP requests for retryable errors such as HTTP 429.

Root cause

OpenAIChatModel.doStream0() returned the streaming Flux directly:

  return client.stream(...).map(...).filter(...);

When ModelUtils.applyTimeoutAndRetry() later applied retryWhen, re-subscriptions reused the same publisher. For transports whose stream() is backed by a one-shot future (notably the default JdkHttpTransport), the same failed future was replayed
instead of sending a new HTTP request. As a result, a maxAttempts=3 retry exhausted after a single server hit.

Fix

Wrap the streaming branch in Flux.defer(), matching the existing non-streaming branch, so every retry creates a fresh publisher and re-issues the HTTP request:

  return Flux.defer(() -> client.stream(...).map(...).filter(...))
          .subscribeOn(Schedulers.boundedElastic());

Verification

Added OpenAIChatModelStreamingRetryTest with MockWebServer scenarios:

• Streaming 429 exhausts retries and hits the server maxAttempts times.
• Streaming 429 retries then succeeds on the final attempt.
• Streaming 400 does not retry.
• Streaming 429 retry re-issues requests for JdkHttpTransport, OkHttpTransport, and the default transport.
• Non-streaming 429 retry re-issues requests as a control.

All tests pass locally:

  mvn spotless:apply
  mvn test -pl agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai

Checklist

• [x] Code has been formatted with mvn spotless:apply
• [x] All tests are passing (mvn test)
• [x] Javadoc comments are complete and follow project conventions
• [ ] Related documentation has been updated (not needed for this internal bug fix)
• [x] Code is ready for review

…re-issue HTTP requests

OpenAIChatModel.doStream0's streaming branch returned a Flux tied to a
single transport.stream() invocation. For transports whose stream() is
backed by a one-shot future (e.g. JdkHttpTransport), retryWhen re-
subscriptions replayed the already-failed publisher instead of sending a
new HTTP request. This caused 429/5xx retry to exhaust after a single
attempt.

Wrap the streaming branch in Flux.defer, matching the non-streaming
branch, so each retry creates a fresh publisher and re-issues the HTTP
request.

Add OpenAIChatModelStreamingRetryTest to verify that streaming 429
responses are retried and hit the server maxAttempts times for
OkHttpTransport, JdkHttpTransport, and the default transport.
@birdie7761 birdie7761 requested a review from a team July 9, 2026 07:52
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@oss-maintainer oss-maintainer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Fixes a bug where streaming retry in OpenAIChatModel did not re-issue HTTP requests for retryable errors (e.g. HTTP 429). Root cause: the streaming Flux was returned directly, so re-subscriptions replayed the same one-shot failed future instead of creating a new HTTP request.

Findings

  • [Info] OpenAIChatModel.java:310 — Wrapping in Flux.defer() + subscribeOn(Schedulers.boundedElastic()) is the correct fix, matching the existing non-streaming branch pattern.
  • [Info] OpenAIChatModelStreamingRetryTest.java — Excellent test coverage with MockWebServer: 429 exhaust, 429→success, 400 no-retry, multi-transport verification. The control test for non-streaming is a nice touch.

Verdict

Minimal, correct fix with thorough tests. LGTM.


Automated review by github-manager

@AgentScopeJavaBot AgentScopeJavaBot added bug Something isn't working area/core/model Model providers and formatters labels Jul 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/core/model Model providers and formatters bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants