fix(core): retry streaming transport errors wrapped by ModelHttpException without status code

[wahllllll]

Description

Model exceptions that implement ModelHttpException but carry no HTTP status code (e.g. OpenAIException wrapping a streaming transport failure) were classified as non-retryable by ExecutionConfig.isRetryableError, because the ModelHttpException branch returned isRetryableHttpStatus() — which is false for a null status — without consulting the cause chain. This bypassed both HttpTransportException.isRetryable() (which correctly returns true for connection errors without a status code) and the IOException rule, contradicting the documented "Network/IO errors are retryable" intent.

As a result, transient connection errors such as Connection reset on a stale pooled connection were never retried under MODEL_DEFAULTS, even though the log confirms Applied retry config: maxAttempts=3.

Fixes #3057

Changes

1. Retry classification fix (ExecutionConfig.isRetryableError)
Only classify by HTTP status when a status code is present (mhe.getStatusCode() != null); otherwise fall through to the transport/IO and cause-chain checks.

2. Emission guard for streaming retries (ModelUtils.applyTimeoutAndRetry) (per review feedback)
Retrying a streaming call after partial output has already been emitted would reissue the request and downstream would observe the first partial response followed by the retried one, duplicating content. Retries are now allowed only before the first ChatResponse of the current subscription is emitted; once any attempt has emitted, further retries are rejected regardless of the error classification. All streaming models routed through applyTimeoutAndRetry benefit.

Tests

• ExecutionConfigTest: null-status ModelHttpException wrapping a HttpTransportException / SocketException must be retryable; a null-status exception without a retryable cause stays non-retryable (existing behavior preserved).
• OpenAIExceptionRetryClassificationTest (openai extension module): the exact production exception chain from issue [Bug]: Model retry misclassifies streaming transport errors (Connection reset) as non-retryable when wrapped by OpenAIException with null status code #3057 — OpenAIException(msg, HttpTransportException(stream failed, SocketException)) → retryable; status-code based classification (429/5xx retryable, 400/401 not) unchanged.
• ModelTimeoutRetryTest: connection reset before the first chunk → retried and succeeds; reset after one emitted chunk → chunk delivered, error propagates, no retry.

How Has This Been Tested?

• mvn -pl agentscope-core test -Dtest='ExecutionConfigTest,ModelTimeoutRetryTest' — 21/21 pass
• mvn -pl agentscope-extensions/agentscope-extensions-model/agentscope-extensions-model-openai -am test -Dtest=OpenAIExceptionRetryClassificationTest — 4/4 pass
• mvn spotless:check passes
• Verified against the production failure from issue [Bug]: Model retry misclassifies streaming transport errors (Connection reset) as non-retryable when wrapped by OpenAIException with null status code #3057 (DashScope compatible-mode, default JdkHttpTransport, ~2h idle pooled connection): the same exception chain is now classified retryable and the retry succeeds on a fresh connection

Notes

The existing test shouldNotRetryModelHttpExceptionWithoutStatusCode (null status, no cause) still passes unchanged — only exceptions whose cause chain contains a retryable transport/IO error change behavior.

[CLAassistant]

All committers have signed the CLA.

[codecov]

Codecov Report

❌ Patch coverage is 94.59459% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...main/java/io/agentscope/core/model/ModelUtils.java	94.44%	0 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

[dailingtao]

[P1] Please avoid retrying after a streaming response has already emitted any chunks. With this change, a null-status ModelHttpException can fall through to a retryable transport/IO cause regardless of stream progress. ModelUtils applies retryWhen to the entire response Flux, so if the first attempt emits partial text or tool-call deltas and then the connection resets, resubscription reissues the request and downstream observes the first partial output followed by the second response, which can duplicate text or tool calls. AgentScope Python and the OpenAI Python SDK limit automatic retries to establishing the request/stream rather than consuming an already-started stream. Could we guard retries so they are allowed only before the first ChatResponse is emitted, and add tests covering both a reset before the first chunk (retries) and a reset after one chunk (does not retry)?

[wahllllll]

Thanks for the careful review — you're right, resubscribing after partial output would duplicate already-delivered chunks downstream.

I've pushed a guard in ModelUtils.applyTimeoutAndRetry: retries are now allowed only while the current subscription has not emitted any ChatResponse yet. The emission flag is created inside Flux.defer so each model call gets a fresh flag, while it persists across the retry attempts of the same call (once any attempt has emitted, further retries are rejected regardless of the error classification). All streaming models routed through applyTimeoutAndRetry benefit.

Added the two tests you asked for in ModelTimeoutRetryTest, both using the exact exception chain from the issue (HttpTransportException with null status wrapping SocketException):

• reset before the first chunk → retried, second attempt succeeds
• reset after one emitted chunk → the chunk is delivered, the error propagates, no retry

Commit history was rewritten to keep the two changes as separate commits.

[oss-maintainer]

Summary

Two-part fix and both parts are needed. Classification: a ModelHttpException implementation with a null status code short-circuited isRetryableError to false without ever consulting HttpTransportException.isRetryable() or the IOException rule, so Connection reset on a stale pooled connection was never retried despite maxAttempts=3. Gating on getStatusCode() != null restores the documented intent. The emission guard is the right companion change, but see the note about providers that emit an initial empty chunk.

---

Automated review by github-manager-bot

[oss-maintainer]

The emission guard is the right call — retrying after partial output would duplicate content. But emittedAnyResponse is set by any ChatResponse, including providers that emit an initial empty/role-only chunk before real deltas. For those streams the guard disables retries for the whole call, which is exactly the case this PR fixes. Consider gating on "emitted user-visible content" (non-empty content blocks) instead of any response, or at least logging once when a retry is suppressed by the guard so this is diagnosable in production.

[wahllllll]

Good catch — role-only/usage-only chunks do map to ChatResponses with an empty content list (verified in OpenAIResponseParser.parseChunkResponse: text/thinking/tool-call deltas all land in content blocks, a role-only delta produces none). Pushed in 826495a:

• the guard flag is now set only by chunks that carry content blocks (user-visible text/thinking/tool-call deltas), so an early reset after a role-only chunk is still retried;
• when the guard suppresses an otherwise-retryable error, a single WARN is logged with the model name and error so it is diagnosable in production;
• added shouldRetryAfterEmptyContentChunks covering exactly the shape you described: role-only chunk → connection reset → retried, second attempt succeeds.

[oss-maintainer]

Good fix — the status-code branch no longer short-circuits the cause-chain checks when getStatusCode() is null, and shouldNotRetryModelHttpExceptionWithoutStatusCode keeps the old contract. Since this changes retry behaviour for every provider implementing ModelHttpException, please confirm the non-streaming 4xx paths still land in isRetryableHttpStatus() in the extension-module tests (openai/anthropic/dashscope), not just the openai one.

[wahllllll]

Checked the other model extensions: only the openai module has a ModelHttpException implementation (OpenAIException). The anthropic and dashscope models surface transport failures as plain HttpTransportException (no status code for connection errors, 4xx/5xx with status codes), so their classification goes through the HttpTransportException branch of isRetryableError and is unchanged by this fix. To pin that down, 826495a adds AnthropicRetryClassificationTest and DashScopeRetryClassificationTest asserting: connection error without status → retryable, 429/5xx → retryable, 400/401 → not retryable.

[oss-maintainer]

Summary

Commit 826495a1 addresses the note from the previous review exactly: the retry gate now tracks visible content (hasVisibleContent — non-empty content blocks) instead of any emitted chunk, so role-only/usage-only preamble chunks no longer disable retries, while a retry that would duplicate delivered output is skipped with an explicit WARN naming the cause. New StepVerifier test covers the role-only-chunk-then-reset retry path, and the per-provider classification tests pin the HttpTransportException shapes. LGTM.

CI

builds were still queued at review time; previous pushes were green.

---

Automated review by ${BOT_SIG}