fix: report credential-safe key validation failures - #75
Conversation
| return await getApi(apiKey).get() | ||
| } catch (error) { | ||
| if ( | ||
| error instanceof SeamHttpInvalidTokenError || |
There was a problem hiding this comment.
Why are we replacing our default errors with custom ones
There was a problem hiding this comment.
The intent was to distinguish local token-format failures from actual 401 responses and keep SDK/request details out of display and telemetry. Main already wrapped SDK errors in ApiKeyError, but those goals don't require new exception classes.
Revised in 58e9f3e: native SDK/filesystem errors now propagate unchanged, preserving type, stack, cause and details. The known stage is associated without mutating the error; classification and fixed safe messages live at the display/analytics boundary. No raw error text, causes, URLs, keys or fingerprints are reported.
248 tests pass locally, including preserved native causes and credential-safe browser/paste telemetry; typecheck, lint and build pass. CI also passes on this exact head (Node 22/24 tests, lint/typecheck, package and install checks).
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58e9f3ee1d
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const status = knownHttpStatus(error.response.status) | ||
| if (status != null) return keyValidationFailure('api_error', status) |
There was a problem hiding this comment.
When the API or an intermediary returns a non-JSON 401 response, the SDK can leave it as an Axios error—the scenario this branch explicitly handles—but the code categorizes every recognized status as api_error. Consequently the wizard reports “Please try again in a moment” and emits reason: api_error instead of telling the user the key was rejected and recording unauthorized; handle status 401 specially before returning the generic HTTP failure.
Useful? React with 👍 / 👎.
1 participant
A browser-returned or pasted key can fail local SDK token validation before an HTTP request occurs. The wizard currently reports that as a 401, calls every unexpected exception a network failure, and sends arbitrary exception text in connection-failure telemetry.
This change preserves native SDK and filesystem exceptions, including their type, stack, cause and details. It associates the known failure stage with the original error in a WeakMap without mutating or wrapping the exception. At the display/analytics boundary, the wizard classifies supported token format/type errors, actual unauthorized responses, other HTTP failures, recognized transport failures, and unknown exceptions into safe descriptors.
Connection failures report only a fixed reason, the known stage (
browser_callback,key_validation,env_write, orunknown), and a known HTTP status or null. Raw exception strings, causes, request URLs, keys, and fingerprints never enter those descriptors. The third paste failure retains the specific safe explanation, and save failures explain that verification succeeded.The browser callback protocol, key selection, authorization checks, and retry limit remain unchanged. This does not establish the cause of previously rejected keys or imply a browser transport defect. Native error details remain available in process to callers, so callers must use the safe descriptor when displaying or reporting connection failures.
Validation:
npm test(250 tests across 28 files),npm run typecheck,npm run lint, andnpm run buildon Node 24.13.0. Regressions use fixed dummy inputs, mocked fetch/browser opening, temporary projects, and a localhost callback. Coverage includes native SDK exceptions and preserved causes, malformed/wrong token types, JSON and non-JSON 401 plus status-only Axios fallback, structured 503 and non-JSON 502, generic and transport exceptions, paste retries/give-up, browser key receipt then validation failure, save failure, and credential-safe telemetry. No live credential, device, deployment, or configuration tests were performed.