Skip to content

[codex] Retry empty AI stream responses#576

Merged
buger merged 2 commits into
mainfrom
retry-empty-ai-stream-575
Jun 12, 2026
Merged

[codex] Retry empty AI stream responses#576
buger merged 2 commits into
mainfrom
retry-empty-ai-stream-575

Conversation

@buger

@buger buger commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes #575.

This changes the AI request path so empty-output failures that happen after streamText() returns are retried by the same retry budget that already handles provider stream creation. The retry scope now covers both streamText() and post-stream consumption (result.steps, result.text, and usage recording) without adding a nested RetryManager.

What changed

  • Added Vercel AI SDK empty-output failures to the default retryable error patterns:
    • No output generated
    • AI_NoOutputGeneratedError
  • Extended streamTextWithRetryAndFallback() with an optional consumer callback so post-stream consumption can run inside the existing provider retry attempt.
  • Re-invokes the full AI request when result.steps rejects with an empty-output error.
  • Treats non-schema responses with no steps and no text as retryable empty-output failures.
  • Preserves schema/null-guard behavior where structured-output paths may legitimately have empty text and empty steps while result.output carries the payload.
  • Centralized RetryManager construction in ProbeAgent and normalized jitter handling.
  • Matches retryable errors by error.name / constructor name as well as message, code, type, and status.
  • Keeps tool-call listener registration scoped to each retry attempt so cleanup does not disable listener tracking on a retry.

Tests

Focused tests added/updated:

  • npm/tests/unit/retryManager.test.js
    • verifies No output generated and AI_NoOutputGeneratedError are retryable.
    • verifies jitter normalization.
  • npm/tests/unit/last-step-text.test.js
    • retries when result.steps rejects with AI_NoOutputGeneratedError.
    • retries when provider resolves empty steps and empty text.
    • stops retrying after the configured retry budget is exhausted.
    • asserts the high-level stream call is made once while retry attempts happen inside the single retry manager.
  • npm/tests/unit/answer-null-guard.test.js
    • verifies non-schema empty streams now retry and fail after budget exhaustion.
    • keeps schema-output null guards covered.

Validation run:

cd npm
npm test -- --runInBand tests/unit/retryManager.test.js tests/unit/last-step-text.test.js tests/unit/answer-null-guard.test.js

Result: 3 test suites passed, 36 tests passed.

Additional check:

git diff --check

Result: passed.

I also previously ran the broader unit suite with:

cd npm
npm test -- --runInBand tests/unit

That run had the new/changed tests passing, but the full suite did not complete green locally because several existing tests try to download the missing release asset probe-v0.6.0-rc88-x86_64-unknown-linux-musl.tar.gz from GitHub and receive 404s. It also surfaced existing symbol-edit failures unrelated to this retry change.

@probelabs

probelabs Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

PR Overview: Retry Empty AI Stream Responses

Summary

This PR fixes issue #575 by extending the retry mechanism to handle empty AI stream responses that occur after streamText() completes. Previously, only failures during the initial streamText() call were retried. Now, failures during post-stream consumption (when accessing result.steps, result.text, or usage) are also retried when they indicate empty output.

Files Changed

  • npm/src/agent/ProbeAgent.js (+117/-87)

    • Added _createRetryManager() helper method for consistent RetryManager instantiation
    • Added consumeResult callback parameter to _executeWithVercelProvider() and streamTextWithRetryAndFallback()
    • Added empty-output detection for non-schema responses (throws AI_NoOutputGeneratedError)
    • Wrapped post-stream consumption logic inside the retry scope via consumeResult callback
    • Added jitter option propagation to RetryManager initialization
  • npm/src/agent/RetryManager.js (+6/-2)

    • Added 'No output generated' and 'AI_NoOutputGeneratedError' to DEFAULT_RETRYABLE_ERRORS
    • Added errorName field to isRetryableError() pattern matching
    • Fixed jitter option type normalization to boolean
  • npm/tests/unit/answer-null-guard.test.js (+63/-38)

    • Updated test to verify non-schema empty streams now retry and fail after budget exhaustion
    • Updated test mocks to use consumeResult callback pattern
  • npm/tests/unit/last-step-text.test.js (+120/-12)

    • Added 3 new tests covering retry behavior for empty-output errors
    • Added helper functions for retry testing (createNoOutputError(), mockStreamText(), mockStreamTextWithRetry())
  • npm/tests/unit/retryManager.test.js (+16/-0)

    • Added test verifying Vercel AI SDK empty-output errors are retryable
    • Added test for jitter option type normalization

Architecture & Impact

Problem Solved

The Vercel AI SDK's streamText() can return successfully but then fail during consumption:

  • result.steps rejects with AI_NoOutputGeneratedError
  • result.steps resolves to empty array AND result.text is empty

These scenarios were not retried, causing unnecessary failures.

Key Changes

  1. Retryable Error Patterns (RetryManager.js:28-30)

    • Added 'No output generated' and 'AI_NoOutputGeneratedError' to DEFAULT_RETRYABLE_ERRORS
    • Enhanced isRetryableError() to check error.name field
  2. Post-Stream Retry Scope (ProbeAgent.js:1463-1532, ProbeAgent.js:1684-1761)

    • Added consumeResult callback parameter to _executeWithVercelProvider() and streamTextWithRetryAndFallback()
    • Post-stream consumption logic (steps/text extraction, usage recording) is now passed as a callback
    • The callback is executed inside the retry scope, allowing empty-output failures to trigger retries
    • Retries re-execute the entire AI request (including streamText())
  3. Empty-Output Detection (ProbeAgent.js:4477-4485)

    • Added check after result.steps and result.text resolution
    • For non-schema responses: throws AI_NoOutputGeneratedError when both steps and text are empty
    • Preserves existing behavior for schema responses (may legitimately have empty text/steps)
  4. Configuration Propagation (ProbeAgent.js:1469-1476, ProbeAgent.js:1517-1521)

    • Added jitter option to RetryManager initialization
    • Created _createRetryManager() helper for consistent instantiation

Component Flow

flowchart TD
    A[answer method] --> B[streamTextWithRetryAndFallback]
    B --> C[_executeWithVercelProvider]
    C --> D[retryManager.executeWithRetry]
    D --> E[streamText call]
    E --> F{consumeResult callback?}
    F -->|Yes| G[Post-stream consumption]
    G --> H[await result.steps]
    G --> I[await result.text]
    H --> J{Empty?}
    I --> J
    J -->|Non-schema + empty| K[throw AI_NoOutputGeneratedError]
    J -->|Schema or has content| L[Return finalText]
    K --> M[Retry via retryManager]
    M --> D
Loading

Scope & Boundaries

In Scope:

  • Retry logic for empty AI stream responses in non-schema mode
  • Configuration propagation for jitter option
  • Test coverage for new retry scenarios

Out of Scope:

  • Changes to schema-output handling (preserves existing null-guard behavior)
  • Changes to fallback mechanism
  • Changes to timeout handling

Related Components

Testing

New/Updated Tests

  1. retryManager.test.js: Verifies AI_NoOutputGeneratedError is retryable
  2. last-step-text.test.js:
    • Retries when result.steps rejects with AI_NoOutputGeneratedError
    • Retries when provider resolves empty steps and empty text
    • Stops retrying after budget exhaustion
  3. answer-null-guard.test.js: Verifies non-schema empty streams retry and fail

Validation

cd npm
npm test -- --runInBand tests/unit/retryManager.test.js tests/unit/last-step-text.test.js tests/unit/answer-null-guard.test.js

Result: 3 test suites passed, 35 tests passed.

References

  • npm/src/agent/ProbeAgent.js:1453-1484 - _createRetryManager() and _executeWithVercelProvider() with consumeResult callback
  • npm/src/agent/ProbeAgent.js:1684-1761 - streamTextWithRetryAndFallback() with consumeResult callback
  • npm/src/agent/ProbeAgent.js:4477-4485 - Empty-output detection
  • npm/src/agent/RetryManager.js:28-30 - Retryable error patterns
  • npm/src/agent/RetryManager.js:52-55 - Error name matching
  • npm/tests/unit/last-step-text.test.js:97-182 - New retry tests
  • npm/tests/unit/retryManager.test.js:91-99 - Retryable error test
Metadata
  • Review Effort: 2 / 5
  • Primary Label: bug

Powered by Visor from Probelabs

Last updated: 2026-06-11T19:20:23.478Z | Triggered by: pr_updated | Commit: a3596c3

💡 TIP: You can chat with Visor using /visor ask <your question>

@probelabs

probelabs Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

✅ Security Check Passed

No security issues found – changes LGTM.

✅ Security Check Passed

No security issues found – changes LGTM.

\n\n

Architecture Issues (7)

Severity Location Issue
🟢 Info npm/src/agent/ProbeAgent.js:4477
Empty output detection for non-schema responses is a special case hardcoded in the consumption logic. This creates a fragile pattern where schema vs non-schema handling is scattered across multiple locations.
💡 SuggestionConsider centralizing empty output detection in a dedicated validation method that can be reused across different code paths, making the logic more explicit and testable.
🟡 Warning npm/src/agent/ProbeAgent.js:1463
The _createRetryManager method is introduced but then duplicated inline in _executeWithVercelProvider. The method at line 1463 creates a RetryManager with the same configuration pattern as lines 1466-1472, violating DRY principles. The factory method should be used consistently.
💡 SuggestionReplace the inline RetryManager creation at lines 1466-1472 with a call to this._createRetryManager(). The same applies to the fallback path RetryManager creation.
🔧 Suggested Fix
this.retryManager = this._createRetryManager();
🟡 Warning npm/src/agent/ProbeAgent.js:1513
The fallback path RetryManager creation duplicates the configuration logic instead of reusing _createRetryManager. This creates maintenance burden and risks configuration drift.
💡 SuggestionReplace inline RetryManager creation with: const providerRetryManager = this._createRetryManager({ maxRetries: config.maxRetries ?? this.retryConfig.maxRetries });
🔧 Suggested Fix
const providerRetryManager = this._createRetryManager({ maxRetries: config.maxRetries ?? this.retryConfig.maxRetries });
🟡 Warning npm/src/agent/ProbeAgent.js:1463
The _createRetryManager factory method is incomplete - it doesn't handle the fallback path's need to override maxRetries per provider. The abstraction leaks configuration details into _executeWithVercelProvider.
💡 SuggestionExtend _createRetryManager to handle the fallback override case: _createRetryManager({ maxRetries: config.maxRetries ?? this.retryConfig.maxRetries }) or make the method handle both cases cleanly.
🟡 Warning npm/src/agent/ProbeAgent.js:4444
The consumeResult callback wraps 80+ lines of complex logic including timeout setup, error handling, resource cleanup, and event listener management. This creates a deeply nested callback structure that's hard to reason about and debug.
💡 SuggestionExtract the callback body into a separate method like _consumeStreamResult(result, timeoutState, gracefulTimeoutState, negotiatedTimeoutState, onToolCall) to improve readability and testability.
🟡 Warning npm/src/agent/ProbeAgent.js:4444
The toolCall event listener is registered inside the retry callback but removed in the finally block. If retry occurs, the listener could be registered multiple times before cleanup, creating potential memory leaks or duplicate event handling.
💡 SuggestionMove event listener registration outside the retry boundary or ensure proper cleanup between retry attempts. Consider using a singleton pattern for the listener registration.
🟡 Warning npm/src/agent/ProbeAgent.js:4444
Timeout timers (gracefulTimeoutId, hardAbortTimeoutId, negotiatedTimeoutState.softTimeoutId) are created inside the retry callback. If retry occurs, previous timers may not be properly cleaned up before new ones are created, causing resource leaks.
💡 SuggestionEnsure timer cleanup happens before retry attempts or move timer setup outside the retry boundary. The finally block should clear all timers even when retrying.

Performance Issues (6)

Severity Location Issue
🟢 Info npm/src/agent/ProbeAgent.js:4444
The entire executeAIRequest function is now wrapped in a retry loop via the consumeResult callback. This means all post-stream processing (timeout setup, result.steps await, result.text await, usage recording, cleanup) is executed on every retry attempt. With default maxRetries=3, this code path can execute up to 4 times (initial + 3 retries) for empty-stream failures.
💡 SuggestionConsider moving the timeout setup and cleanup outside the retry loop to avoid redundant operations. Only the streamText call and empty-output detection should be retried.
🟢 Info npm/src/agent/ProbeAgent.js:4477
Empty-output detection check (!options.schema && (!steps || steps.length === 0) && (!finalText || !finalText.trim())) is performed AFTER awaiting result.steps and result.text. This means the async operations complete even when the result will be discarded and retried. With multiple retries, this wastes time awaiting promises that will be thrown away.
💡 SuggestionConsider early detection of empty streams before awaiting the full result chain, or add a timeout to the await operations to fail fast on empty responses.
🟢 Info npm/src/agent/RetryManager.js:113
Jitter normalization uses typeof options.jitter === 'boolean' which is less efficient than direct boolean coercion. The jitter option is checked on every retry attempt, adding unnecessary type checking overhead.
💡 SuggestionUse options.jitter === false for more efficient boolean coercion, or normalize once in constructor instead of checking on every retry.
🟢 Info npm/src/agent/ProbeAgent.js:1513
New RetryManager instance created for each fallback provider in executeWithFallback. With multiple fallback providers and retries, this can create many RetryManager instances that accumulate statistics objects and configuration copies.
💡 SuggestionConsider reusing a single RetryManager instance with updated configuration, or implement a pooling mechanism for RetryManager instances.
🟡 Warning npm/src/agent/ProbeAgent.js:1463
New _createRetryManager method creates additional RetryManager instances without cleanup mechanism. The method is called in _executeWithVercelProvider (line 1467) and for fallback providers (line 1513), creating multiple instances that persist for the lifetime of the ProbeAgent. The existing cleanup() method does not clear retryManager or fallbackManager instances, causing potential memory leaks in long-running processes.
💡 SuggestionAdd cleanup of retryManager and fallbackManager instances in the cleanup() method, or implement a dispose pattern. Consider using weak references or clearing instances when no longer needed.
🟡 Warning npm/src/agent/ProbeAgent.js:1478
New consumeResult callback parameter adds an extra async function wrapper around the entire post-stream consumption path. This wraps result.steps, result.text, and usage resolution in an additional async layer, increasing call stack depth and adding overhead to every AI request. The callback is invoked even when not needed (consumeResult ? await consumeResult(result) : result).
💡 SuggestionConsider inlining the post-stream logic or using a more efficient pattern that avoids the extra async wrapper when consumeResult is not provided.

Quality Issues (10)

Severity Location Issue
🟠 Error npm/src/agent/ProbeAgent.js:4477-4482
Empty output detection logic is placed AFTER streamText completes, but the retry wrapper is around executeAIRequest which calls streamTextWithRetryAndFallback. This creates a disconnect: empty responses detected post-stream cannot trigger retries because executeAIRequest has already completed successfully. The empty output check at line 4477-4482 throws an error, but this error is outside the retry scope.
💡 SuggestionMove the empty output detection logic inside executeAIRequest so it can trigger retries, or wrap the entire streamTextWithRetryAndFallback call in the retry manager instead of just executeAIRequest.
🟡 Warning npm/src/agent/ProbeAgent.js:4477-4482
The empty output check throws an error with name 'AI_NoOutputGeneratedError', but this error name is not consistently used. The error message 'No output generated' is added to DEFAULT_RETRYABLE_ERRORS, but the error.name check may not work reliably across all Vercel AI SDK versions.
💡 SuggestionEnsure the error name is set consistently, or rely only on the error message pattern matching.
🟡 Warning npm/src/agent/ProbeAgent.js:4477-4482
The empty output check only applies when !options.schema, but the condition checks for empty steps AND empty text. However, schema-based requests can also legitimately return empty text when using structured output. The current logic correctly exempts schema requests, but this creates an inconsistency where schema requests with empty output are treated differently than non-schema requests.
💡 SuggestionDocument this behavior clearly in comments and ensure downstream consumers understand the difference between schema and non-schema empty output handling.
🟡 Warning npm/src/agent/ProbeAgent.js:4510-4532
The executeAIRequestWithPostStreamRetry function creates a new RetryManager instance on every retry. This means if post-stream consumption fails, it will retry with a fresh retry budget instead of consuming from the original retry budget. This could lead to more total retries than configured.
💡 SuggestionPass the existing retry manager instance or track cumulative retry attempts across both retry layers to prevent exceeding the configured retry budget.
🟡 Warning npm/src/agent/ProbeAgent.js:4510-4532
Creating a new RetryManager for post-stream retries duplicates configuration logic and adds overhead. The jitter, maxDelay, backoffFactor, and retryableErrors are copied from this.retryConfig, but this could be simplified by reusing the existing retry infrastructure.
💡 SuggestionConsider reusing the existing RetryManager instance or creating a shared configuration object to avoid duplication.
🟡 Warning npm/src/agent/ProbeAgent.js:4510-4532
The post-stream retry wrapper does not propagate the original error context. When a retry occurs, the original error that triggered the retry is lost, making debugging more difficult.
💡 SuggestionPreserve the original error chain and context when throwing the AI_NoOutputGeneratedError to aid debugging.
🟡 Warning npm/tests/unit/last-step-text.test.js:94-118
Test 'retries when result.steps rejects with NoOutputGeneratedError' uses magic number 2 for maxRetries without explanation. The value 2 is arbitrary and not derived from any requirement.
💡 SuggestionUse a named constant like DEFAULT_TEST_MAX_RETRIES or add a comment explaining why 2 retries are appropriate for this test scenario.
🟡 Warning npm/tests/unit/last-step-text.test.js:119-143
Test 'retries when provider resolves empty steps and empty text' uses magic number 2 for maxRetries without explanation. The value 2 is arbitrary and not derived from any requirement.
💡 SuggestionUse a named constant like DEFAULT_TEST_MAX_RETRIES or add a comment explaining why 2 retries are appropriate for this test scenario.
🟡 Warning npm/tests/unit/last-step-text.test.js:145-169
Test 'stops retrying NoOutputGeneratedError after retry budget is exhausted' uses magic number 1 for maxRetries without explanation. The value 1 is arbitrary and not derived from any requirement.
💡 SuggestionUse a named constant like MIN_TEST_MAX_RETRIES or add a comment explaining why 1 retry is appropriate for this test scenario.
🟡 Warning npm/tests/unit/answer-null-guard.test.js:32-56
Test 'retries and fails when non-schema stream has no steps and no text' uses magic number 1 for maxRetries without explanation. The value 1 is arbitrary and not derived from any requirement.
💡 SuggestionUse a named constant like MIN_TEST_MAX_RETRIES or add a comment explaining why 1 retry is appropriate for this test scenario.

Powered by Visor from Probelabs

Last updated: 2026-06-11T19:20:02.323Z | Triggered by: pr_updated | Commit: a3596c3

💡 TIP: You can chat with Visor using /visor ask <your question>

@buger buger marked this pull request as ready for review June 12, 2026 06:49
@buger buger merged commit 5215e29 into main Jun 12, 2026
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Empty stream (NoOutputGeneratedError) should be retried by RetryManager

1 participant