fix(oauth): include the transport root cause in connection error messages#1808
fix(oauth): include the transport root cause in connection error messages#1808wbxl2000 wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: 37082db The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37082dbdc8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let current: Error | undefined = error; | ||
| while (current !== undefined) { | ||
| messages.add(current.message); | ||
| current = current.cause instanceof Error ? current.cause : undefined; |
There was a problem hiding this comment.
Traverse AggregateError entries when describing fetch failures
When connection attempts cover multiple resolved addresses, Node 24.15 can reject fetch with a TypeError whose cause is an AggregateError; the individual ECONNREFUSED details are in AggregateError.errors, while its own message is empty and it has no cause. This loop therefore produces fetch failed: and still hides the transport reasons this change is meant to expose. Flatten AggregateError.errors as well as the linear cause chain.
Useful? React with 👍 / 👎.
commit: |
Related Issue
No issue — the problem is explained in the next section.
Problem
When the OAuth device flow hits a transport-level failure, users only see:
fetch failedis the generic undiciTypeError— the real reason (DNS failure, connection refused, TLS interception, timeout) lives in the nestederror.causechain, which the previous code discarded. This makes reports like "cannot log in with OAuth" undiagnosable without reproducing locally: nobody can tell whether the user's network needs a proxy, the OAuth host is blocked, or certificates are being intercepted. This was reported in the wild by a VS Code extension user.What changed
OAuthError/OAuthConnectionErrornow accept standardErrorOptions, andpostForm(src/oauth.ts) throws the connection error with{ cause: error }attached, so log stacks show the fullCaused by:chain.describeFetchFailure) and includes the root reason, e.g.:OAuth request to ... failed: fetch failed: connect ECONNREFUSED 127.0.0.1:1— deduplicated, non-Error throwables fall back toString(error).OAuthConnectionError/status codes is unaffected..causepreserves the original rejection.Checklist
gen-changesetsskill, or this PR needs no changeset. (Changeset included:@moonshot-ai/kimi-codepatch — the change enters the CLI bundle and improveskimilogin error messages; the oauth workspace itself is private/unpublished.)gen-docsskill, or this PR needs no doc update. (Error-message wording only, no documented behavior change.)