-
Notifications
You must be signed in to change notification settings - Fork 1
fix: improve api error messages #176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves the API error messaging in the atlas client by refining the error detail extraction and formatting.
- Introduces an additional parameter (body) to capture detailed API errors.
- Refactors the static error construction method to handle both JSON and text responses gracefully.
Co-authored-by: Copilot <[email protected]>
src/common/atlas/apiClientError.ts
Outdated
try { | ||
text = await response.text(); | ||
} catch { | ||
text = ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we set to something like "couldn't fetch error" or "unknown error" too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
src/common/atlas/apiClientError.ts
Outdated
private constructor( | ||
message: string, | ||
public readonly response?: Response, | ||
public readonly body?: ApiError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a benefit to us storing the body?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting the ApiError model gives us insight into backend error codes, some tools might require an if/else depending on the backend error code.
Looks good but I agree we should hold off from merging/releasing for the time being like with #161 as it's not super critical yet. |
@gagik I think this change fixes type errors from apiClient |
No description provided.