Repository navigation
fix(server): log why a remote chat call failed, and warn when an upstream key is unset - #738
Merged
Merged
Conversation
…ream key is unset The single-node remote chat route answered a failed upstream call with 502 or 503 and logged only the access line, although the server holds a fixed reason for each failure. It now logs one WARNING per failed call with the model, the upstream and that reason, including a stream that fails after its first event and chat rendered through the model's template. An upstream whose credential variable is unset, blank or unusable now logs one WARNING at startup naming the upstream and the variable. The server still starts and reads the variable for every request, as before.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 9 files. (1 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly summarizes the main changes: logging remote chat failures and warning when an upstream credential is unset.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 9 files. (1 skipped: 1 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
added 4 commits
October 14, 2026 21:07
… the existing ones
The single-node remote chat route answered every refused or broken upstream call with the same fixed 502 (or, after output, the same in-stream error) and logged nothing but the access line, so a missing upstream key, a 401, a 402, a 404 and an upstream error event all looked the same to an operator. The route now logs the failure's reason at warning level, for example "the upstream credential is unavailable" or "upstream answered 401". The reason is the server's own fixed text and repeats nothing the upstream sent. The caller's answer is unchanged. An OpenAI-compatible answer that carries an error object in place of the chat answer, as an upstream sends when the provider behind it fails after the 200, is now reported as "upstream sent an error instead of a chat answer" rather than as an invalid chat response, so the log tells it apart from an answer the parser refused.
A remote chat, native generation or completions request whose upstream could not be reached answered 503 and logged nothing. Single-node chat now logs the reason with the line it already uses for a 502. Native generation and completions, and chat the server renders through the model's template, log it from the remote adapters before the reason is mapped to a retryable capacity error. A native streamed request that fails before output now logs the failure as its buffered and after-output paths already do.
This was referenced Oct 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On a single node,
POST /v1/chat/completionsfor a remote profile answers a failed upstream call with502 upstream_error: upstream generation request failed(or a503), and the server log shows only the access line. The server already holds a fixed reason for each failure, for exampleupstream answered 401orthe upstream credential is unavailable, but the route dropped it. An upstream that could not be reached was silent on every generation route, and native/v1/generatestreamed was silent for any failure before output. An upstream whoseapi_key_secretvariable was unset also started without a word and then failed every request.Change
remote chat for <model> failed: <reason>atWARNING, orremote chat for <model> failed after output: <reason>when a stream fails after its first event. An OpenAI-compatible answer that carries an error object in place of the chat answer is reported asupstream sent an error instead of a chat answer. These lines and their tests are commit 2a8d1a2 from feat(examples): run the OpenCode example on self-hosted SIE with an OpenRouter upstream #735, carried here unchanged. feat(examples): run the OpenCode example on self-hosted SIE with an OpenRouter upstream #735 picks them up frommainwhen it next merges it.remote chat for <model> failed: upstream 'chat-api' is unavailable: request failed (ConnectError). Native generation, completions, and chat the server renders through the model's template reach the upstream through the remote adapters'generate, which turns the failure into a retryable capacity error and drops its reason. They now logremote generation failed: <reason>at that point, once per call./v1/generatestreamed, before output. A failure there answered500with no log line. It now logsstream_generate failed before outputwith the traceback, as the buffered (generate failed for <model>) and after-output (stream_generate failed mid-stream) paths of the same route already do.WARNINGnaming the upstream and the variable. The check is the one each request already runs (Upstream.api_key()). The server still starts and still reads the variable for every request, asconfig/upstreams.pyandREMOTE_BACKENDS.mddescribe.REMOTE_BACKENDS.mdstates both, next to the credential example and the failure behaviour.What an operator now sees (model
org/model, upstreamchat-api, variableCHAT_API_KEY):No line contains the credential, the prompt, or anything the upstream sent. Every reason is text the server builds itself. A refused input (
400) is already logged byopen_streamasupstream '<name>' refused a generation request: ....Every remote generation path, after this change
Checked on the same scripted upstream (401, 404, connection failure, failure after output), for both upstream kinds:
remote chat for <model> failed: <reason>... failed after output: <reason>remote chat for <model> failed: <reason>remote generation failed: <reason>... failed after output: <reason>/v1/generate, bufferedgenerate failed for <model>+ traceback (unchanged)remote generation failed: <reason>/v1/generate, streamedstream_generate failed before output+ tracebackremote generation failed: <reason>stream_generate failed mid-stream+ traceback (unchanged)/v1/completionsOpenAI completion failed ...+ traceback (unchanged)remote generation failed: <reason>Generate iterator raised for <request>/<attempt>+ traceback (unchanged)remote generation failed: <reason>All are
WARNING. The queued worker already logged every failure, so it is unchanged except for the adapter line, which adds the transport reason that its capacity error does not carry.Classification
Ordinary review path. This is not an authentication or authorization change: how the upstream credential is read, sent and enforced is unchanged. The startup check calls the same
Upstream.api_key()that each request already calls, keeps nothing, and logs only the variable's name.Tests
test_a_remote_generation_failure_logs_one_warning_with_its_reason: the real routes with aMockTransportupstream. Chat 404 and connection failure, rendered-template chat 401 and connection failure, native and completions connection failure, each streamed and buffered, for both upstream kinds (24 cases). Each asserts exactly oneWARNINGwith its logger and exact text, that the configured credential was sent as the bearer token, and that a canary credential, the prompt and the upstream's error text never reach the captured log atDEBUG.test_a_native_remote_failure_before_output_logs_one_warning_with_its_reason: native 401, buffered and streamed, one warning whose exception isupstream answered 401.test_startup_warns_about_an_unusable_credential_and_still_starts:AppFactory.create_appwith an upstreams file, for an unset, blank, unusable and set variable. It asserts one warning or none, that both upstreams are installed, and that the canary value never reaches the log.Mutations, each caught: no chat log on a 502, on an unreachable upstream, or after output; the chat 502 line at
INFOor logging the request body; the route also logging the rendered-template capacity error (two lines); no adapter line in either adapter; the adapter line logging the transport exception; no native before-output line, or the line without its traceback; no startup warning; the startup warning logging the value; startup refusing to start; the startup check treating only an absent variable as unset.Validation:
mise run lintandmise run typecheckpass.mise run test -- packages/sie_server/testsgives 11764 passed and 5 failed on macOS; the 5 (tests/config/test_serving_artifacts.py,tests/core/test_model_loader_cache.py) fail identically withmain's source, on read-only directory renames, and pass in CI.