Skip to content

fix(zai): treat HTTP 200 stream errors as failures - #12454

Merged
diegosouzapw merged 5 commits into
release/v3.8.51from
fix/v3851-zai-web-stream-error-boundary
Sep 3, 2026
Merged

diegosouzapw merged 5 commits into
release/v3.8.51from
fix/v3851-zai-web-stream-error-boundary

Conversation

@diegosouzapw

@diegosouzapw diegosouzapw commented Sep 2, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • stop converting Z.ai HTTP 200 error frames into assistant text followed by a normal stop
  • return a sanitized pre-content error-only frame so readiness produces a 502 and combo fallback remains available
  • preserve already-streamed content, then fail the shared pipeline so Chat, Claude, and Responses clients receive their native terminal error and failure usage is persisted
  • make delta delivery backpressure-aware and keep client cancellation non-blocking even when the upstream body ignores cancellation
  • isolate the four stateful stream-boundary checks in a child fixture so DATA_DIR, plugins, DB, and logger teardown cannot contaminate the shared --test-isolation=none unit runner

Evidence

Check Result
Exact starting release 7802f6ea163f18f349348dbfd8710a159ac32e15
Baseline focused suite PASS — 37/37
TDD RED PASS — reproduced cross-protocol response.failed leakage, false-success persistence in passthrough/translate, queued partial loss, and stalled cancellation
Final focused suite before isolation repair PASS — 41/41
Process-isolated child fixture PASS — 4/4 real stream-boundary checks
Shared-runner reproduction pair PASS — 3/3 with absolute paths and the exact test:unit:fast setup imports/flags, --test-isolation=none, and no --test-concurrency; the prior call-log artifact failure is gone
Z.ai regression family PASS — 84/84
Strict Chat passthrough PASS — partial content + chat.completion.chunk with finish_reason:error; no response.failed or normal stop
OpenAI to Claude translation PASS — partial content + event:error/message_stop; persisted 502 stream_pipeline_error
Pre-content readiness PASS — sanitized 502; fake key, stack, and path do not reach the public body
Stalled upstream cancellation PASS — consumer cancel resolves bounded and requests upstream cancel exactly once
Production snapshot after test repair UNCHANGED — open-sse/executors/zai-web/stream.ts SHA-256 e2560b9390a9f92304c8ed80d938dc2028a5db0db739a7e34e3c21a7bda92538
TypeScript PASS — npm run check:open-sse-typecheck reports 0 errors on the reconciled candidate
ESLint with official suppressions PASS
Prettier / diff check / changelog integrity PASS
Latest release reconciliation PASS — merged e243b04de22da2d78900c27fde5f83f4fbc3d7f9 without conflicts; 10/10 focused tests and gates rerun on the merged candidate
Final release base e243b04de22da2d78900c27fde5f83f4fbc3d7f9
Candidate SHA 1723d47e2a25422755ace3c74bf9903e5268e9cc

Validation boundary

All regressions use local deterministic streams, synthetic directories, a synthetic API secret, and stubs. The child environment is an explicit allowlist and does not inherit HOME, CODEX_HOME, real credentials, or the parent NODE_TEST_CONTEXT. No live Z.ai credentials or upstream provider calls were used, so live credential/WAF behavior remains an explicit HOLD rather than an implied pass.

@diegosouzapw
diegosouzapw marked this pull request as ready for review September 3, 2026 23:58
@diegosouzapw
diegosouzapw merged commit 855eda1 into release/v3.8.51 Sep 3, 2026
20 of 21 checks passed
linhdmn added a commit to linhdmn/omniroute-INITIAL_PASSWORD-fix that referenced this pull request Sep 4, 2026
…om release/v3.8.51

All 15 failing tests across shards 1-4 reproduce on pristine base c41ec7f;
this drains them at the source so every PR stops inheriting them.

Sanitizer pipeline (regressions introduced by 2265ce7's path-span rewrite):

- errorPathRedaction: credential assignments are now redacted BEFORE path spans
  resolve (errorSanitization runs an early redactLabeledCredentialAssignments pass
  in sanitizeErrorMessageWithStackPolicy), and the span scanner treats sanitizer
  output as prose boundaries — 'label=[REDACTED]', bare '[REDACTED]', and
  'label:' introducers (CREDENTIAL_LABEL_BOUNDARY mirrors BLOCKED_KEYS) stop the
  fail-closed span instead of being swallowed with it. 'with' joins
  CLEAR_PROSE_BOUNDARIES. Fixes: error-sanitizer-sk-key-qv45 (AIza 33-char
  fixture), huggingchat transport + stream boundaries, stream-handler public
  boundary, dashboard request-failed redaction, tunnel canary.

- credentialPatterns: google pattern /AIza[0-9A-Za-z_-]{35}/ → {20,} to match the
  GHSA-qv45-56jc-4wmj test oracle (the qv45 fixture key is 33 chars after AIza;
  the {35} form missed it and redactSensitiveErrorText returned it verbatim).

- error.ts SAFE_PUBLIC_ERROR_IDENTIFIERS: + huggingchat_generation_error,
  zai_stream_error — both were passing through projectPublicErrorIdentifier as
  raw provider codes before cheaper-inference#12506 added the bounded vocabulary; without them
  huggingchat-stream-error-boundary and zai-web-silent-empty-repro see
  'bad_gateway' instead of the pinned codes.

- imageGeneration.saveImageErrorResult: String(error) throws
  'Cannot convert object to primitive value' on the null-prototype objects
  sanitizeUpstreamDetails returns — use JSON.stringify. Fixes 3 codex
  image-generation tests.

- streamFailureBoundary: new keepReadable option; emitTranslatedFailureAndAbort
  (parsed.error mid-stream path) uses it so the forwarded response.failed frame
  is not discarded when the readable errors — restores the kiro
  response.failed contract from cheaper-inference#12454/cheaper-inference#12455 that 2265ce7 broke.

- chat.ts image-model rejection hint: 'Use POST /v1/images/generations instead.'
  → 'Then POST …' — 'Use' is not a prose boundary, so the fail-closed span
  swallowed the endpoint and the cheaper-inference#6457 assertion never matched.

- sync-models route: parseError messages gain a 'GET' route-context marker
  ('Invalid JSON response from GET /models') so the path redactor's route shield
  preserves the endpoint (test updated to the new strings).

- tunnel-routes canary: updated for the sanitizer's intentional growth —
  quoted-path and windows-shape leaks are now covered by the shared sanitizer;
  the canary pins the new coverage map per its own instructions.
JJAbrams-eng added a commit to JJAbrams-eng/OmniRoute that referenced this pull request Sep 22, 2026
Combining cheaper-inference#12454 (adds zai_stream_error) with the error-boundary hardening
commits (cheaper-inference#12506/cheaper-inference#12620) left the new code missing from
SAFE_PUBLIC_ERROR_IDENTIFIERS, so buildErrorBody() silently downgraded it
to the generic bad_gateway fallback.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…#12454)

Validado em lote numa worktree combinada com os 14 PRs desta campanha de error-boundary sobre o tip de `release/v3.8.51`: `typecheck:core` limpo e **120/120** nos 23 arquivos de teste que os PRs trazem.

Um ponto que só apareceu no tree combinado: **cheaper-inference#12465 e cheaper-inference#12466 criam o mesmo arquivo novo** `open-sse/utils/streamReadiness.ts` (que não existe no tip) com desenhos divergentes de cancelamento — `cancelled` + `releaseLock` imediato num, `readInFlight`/`cancelRequested` com `cancelReader` fire-and-forget no outro. Adotei a versão do cheaper-inference#12466, que difere e defere o release do lock para quando a leitura em voo termina, e validei a escolha rodando as suítes dos **dois** PRs contra ela: 21/21 no readiness compartilhado e 22/22 incluindo o boundary do Perplexity.
@diegosouzapw
diegosouzapw deleted the fix/v3851-zai-web-stream-error-boundary branch October 18, 2026 23:51
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