-
Notifications
You must be signed in to change notification settings - Fork 5
feat: Add the ability to pass additional debug context #346
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -547,7 +547,7 @@ describe('config/utils', () => { | |
| { | ||
| ...PAK_ACCOUNT, | ||
| someUndefinedField: undefined, | ||
| } as any, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was breaking the build for me
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Strange. I just tested with |
||
| }, | ||
| ], | ||
| }; | ||
| const formattedConfig = formatConfigForWrite(configWithUndefined); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,12 @@ | ||
| import { HubSpotHttpError } from '../models/HubSpotHttpError'; | ||
| import { | ||
| HubSpotHttpError, | ||
| HubSpotHttpErrorName, | ||
| } from '../models/HubSpotHttpError'; | ||
| import { BaseError } from '../types/Error'; | ||
| import { FileSystemError } from '../models/FileSystemError'; | ||
| import { | ||
| FilerSystemErrorName, | ||
| FileSystemError, | ||
| } from '../models/FileSystemError'; | ||
| import { HubSpotConfigError } from '../models/HubSpotConfigError'; | ||
|
|
||
| export function isSpecifiedError( | ||
|
|
@@ -60,7 +66,7 @@ export function isAuthError(err: unknown): err is HubSpotHttpError { | |
| ); | ||
| } | ||
|
|
||
| export function isValidationError(err: unknown): boolean { | ||
| export function isValidationError(err: unknown): err is HubSpotHttpError { | ||
| return ( | ||
| isHubSpotHttpError(err) && | ||
| isSpecifiedError(err, { statusCode: 400 }) && | ||
|
|
@@ -69,10 +75,12 @@ export function isValidationError(err: unknown): boolean { | |
| } | ||
|
|
||
| export function isHubSpotHttpError(error?: unknown): error is HubSpotHttpError { | ||
| return !!error && error instanceof HubSpotHttpError; | ||
| return ( | ||
| !!error && error instanceof Error && error.name === HubSpotHttpErrorName | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is that?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When packages are linked in some places, but not other places, you get multiple versions of the same error class. I.E. if we have LDL linked in the CLI, but not in PPL they will each have their own version of the |
||
| ); | ||
| } | ||
|
|
||
| export function isGithubRateLimitError(err: unknown): boolean { | ||
| export function isGithubRateLimitError(err: unknown): err is HubSpotHttpError { | ||
| if (!isHubSpotHttpError(err)) { | ||
| return false; | ||
| } | ||
|
|
@@ -96,7 +104,7 @@ export function isSystemError(err: unknown): err is BaseError { | |
| } | ||
|
|
||
| export function isFileSystemError(err: unknown): err is FileSystemError { | ||
| return err instanceof FileSystemError; | ||
| return err instanceof Error && err.name === FilerSystemErrorName; | ||
| } | ||
|
|
||
| export function isHubSpotConfigError(err: unknown): err is HubSpotConfigError { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,8 @@ import { HttpMethod } from '../types/Api'; | |
| import { HTTP_METHOD_PREPOSITIONS, HTTP_METHOD_VERBS } from '../constants/api'; | ||
| import { i18n } from '../utils/lang'; | ||
|
|
||
| export const HubSpotHttpErrorName = 'HubSpotHttpError'; | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| export class HubSpotHttpError<T = any> extends Error { | ||
| public status?: number; | ||
|
|
@@ -27,7 +29,7 @@ export class HubSpotHttpError<T = any> extends Error { | |
| context?: HubSpotHttpErrorContext | ||
| ) { | ||
| super(message, options); | ||
| this.name = 'HubSpotHttpError'; | ||
| this.name = HubSpotHttpErrorName; | ||
| this.context = context; | ||
| this.cause = options?.cause; | ||
|
|
||
|
|
@@ -77,11 +79,18 @@ export class HubSpotHttpError<T = any> extends Error { | |
| } | ||
| } | ||
|
|
||
| public updateContext(context: Partial<HubSpotHttpErrorContext>) { | ||
| public updateContext( | ||
| context: Partial<HubSpotHttpErrorContext>, | ||
| additionalDebugContext?: string | ||
| ) { | ||
| this.context = { ...this.context, ...context }; | ||
| // Update the error messages when the context is updated | ||
| if (isAxiosError(this.cause)) { | ||
| this.message = this.joinErrorMessages(this.cause, this.context); | ||
| this.message = this.joinErrorMessages( | ||
| this.cause, | ||
| this.context, | ||
| additionalDebugContext | ||
| ); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -176,9 +185,9 @@ export class HubSpotHttpError<T = any> extends Error { | |
|
|
||
| private joinErrorMessages( | ||
| error: AxiosError<{ message: string; errors: { message: string }[] }>, | ||
| context: HubSpotHttpErrorContext = {} | ||
| context: HubSpotHttpErrorContext = {}, | ||
| additionalDebugContext?: string | ||
| ): string { | ||
| const i18nKey = 'errors.apiErrors'; | ||
| const status = error.response?.status; | ||
| const method = error.config?.method as HttpMethod; | ||
|
|
||
|
|
@@ -196,57 +205,75 @@ export class HubSpotHttpError<T = any> extends Error { | |
| ? `${action} ${preposition} '${context.request}'` | ||
| : action; | ||
|
|
||
| messageDetail = i18n(`${i18nKey}.messageDetail`, { | ||
| messageDetail = i18n(`errors.apiErrors.messageDetail`, { | ||
| accountId: context.accountId, | ||
| requestName, | ||
| }); | ||
| } else { | ||
| messageDetail = i18n(`${i18nKey}.genericMessageDetail`); | ||
| messageDetail = i18n(`errors.apiErrors.genericMessageDetail`); | ||
| } | ||
|
|
||
| const errorMessage: Array<string> = []; | ||
|
|
||
| if ((method === 'put' || method === 'post') && context.payload) { | ||
| errorMessage.push( | ||
| i18n(`${i18nKey}.unableToUpload`, { payload: context.payload }) | ||
| i18n(`errors.apiErrors.unableToUpload`, { payload: context.payload }) | ||
| ); | ||
| } | ||
|
|
||
| let statusBasedMessage: string | undefined; | ||
|
|
||
| switch (status) { | ||
| case 400: | ||
| errorMessage.push(i18n(`${i18nKey}.codes.400`, { messageDetail })); | ||
| statusBasedMessage = i18n(`errors.apiErrors.codes.400`, { | ||
| messageDetail, | ||
| }); | ||
| break; | ||
| case 401: | ||
| errorMessage.push(i18n(`${i18nKey}.codes.401`, { messageDetail })); | ||
| statusBasedMessage = i18n(`errors.apiErrors.codes.401`, { | ||
| messageDetail, | ||
| }); | ||
| break; | ||
| case 403: | ||
| break; | ||
| case 404: | ||
| errorMessage.push(i18n(`${i18nKey}.codes.404`, { messageDetail })); | ||
| statusBasedMessage = i18n(`errors.apiErrors.codes.404`, { | ||
| messageDetail, | ||
| }); | ||
| break; | ||
| case 429: | ||
| errorMessage.push(i18n(`${i18nKey}.codes.429`, { messageDetail })); | ||
| statusBasedMessage = i18n(`errors.apiErrors.codes.429`, { | ||
| messageDetail, | ||
| }); | ||
| break; | ||
| case 503: | ||
| errorMessage.push(i18n(`${i18nKey}.codes.503`, { messageDetail })); | ||
| statusBasedMessage = i18n(`errors.apiErrors.codes.503`, { | ||
| messageDetail, | ||
| }); | ||
| break; | ||
| default: | ||
| if (status && status >= 500 && status < 600) { | ||
| errorMessage.push( | ||
| i18n(`${i18nKey}.codes.500Generic`, { messageDetail }) | ||
| ); | ||
| statusBasedMessage = i18n(`errors.apiErrors.codes.500Generic`, { | ||
| messageDetail, | ||
| }); | ||
| } else if (status && status >= 400 && status < 500) { | ||
| errorMessage.push( | ||
| i18n(`${i18nKey}.codes.400Generic`, { messageDetail }) | ||
| ); | ||
| statusBasedMessage = i18n(`errors.apiErrors.codes.400Generic`, { | ||
| messageDetail, | ||
| }); | ||
| } else { | ||
| errorMessage.push( | ||
| i18n(`${i18nKey}.codes.generic`, { messageDetail }) | ||
| ); | ||
| statusBasedMessage = i18n(`errors.apiErrors.codes.generic`, { | ||
| messageDetail, | ||
| }); | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| if (statusBasedMessage) { | ||
| errorMessage.push( | ||
| `${statusBasedMessage}${additionalDebugContext ? ` ${additionalDebugContext}` : ''}` | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If there is additional context, append it to the error message |
||
| ); | ||
| } | ||
|
|
||
| if (error?.response?.data) { | ||
| const { message, errors } = error.response.data; | ||
|
|
||
|
|
||
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.
Prettier write added these, so I just went with it