Skip to content

fix(relay): stop reporting environment credential rejections as unreachable - #8848

Open
Noahtarp wants to merge 2 commits into
pingdotgg:mainfrom
Noahtarp:fix/ios-relay-endpoint-failure
Open

Noahtarp wants to merge 2 commits into
pingdotgg:mainfrom
Noahtarp:fix/ios-relay-endpoint-failure

Conversation

@Noahtarp

@Noahtarp Noahtarp commented Aug 31, 2026 •

Copy link
Copy Markdown

Summary

  • EnvironmentConnector.connect() — the relay code path T3 Connect uses whenever a client opens a thread — collapsed every mint-credential failure into EnvironmentMintRequestFailed / endpoint_request_failed, including well-typed errors the environment itself returned (401/409/500) and response schema mismatches. That told users "the endpoint is unreachable" even when the environment had answered, with no way to tell the two apart, and nothing was logged about the real cause.
  • Classify the failure the same way getEnvironmentStatus() already does: a decoded application error or a response schema failure now maps to the existing EnvironmentMintResponseInvalid (endpoint_response_invalid); genuine transport failures keep EnvironmentMintRequestFailed (endpoint_request_failed). Also logs the classified reason against the request's trace ID.
  • No wire-contract changes — endpoint_response_invalid already existed and every client version already understands it.

Fixes #8844

Test plan

  • vp test run infra/relay/src/environments/EnvironmentConnector.test.ts (17/17, includes two new regression tests)
  • vp test run infra/relay/src/http/Api.test.ts (11/11)
  • vp run t3code-relay#typecheck
  • vp lint on changed files

Note

Medium Risk
Changes which error clients see on the connect/mint path (better diagnostics, same wire codes), affecting a core T3 Connect flow without new API surface.

Overview
T3 Connect mint failures no longer all surface as “endpoint unreachable.” When EnvironmentConnector.connect() calls the managed environment’s mint endpoint, typed HTTP error responses (e.g. 500 with EnvironmentHttpInternalServerError) and schema decode failures now return EnvironmentMintResponseInvalid (endpoint_response_invalid) instead of EnvironmentMintRequestFailed (endpoint_request_failed). Transport-level failures (e.g. ECONNREFUSED) and timeouts are unchanged.

The mint path now mirrors the existing health/status classification: shared helpers (isEnvironmentCloudResponseError, environmentRequestFailureReason) and warning logs with trace ID and relay.environment_mint.failure_reason. EnvironmentMintResponseInvalid gains an optional cause when the invalid response wraps a decoded environment error or schema error (proof-verification mismatches still omit it).

Two regression tests lock in invalid-response vs unreachable behavior.

Reviewed by Cursor Bugbot for commit 644424b819c0bb8535f89a8f96df948bd21bddaa. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Fix EnvironmentConnector.connect to distinguish invalid environment responses from unreachable endpoints

  • EnvironmentConnector.connect now returns EnvironmentMintResponseInvalid (with preserved cause) when a reachable endpoint returns a typed EnvironmentHttp*Error or schema-invalid response, instead of misclassifying it as EnvironmentMintRequestFailed
  • Genuine transport failures still return EnvironmentMintRequestFailed; timeouts now return EnvironmentMintRequestTimedOut directly without wrapping in an Option-flatMap failure
  • EnvironmentMintResponseInvalid gains an optional cause field (Schema.optional(Schema.Defect())) to carry the underlying decoded environment error or schema decode error
  • New isEnvironmentResponseInvalid helper classifies responses; environmentRequestFailureReason replaces the health-specific version and is reused for both health and mint paths
  • Behavioral Change: callers that previously saw EnvironmentMintRequestFailed for 500-level or schema-invalid responses will now see EnvironmentMintResponseInvalid; the old isEnvironmentHealthError util is renamed to isEnvironmentCloudResponseError

Macroscope summarized e43b90c.

@coderabbitai

coderabbitai Bot commented Aug 31, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3d139629-6678-4812-b37d-3ec54111f1e2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can activate Security or dismiss this notice.


Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Aug 31, 2026

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two convention issues in the new mint-failure classification path in infra/relay/src/environments/EnvironmentConnector.ts: the new EnvironmentMintResponseInvalid translation drops the underlying failure instead of preserving it as cause, and the caller-visible health-status error message text was changed as part of the refactor.

Posted via Macroscope — Effect Service Conventions

Comment thread infra/relay/src/environments/EnvironmentConnector.ts
Comment thread infra/relay/src/environments/EnvironmentConnector.ts Outdated
@macroscopeapp

macroscopeapp Bot commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 644424b

Macroscope's review found this PR approvable — This is a contained relay error-classification fix that reuses existing wire-level error reasons and preserves transport and timeout handling. Targeted regression tests cover both environment rejections and genuinely unreachable endpoints, with no product-default, schema, deployment, or security-sensitive changes.

No code changes detected at e43b90c. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

Noahtarp and others added 2 commits September 6, 2026 11:45
…chable

T3 Connect's connectEnvironment (used whenever a client opens a thread
through the relay, most visibly on iOS) collapsed every mint-credential
failure into EnvironmentMintRequestFailed / "endpoint_request_failed" -
including well-typed errors the environment actually returned (401/409/500)
and response schema mismatches. That told users "the endpoint is
unreachable" even when it had answered, and hid the real cause from the
trace-correlated logs, making the failure impossible to self-diagnose.

Classify the cause the way getEnvironmentStatus already does: a decoded
EnvironmentHttpCloudErrors response or a response schema failure now maps
to the existing EnvironmentMintResponseInvalid ("endpoint_response_invalid"),
while genuine transport failures keep EnvironmentMintRequestFailed
("endpoint_request_failed"). Also logs the classified failure reason
against the request's trace ID, matching the health-check code path.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- EnvironmentMintResponseInvalid now carries an optional cause so the
  decoded EnvironmentHttp*Error or SchemaError it wraps isn't discarded.
- Restore the status()-only client-visible message wording ("Managed
  endpoint health request failed") that the earlier refactor accidentally
  changed; only the shared, log-only reason classifier was meant to be
  generalized.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@shivamhwp
shivamhwp force-pushed the fix/ios-relay-endpoint-failure branch from 644424b to e43b90c Compare September 6, 2026 06:15
@cursor

cursor Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@shivamhwp

Copy link
Copy Markdown
Collaborator

Note: GPT-6 on behalf of shivam (@shivamhwp).

Please finish the response-error handling in isEnvironmentResponseInvalid. An HTTP 500 with an unrecognized JSON body and a proxy HTML 502 arrive as response-bearing HttpClientErrors, but currently fall through to EnvironmentMintRequestFailed. Handle those separately from failures where no HTTP response arrived, preserve the cause, and add coverage for both forms. A proxy 502 can indicate a broken tunnel/origin, so retaining its HTTP status matters for diagnosing the connection failure.

The new warning records only failureReason, such as StatusCodeError or TransportError. Please include the upstream HTTP status when available and a safe underlying transport error code, correlated with the trace ID. Otherwise DNS/refused connections and different upstream HTTP failures still cannot be distinguished from these logs.

For the actual connection failure in #8844, please investigate relay trace 3b4c5cf5a3bf5af6788ace70ab71afa8 through the credential request to /api/t3-connect/mint-credential. Determine whether that request reaches the host, what status/cause it returns, and whether the tunnel ingress points to the Windows server's active listener on port 3773. Device discovery and published activity do not establish that this inbound request works.

There is a concrete related origin-port bug covered by #8353: link proofs requested through a TCP forward can record the client's forwarded port instead of the server listener. That is not yet established as #8844's cause, and #8353 should not be folded into this PR without evidence that it applies. We need the failing request's trace to identify and address the connection defect, beyond changing its error category.

@shivamhwp

Copy link
Copy Markdown
Collaborator

Note: GPT-6 on behalf of shivam (@shivamhwp).

The response-classification issue from the earlier comment is still present at e43b90c: an HTTP 500 with an unrecognized JSON body produces StatusCodeError, and an HTML 502 produces DecodeError. Both still become EnvironmentMintRequestFailed, even though an HTTP response arrived. Please classify response-bearing HTTP errors separately from transport failures and retain the upstream status. This remains a blocker for the stated fix.

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Sep 30, 2026 — with ChatGPT Codex Connector

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: T3 Connect fails to open environment: endpoint_request_failed on Windows → iOS

3 participants