Skip to content

fix(huggingchat): surface HTTP 200 JSONL failures - #12456

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

diegosouzapw merged 4 commits into
release/v3.8.51from
fix/v3851-huggingchat-stream-error-boundary

Conversation

@diegosouzapw

@diegosouzapw diegosouzapw commented Sep 2, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • treat HuggingChat HTTP 200 JSONL status: "error" as an authoritative generation failure
  • return a sanitized HTTP 502 when the error arrives before assistant content
  • preserve partial content, then propagate a fixed public 502 stream error through the normal stream handler so fallback and request persistence record failure instead of finish_reason: "stop"
  • make provider-error cleanup and downstream cancellation fire-and-forget, including a blocked upstream reader whose cancel() never resolves
  • isolate the stateful DB/usage regression suite in a credential-free child process so test:unit:fast cannot share module-cached persistence state with adjacent files

Behavioral boundary

Upstream outcome Client outcome Routing / persistence outcome
JSONL error before content Sanitized JSON HTTP 502 with huggingchat_generation_error Eligible for normal fallback; no fake assistant content
JSONL error after partial content Partial content followed by Chat-compatible finish_reason: "error" and [DONE]; fixed public message Status 502, stream_pipeline_error, failure usage and fallback callback recorded
Normal JSONL finish Existing assistant chunks, finish_reason: "stop", [DONE] Existing success behavior unchanged
Client cancels during blocked read Downstream cancel resolves without awaiting hostile transport cleanup Upstream reader cancel is triggered; no stop/DONE or spurious provider-error log

TDD evidence

The production regression was written against release base 7802f6ea163f18f349348dbfd8710a159ac32e15 before implementation.

  • RED: pre-content JSONL error returned HTTP 200; partial error became finish_reason: "stop" + [DONE]
  • RED: trailing non-stream JSONL error without a newline returned HTTP 200
  • RED: provider-error cleanup blocked forever when upstream cancel() never resolved
  • RED: downstream cancellation did not reach a locked upstream reader
  • RED: a pending generator read still emitted stop/DONE and logged Controller is already closed after cancel
  • GREEN: all eight child-fixture boundary cases pass, including executor → readiness → passthrough → stream handler → finalizer → persistence/fallback

The test-isolation regression was then reproduced and repaired separately:

Phase Topology Result Evidence
RED Original stateful HuggingChat file paired with call-log-save-drain, --test-isolation=none, concurrency 1 9/10 The call-log artifact assertion failed because both files shared module-cached DB/usage state
GREEN Exact final candidate, same pair under the real test:unit:fast imports and --test-force-exit, no concurrency override 3/3 Parent wrapper 1/1 plus call-log drain 2/2; the child independently reports 8/8

Final candidate evidence

Reconciled by normal merge commits through live release SHA bf0d902dfc5369bd025f64808d96fe6cb473ea75. The latest merge commit has that exact live tip as its second parent. Drift from 6da2418247d75acd0af3a617c7289b004b5a86f5 to bf0d902dfc5369bd025f64808d96fe6cb473ea75 had no intersection with the five owned PR paths, so no semantic conflict resolution or production edit was required.

Head SHA: 20d7b82118cfae850eb299ca737f810ae12534cf

Status Gate Evidence
PASS Stateful cross-file isolation 3/3 under test:unit:fast topology, --test-isolation=none, no concurrency override
PASS Child boundary fixture 8/8 with unique DATA/plugin/usage identities and a fresh SQLite database
PASS Post-reconciliation HuggingChat regressions 13/13 across the boundary wrapper, JSONL split, executor, and model catalog; the wrapper independently certifies its internal 8/8 fixture
PASS Stream-handler regressions 35/35 across handler, disconnect, 499 classification, and catch-logging suites
PASS Open-SSE TypeScript openSseTypecheckErrors=0
PASS ESLint All four owned TypeScript files, official suppressions file plus --pass-on-unpruned-suppressions, zero findings
PASS Prettier All five owned files formatted
PASS Changelog integrity + diff check No base bullets lost versus origin/release/v3.8.51; clean diff
PASS Secret and repository hygiene secretFindings=0; no forbidden tracked artifacts
PASS Network and credential isolation Every HuggingChat fetch intercepted; child env is an explicit allowlist with synthetic DATA/plugin/API-secret values and no inherited HOME, CODEX_HOME, credentials, or NODE_TEST_CONTEXT
PASS Resource teardown Call-log saves drain before artifact-writer/logger close and DB reset; parent removes the temp tree only after child exit
PASS Independent adversarial source review Zero P1/HIGH/material P2 on the unchanged production source hashes
HOLD Full repository unit + Vitest matrices Not run locally; proportional provider/stream gates above are green and CI remains authoritative
BASE-RED None observed in required gates No required local gate red needed base attribution

Exact source hashes after reconciliation:

File SHA-256
open-sse/executors/huggingchat.ts 41013d05ad4fa1ed099cbc10db1822dbc2f5ff968adaf27246efc8870c93dd3c
open-sse/executors/huggingchat/jsonlStream.ts e63c60ab3f974978cc86f65bccdf7b3936091a36f4a1fe41283a1a0f4fb1c387
tests/unit/huggingchat-stream-error-boundary.test.ts 3e47baea663db40dda632a80d3e4da745a3eb43cf9051dfc07379dfef8038eaf
tests/unit/_fixtures/huggingchat-stream-error-boundary.fixture.ts ddf5a7878dd410f42d61fb6d30ec3db5c56a1b5363d8f24b15a231a55c66627c

Graph verification

Best-effort project home-diegosouzapw-dev-proxys-OmniRoute, generation 2026-09-02T07:48:51Z: the operated production paths report metadata_match / no_recorded_issue. The wrapper reports graph freshness missing and the child fixture is intentionally excluded under tests/unit/_fixtures, so both were verified directly from source and by execution.

Scope note

Pre-existing fetch-catch response handling elsewhere in huggingchat.ts is unchanged and is being hardened in a separate public-error PR; this change intentionally owns only the JSONL generation boundary.

This PR is intentionally left open as a draft. No merge, automerge, deployment, or release action is included.

Comment on lines +558 to +563
JSON.stringify(
buildErrorBody(502, message, undefined, {
type: "upstream_error",
code: "huggingchat_generation_error",
})
),
Comment on lines +633 to +638
JSON.stringify(
buildErrorBody(502, message, undefined, {
type: "upstream_error",
code: "huggingchat_generation_error",
})
),
@diegosouzapw
diegosouzapw marked this pull request as ready for review September 3, 2026 23:59
@diegosouzapw
diegosouzapw merged commit d63d25f into release/v3.8.51 Sep 3, 2026
21 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
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: **diegosouzapw#12465 e diegosouzapw#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 diegosouzapw#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.
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.

2 participants