Skip to content

fix(gateway): distinguish interrupted and unreachable model connections - #109701

Closed
Lei-k wants to merge 2 commits into
NousResearch:mainfrom
Lei-k:fix/upstream-connection-error-wording
Closed

Lei-k wants to merge 2 commits into
NousResearch:mainfrom
Lei-k:fix/upstream-connection-error-wording

Conversation

@Lei-k

@Lei-k Lei-k commented Sep 13, 2026

Copy link
Copy Markdown

What does this PR do?

Distinguishes three connection failure causes in the gateway's user-facing provider-error reply:

  • Interrupted/reset connection: describe the interrupted transport, without asserting that the model endpoint stopped or that every retry failed the same way.
  • Refused/unroutable endpoint: retain the existing model-server-not-running/unreachable guidance.
  • Cause-free SDK connection error: acknowledge that the connection may not have been established or may have been interrupted, without guessing which.

This changes wording/classification only. Authentication, policy and rate-limit precedence, secret redaction, provider-envelope detection and raw programmatic-surface passthrough remain unchanged. The original _CONNECTION_ERROR_MARKERS tuple is deliberately untouched: its first eight entries feed the provider-envelope gate, so changing it would alter which messages are filtered, not just their wording. No retry policy, model configuration or delivery lifecycle changes.

Related issue and provenance

Refs #26339: a live local endpoint can reset a connection; this PR does not fix that issue's payload-side root cause.

Preserves the refused-endpoint guidance introduced by #86729 for #86570.

Selectively adapted from Lei-k#16, original commit 497cc47556b10eba94c65147cceedeba622aa087, fork merge 5cd65c0c21003ee022bc5119c66f87fac78085e3. Source credit is retained in the commit history.

The fork owner reports a week of successful operation. This is user-reported fork experience, not an upstream E2E validation claim.

Scope and deduplication

Based on upstream main b6b53c69a6ed49cb099cf1bfe76b5e6edd718e5a, not fork main. Only gateway/run.py and its existing topical connection-reply test file change.

The other fork #16 changes are intentionally omitted:

No duplicate recovery/delivery implementation is submitted here. Delegation route ownership from fork #22 is an independent logical change and is not bundled, following the contribution guide.

Verification

scripts/run_tests.sh -j 2 tests/gateway/test_local_model_connection_reply.py tests/gateway/test_compaction_heartbeat_gateway_filter.py tests/gateway/test_telegram_noise_filter.py tests/gateway/test_compression_progress_notices.py
  • Final canonical focused gate: 190 passed, 0 failed, 4 files, exit 0, isolated local environment with two workers.
  • New regressions against unchanged upstream implementation: 4 failed, 6 passed; candidate connection-reply file: 10 passed. Tests assert distinct semantic categories, retained refused-endpoint guidance, secret redaction, other-error precedence and programmatic passthrough.
  • Ruff on both changed files and git diff --check: passed.
  • Full gateway attempts were incomplete (bounded timeout/controller interruption; an initial environment also lacked optional dependencies). No full-suite or hosted-CI pass is claimed. No live transport/provider E2E was run.
  • Independent read-only review of ab7b0f54bbc5466a064f7f885ac3c13590a0f261: PASS, no blocking regression found; focused connection tests and real sanitizer probes reproduced independently. Existing Windows WinError 10053/10054 wordings can still fall through to the generic reply (also true on base); broader Windows marker coverage is intentionally deferred, not claimed fixed. This remains a Draft for user/maintainer review, not authorization to merge.

Checklist

  • Read the contributing guide; one logical change.
  • Behavioral regressions demonstrated failing on upstream base.
  • Retained raw programmatic output and existing secret redaction.
  • Disclosed AI-assisted implementation and independent AI review.
  • Full repository suite passes (not claimed).
  • Live provider/gateway E2E (not performed).

Hermes Agent and others added 2 commits September 13, 2026 06:36
… model endpoint

_gateway_provider_error_reply() answered every connection-shaped provider failure with
"the configured model endpoint is not running or is unreachable". The marker tuple behind
that single row mixes three different failures, and only one of them supports the sentence:

  * interrupted — an ESTABLISHED connection died mid-transfer (ReadError / ECONNRESET /
    RemoteProtocolError). Something accepted and answered, so whether the endpoint is up is
    unknown from this alone; an earlier call in the same turn may have succeeded against it.
  * unreachable — nothing accepted the connection (ECONNREFUSED, WinError 10061, no route).
    Here "not running or unreachable" IS the diagnosis, and it is the case the wording was
    written for (NousResearch#86570, merged in NousResearch#86729). Unchanged.
  * ambiguous — connection-shaped with the cause flattened away by the SDK
    (openai.APIConnectionError: Connection error.). Neither diagnosis is supported.

Split the one connection row into three ordered rows so the first and third stop asserting
that the model server stopped. Neither new reply claims a retry count, which this layer
cannot know.

_CONNECTION_ERROR_MARKERS is deliberately untouched: its first 8 entries are sliced into
_GATEWAY_PROVIDER_ERROR_SHAPE_RE, so editing it would change WHICH texts are treated as
provider envelopes rather than what they are called. Auth/policy/rate-limit precedence,
secret redaction and raw passthrough for programmatic surfaces are unchanged.

Refs NousResearch#26339 (a real errno-104 reset against a live endpoint; its payload-side cause is not
fixed here, only the way it is described).

Co-authored-by: Lei-k <11388531+Lei-k@users.noreply.github.com>
Selectively adapt the connection-wording portion of #16 (497cc47). Preserve endpoint-down guidance for refused connections without asserting that a reset means the endpoint stopped. Leave retry recovery and status/final deduplication to existing upstream contributions.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Sep 13, 2026
@Lei-k
Lei-k marked this pull request as ready for review September 14, 2026 16:37
teknium1 pushed a commit that referenced this pull request Sep 20, 2026
Split the single connection row of the gateway's shaped provider-error reply
into three: an interrupted established connection (reset/EOF/RemoteProtocolError),
a refused/unroutable endpoint (the case #86570 wrote the "not running or is
unreachable" wording for), and a cause-free SDK ``APIConnectionError`` that
supports neither diagnosis. A reset says nothing about whether the endpoint is
up, so telling the user to restart a server that just answered sends them to
debug the wrong thing (#116323).

Selectively adapted from Lei-k#16 (497cc47) via PR #109701,
rebased onto the current reply contract (rate-limit > auth > policy > connection,
every reply names a slash command, no operator jargon).
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks @Lei-k — this landed via #116438 (merge 30de041), which supersedes this PR: your commit was cherry-picked under your authorship (contributor mapping added) and forms the core of the landed fix. Closing with credit; reopen if you see a case it misses.

@teknium1 teknium1 closed this Sep 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants