-
Notifications
You must be signed in to change notification settings - Fork 4
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
API errors expects new error format #93
Conversation
@@ -0,0 +1,108 @@ | |||
import { |
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.
Thanks for adding these! It's a huge help 🙏
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.
lgtm! 🔥
describe('isMissingScopeError()', () => { | ||
it('returns true for missing scope errors', () => { | ||
const error1 = newAxiosError({ | ||
status: 403, | ||
response: { data: { category: 'MISSING_SCOPES' } }, | ||
response: { status: 403, data: { category: 'MISSING_SCOPES' } }, |
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.
I saw the AxiosError type definition does include a status property, but I wasn't able to log any statuses directly from error when testing with actual API calls... I had to go one level deeper into the response (AxiosResponse type) to pull status, hence the changes to the tests here
Description and Context
All the updates here were to update the error utilities to accept the new AxiosError error formats. The APIs were upgraded during a migration in November 2023, but the error handlers were old versions expecting a different format.
I did a full search across all 3 repos (cli-lib, LDL, CLI) and I don't believe any of the exports from
apiErrors.ts
are being used yet, so this should be a fairly low risk PR.Related CLI PR: HubSpot/hubspot-cli#990
Screenshots
n/a
TODO
n/a
Who to Notify
@shannon-lichtenwalter