-
Notifications
You must be signed in to change notification settings - Fork 3
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
Update status code error types #72
Conversation
@@ -13,19 +9,7 @@ function isSystemError(err: BaseError) { | |||
} | |||
|
|||
function debugErrorAndContext(error: BaseError, context?: ErrorContext): void { | |||
if (error.name === 'StatusCodeError') { |
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.
Does removing this result in more content being logged to the console? Is this filtering out some of the extra content that gets included on the error object for status code errors?
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.
errors_DEPRECATED
is only used with the old config_DEPRECATED
to make sure the logging was identical. Config doesn't make any requests so there won't be any StatusCodeError
s thrown
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.
Ah thanks for explaining. I didn't notice the file that this was in 👍
Description and Context
A few months ago, we switched from using
request
andrequest-promise-native
toaxios
, but never updated the types for http related errors. This PR removed theStatusCodeError
type associated withrequest
and updates instances of it to instead useAxiosError
TODO
Should make sure to test this thoroughly. To my knowledge, the nothing currently being used by the CLI threw
StatusCodeError
s so this should be relatively safe. I've tested with my upcoming PR that did rely onStatusCodeError
HubSpot/hubspot-cli#964Who to Notify
@brandenrodgers