Skip to content

Harden upstream error handling: biz errors, count_tokens, stream truncation, setup retries - #94

Open
Rrainich wants to merge 3 commits into
AmanCode22:mainfrom
Rrainich:fix/upstream-hardening
Open

Rrainich wants to merge 3 commits into
AmanCode22:mainfrom
Rrainich:fix/upstream-hardening

Conversation

@Rrainich

Copy link
Copy Markdown

What this fixes

DeepSeek often answers HTTP 200 with a business-level error (code=40003 INVALID_TOKEN, "messages too frequent", a killed web session) rather than a 4xx. The bridge indexed straight into data.biz_data on those responses, which raised TypeError: 'NoneType' object is not subscriptable inside the route. No status was attached, so the token-rotation path never fired — the dead token stayed pinned in the pool and every later turn failed the same way in ~330 ms until it was removed by hand.

Changes

functions.py

  • UpstreamBizError(UpstreamError) + _biz_data() — one helper for data.biz_data. Auth-shaped business messages (login|auth|token|session|expire) map to 401 so the token is quarantined/rotated; everything else maps to 429 so it cools down and rotates. Because the status rides on UpstreamError, _upstream_http_code() and the existing 401/403/429 path in handle_chat pick it up unchanged.
  • Replaced every raw ["biz_data"] index: create_challange_pow, create_new_chat, upload_file (×2), get_file_content (×2).

app.py

  • _setup_failure_outcome / _setup_retry_kwargs — create_new_chat / create_challange_pow run before the try block that owns rotation, so an auth failure there escaped handle_chat as a bare 500 and the dead token was never marked. These now classify the failure, mark the token, and re-enter handle_chat with the failed token excluded — outside the per-signature asyncio.Lock, which is not reentrant (recursing inside it hung the request).
  • POST /v1/messages/count_tokens (plus the /messages/count_tokens alias) — the route did not exist, so every Claude Code context probe was a 404.
  • _with_deadline — the shared ClientSession uses ClientTimeout(total=None, connect=15, sock_read=600), and sock_read resets on every keepalive byte, so a slowly-dripping upstream never tripped anything (the access log has 19.7-minute /v1/messages entries). This puts a configurable ceiling (DEEPSEEKER_REQUEST_DEADLINE, default 600 s) on upstream work and raises UpstreamError(504). Scope note: it is applied inside the SSE generators because they outlive handle_chat's return. It covers "upstream goes silent" and cancels the upstream generator; a slow client still surfaces as CancelledError rather than a 504, so it is not an end-to-end hard ceiling.
  • _is_truncated_stream_error — upstream hang-ups mid-stream (aiohttp.ClientPayloadError / TransferEncodingError) logged a full traceback and left the client on a dead socket. Downgraded to a warning; the terminal error frame still goes out.

Tests

  • tests/test_upstream_hardening.py — 15 cases: count_tokens auth + flattening, _biz_data status mapping, deadline pass-through / 504 on hang / no deadlock on early abort, truncation detection.
  • tests/test_setup_retry.py — 2 cases: a 401 on create_new_chat rotates and succeeds instead of 500-ing, and the retry does not deadlock on the signature lock.
274 passed in 16.08s

Behaviour change to be aware of

A flagged/invalid token now produces an honest 401/429 and rotation instead of a wall of 500s. That is recovery, not repair: if every token belongs to the same flagged account, rotation has nowhere to go and the client sees a 429. That is the correct signal — the previous 500 loop was hiding it.

Not covered

  • The rotation path itself has no live test against a genuinely flagged upstream account; it is covered at the unit level only.
  • hardening-backup.patch in my working tree is a local rollback artifact and is not part of this PR.

Note: this supersedes #92, which was closed for an unrelated reason (a commit in that branch carried an unintended author identity; the branch was rewritten and re-pushed). The (PR #92) citations inside the CHANGELOG entries are from that history and refer to this same change set.

…cation, setup retries

DeepSeek answers several endpoints with HTTP 200 carrying a business-level
error (code=40003 INVALID_TOKEN, "messages too frequent", killed web
session) instead of a 4xx. The old code indexed straight into
data.biz_data, so those responses died with
`TypeError: 'NoneType' object is not subscriptable` inside a route, and
because no status was attached the token-rotation path never fired: a dead
token stayed pinned and every later turn failed the same way in ~330 ms.

functions.py
- Add UpstreamBizError(UpstreamError) and _biz_data(); auth-shaped business
  messages map to 401 (quarantine/rotate), everything else to 429 (cool
  down and rotate). Replace all raw biz_data indexing in
  create_challange_pow, create_new_chat, upload_file and get_file_content.
  Existing rotation in handle_chat picks it up with no changes because the
  status rides on UpstreamError.

app.py
- _setup_failure_outcome / _setup_retry_kwargs: create_new_chat and
  create_challange_pow run before the try block that owns rotation, so an
  auth failure there escaped handle_chat as a bare 500 and the dead token
  was never marked. Classify those, mark the token, and re-enter
  handle_chat with the token excluded — OUTSIDE the per-signature
  asyncio.Lock, which is not reentrant and previously hung the request.
- POST /v1/messages/count_tokens (plus /messages/count_tokens): the route
  did not exist, so every Claude Code context probe returned 404.
- _with_deadline: a hard ceiling on upstream work. ClientTimeout has
  total=None and sock_read resets on keepalive bytes, so a slowly-dripping
  upstream never tripped anything (the access log has 19.7-minute
  /v1/messages entries).
- _is_truncated_stream_error: upstream hang-ups mid-stream logged a full
  traceback and left the client on a dead socket. Downgrade to a warning
  and keep the terminal error frame.

tests
- test_upstream_hardening.py: 15 cases for count_tokens, _biz_data
  mapping, the deadline wrapper and truncation detection.
- test_setup_retry.py: 2 cases for the pre-turn auth failure path,
  including that the retry does not deadlock on the signature lock.

274 tests pass.
CONTRIBUTING requires a CHANGELOG bullet and README update in the same PR
as new user-visible behavior. count_tokens is now served, and upstream
business errors surface as 401/429 instead of a 500 traceback.
Upstream began rejecting parent_message_id=0 with biz_code 26 "invalid
message id" on 2026-10-31. The rejection arrives as an application/json
envelope on the SSE endpoint, so the parser saw zero `data:` lines and
raised the generic "empty stream" 502 — no cause, no traceback.

- Omit parent_message_id entirely on the first turn instead of sending 0
  or null. Edges disagree about null; omission is the only shape that
  works everywhere. Follow-up turns still chain the stored parent id.
- Keep the non-SSE body tail and the response content-type, and log both
  when a stream ends with no output. A JSON error envelope in that tail
  is promoted into the raised UpstreamError message.
@Rrainich

Copy link
Copy Markdown
Author

Credit where it's due: @1515asd diagnosed the parent_message_id: 0 rejection (biz_code 26, delivered as application/json on the SSE endpoint) independently, against a live incident, and published it with a fix in 1515asd/rh-deeperseeker (ac9a90f).

This PR carries the same two changes, implemented separately:

  • First turn omits parent_message_id. Previously the code coerced 0 → null and always sent the key. It now omits the key when there's no parent, and only sends it on follow-up turns — matching the finding that omission is the only shape accepted across edges.
  • Non-SSE tail diagnostics. The SSE parser keeps non-data: body lines and the response content-type, logs both when a stream ends without output, and promotes a JSON error envelope in that tail into the raised UpstreamError. Combined with this branch's UpstreamBizError mapping, that failure now carries a status instead of pinning the token.

Tests: tests/test_parent_message_id.py, 4 cases. Full suite 278 passed.

(This was originally opened as #92; that PR is closed and this one supersedes it with an identical diff.)

@AmanCode22

Copy link
Copy Markdown
Owner

Kindly fix merge conflicts and ruff check

@AmanCode22

Copy link
Copy Markdown
Owner

@Rrainich kindly fix ruff check and conflics of merge

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