fix: improve API error messages and auth failure UX#2051
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis patch release improves SDK error messaging by extracting details from JSON responses and consolidates HTTP error handling into shared utilities. The Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/sdk/src/index.ts (1)
184-184: Consider adding context for localization errors.Unlike
recognizeLocalewhich passes"Error recognizing locale"as context,localizeChunkcallsthrowOnHttpErrorwithout context. For 401/403/404 errors, the user will see only the extracted message without operation context. This is a minor inconsistency.Optional: Add context
- await LingoDotDevEngine.throwOnHttpError(res); + await LingoDotDevEngine.throwOnHttpError(res, "Error localizing content");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/index.ts` at line 184, The call in localizeChunk to LingoDotDevEngine.throwOnHttpError(res) lacks operation context so HTTP errors show only the extracted message; update the call in localizeChunk to pass a descriptive context string (e.g., "Error localizing chunk" or similar) like recognizeLocale does, so replace the bare call to LingoDotDevEngine.throwOnHttpError(res) with a call that supplies the context argument to throwOnHttpError to provide clearer error messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/sdk/src/index.ts`:
- Around line 53-54: The NotFoundError branch uses parsed.entityType and
parsed.id directly which can be undefined; update the NotFoundError handling
(the branch checking parsed?._tag === "NotFoundError") to use safe fallbacks
(e.g., default entityType to "entity" or "unknown entity" and id to "unknown id"
or empty string) when constructing the message so it never produces "undefined
not found: undefined".
---
Nitpick comments:
In `@packages/sdk/src/index.ts`:
- Line 184: The call in localizeChunk to LingoDotDevEngine.throwOnHttpError(res)
lacks operation context so HTTP errors show only the extracted message; update
the call in localizeChunk to pass a descriptive context string (e.g., "Error
localizing chunk" or similar) like recognizeLocale does, so replace the bare
call to LingoDotDevEngine.throwOnHttpError(res) with a call that supplies the
context argument to throwOnHttpError to provide clearer error messages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a2ffd22b-b91f-4606-a5fd-6f430bd22235
📒 Files selected for processing (4)
.changeset/gentle-gifts-tell.mdpackages/cli/src/cli/cmd/run/index.tspackages/cli/src/cli/localizer/lingodotdev.tspackages/sdk/src/index.ts
| if (parsed?._tag === "NotFoundError") { | ||
| return `${parsed.entityType} not found: ${parsed.id}`; |
There was a problem hiding this comment.
Add defensive checks for NotFoundError properties.
If the server sends { "_tag": "NotFoundError" } without entityType or id, this will produce the message "undefined not found: undefined". Consider adding fallback values.
Proposed fix
if (parsed?._tag === "NotFoundError") {
- return `${parsed.entityType} not found: ${parsed.id}`;
+ return `${parsed.entityType ?? "Entity"} not found: ${parsed.id ?? "unknown"}`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (parsed?._tag === "NotFoundError") { | |
| return `${parsed.entityType} not found: ${parsed.id}`; | |
| if (parsed?._tag === "NotFoundError") { | |
| return `${parsed.entityType ?? "Entity"} not found: ${parsed.id ?? "unknown"}`; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sdk/src/index.ts` around lines 53 - 54, The NotFoundError branch
uses parsed.entityType and parsed.id directly which can be undefined; update the
NotFoundError handling (the branch checking parsed?._tag === "NotFoundError") to
use safe fallbacks (e.g., default entityType to "entity" or "unknown entity" and
id to "unknown id" or empty string) when constructing the message so it never
produces "undefined not found: undefined".
* fix: improve API error messages and auth failure UX
2 participants
Summary
Improve API error messages by parsing server response bodies and surfacing actionable auth failure messages instead of generic HTTP status text.
Changes
extractErrorMessagehelper — parses JSON error responses from the API, extracting.messagefield or formattingNotFoundError, instead of relying onres.statusTextthrowOnHttpErrorhelper — deduplicates error-handling logic acrosslocalizeChunkandrecognizeLocalewhoamiremoves try/catch — network errors (DNS, timeout) now propagate instead of being silently swallowed asnull, which caused misleading "Invalid API key" messages when the server was unreachableworkflowId— was passed through_localizeRaw→localizeChunkbut never included in the request bodycheckAuthreturns specific error — "Invalid API key" message whenwhoamireturnsnull(401), real error message whenwhoamithrows (network/server errors)Testing
Business logic tests added:
lingo.dev loginor check your LINGO_API_KEY."Visuals
N/A
Checklist
Summary by CodeRabbit