Skip to content

feat(combo): validate empty 200 responses with retry-before-fallback#2689

Open
berthojoris wants to merge 2 commits into
decolua:masterfrom
berthojoris:feat/combo-empty-200-fallback
Open

feat(combo): validate empty 200 responses with retry-before-fallback#2689
berthojoris wants to merge 2 commits into
decolua:masterfrom
berthojoris:feat/combo-empty-200-fallback

Conversation

@berthojoris

Copy link
Copy Markdown

Summary

Adds empty response body validation to combo fallback. When a provider returns HTTP 200 but the body is empty/null/empty-array/empty-object, the combo now retries the same model once before falling back to the next model. This catches transient empty responses from overloaded providers without prematurely burning combo slots or polluting lastError.

Changes

  • open-sse/services/combo.js — +57/-2 lines
    • New isBodyEmpty() helper detects 12 empty body patterns (empty string, {}, [], {"choices":[]}, {"choices":""}, {"choices":[{}]}, {"choices":[{"delta":{},"finish_reason":null}]}, {"choices":[{"message":{"content":""}}]}, {"choices":[{"message":{}}]}, {"content":""}, {"content":[]}, {"text":""})
    • On 200 + empty body: retry same model once → if still empty → continue to next model
    • Streaming/opaque responses trusted (no body validation)
    • Empty responses do NOT pollute lastError — final "all models failed" message stays accurate

Test coverage

  • tests/unit/combo-empty-200-fallback.test.js (11 tests) — all passing ✅
    • Valid body returns immediately (no retry triggered)
    • Empty object {} → retry → succeed on second attempt
    • Empty choices [] → retry → succeed on second attempt
    • Empty string body → retry → fallback to next model
    • Empty message content {"content":""} → fallback
    • Empty message object {"message":{}} → fallback
    • Empty array [] → fallback
    • Delta-only response → retry → recover on second attempt
    • Both models empty → lastError not polluted (generic "All combo models unavailable")
    • Streaming body → treated as valid, no retry triggered
    • All models return empty → correct 503 with generic error

Edge cases handled

  • Provider returns 200 with {} (empty object) — caught, retried, fallback
  • Provider returns 200 with {"choices":[]} — caught, retried, fallback
  • Provider returns 200 with empty string "" — caught, retried, fallback
  • Streaming SSE responses (text/event-stream) — .text() throws, caught silently, response trusted
  • Empty response retry does not pollute lastError — ensures accurate error reporting

Authoring disclosure

Generated with Hermes Agent (Claude Opus 4); verified locally.

- Add isBodyEmpty() helper to detect 12 empty body patterns (empty
  string, empty object/array, empty choices/content/text)
- On 200 + empty body: retry same model once before falling back
- Streaming/opaque responses are trusted (skip body validation)
- Does not pollute lastError on empty responses — final error
  message stays accurate to the most recent genuine failure
- Tests cover: valid body, empty object, empty array, empty string,
  empty choices, empty message content, empty message object
- Retry behavior: recovery on second attempt, fallback to next model
- lastError not polluted by empty responses
- Streaming responses treated as valid (no retry triggered)
- All 11 tests passing
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.

1 participant