From cdbfd59bf0ad477fefa1cfbbdf3002856a027810 Mon Sep 17 00:00:00 2001 From: Branden Rodgers Date: Fri, 13 Oct 2023 15:55:16 -0400 Subject: [PATCH 1/2] initial work to add tests for apiErrors and standardErrors --- errors/__tests__/apiErrors.ts | 168 +++++++++++++++++++++++++ errors/__tests__/standardErrors.ts | 81 ++++++++++++ errors/apiErrors.ts | 77 +++++++----- errors/standardErrors.ts | 2 +- lang/en.lyaml | 8 +- lib/__tests__/modules.ts | 2 +- lib/__tests__/path.ts | 2 +- {errors => models}/HubSpotAuthError.ts | 0 types/Error.ts | 4 + 9 files changed, 307 insertions(+), 37 deletions(-) create mode 100644 errors/__tests__/apiErrors.ts create mode 100644 errors/__tests__/standardErrors.ts rename {errors => models}/HubSpotAuthError.ts (100%) diff --git a/errors/__tests__/apiErrors.ts b/errors/__tests__/apiErrors.ts new file mode 100644 index 00000000..313fdad3 --- /dev/null +++ b/errors/__tests__/apiErrors.ts @@ -0,0 +1,168 @@ +import { + isApiStatusCodeError, + isMissingScopeError, + isGatingError, + isApiUploadValidationError, + isSpecifiedHubSpotAuthError, + throwStatusCodeError, + throwApiStatusCodeError, + throwApiError, + throwApiUploadError, +} from '../apiErrors'; +import { BaseError, GenericError, StatusCodeError } from '../../types/Error'; + +export const newError = (overrides = {}): BaseError => { + return { + name: 'Error', + message: 'An error ocurred', + statusCode: 200, + errors: [], + ...overrides, + }; +}; + +export const newStatutsCodeError = (overrides = {}): GenericError => { + return { + ...newError(), + name: 'StatusCodeError', + response: { + request: { + href: 'http://example.com/', + method: 'GET', + }, + body: {}, + headers: {}, + statusCode: 200, + }, + ...overrides, + }; +}; + +describe('apiErrors', () => { + describe('isApiStatusCodeError', () => { + it('returns true for api status code errors', () => { + const error1 = newError({ statusCode: 100 }); + const error2 = newError({ statusCode: 599 }); + const error3 = newStatutsCodeError({ statusCode: 99 }); + expect(isApiStatusCodeError(error1)).toBe(true); + expect(isApiStatusCodeError(error2)).toBe(true); + expect(isApiStatusCodeError(error3)).toBe(true); + }); + + it('returns false for non api status code errors', () => { + const error1 = newError({ statusCode: 99 }); + const error2 = newError({ statusCode: 600 }); + expect(isApiStatusCodeError(error1)).toBe(false); + expect(isApiStatusCodeError(error2)).toBe(false); + }); + }); + + describe('isMissingScopeError', () => { + it('returns true for missing scope errors', () => { + const error1 = newStatutsCodeError({ + statusCode: 403, + error: { category: 'MISSING_SCOPES' }, + }); + expect(isMissingScopeError(error1)).toBe(true); + }); + + it('returns false for non missing scope errors', () => { + const error1 = newStatutsCodeError({ + statusCode: 400, + error: { category: 'MISSING_SCOPES' }, + }); + const error2 = newStatutsCodeError({ name: 'NonStatusCodeError' }); + expect(isMissingScopeError(error1)).toBe(false); + expect(isMissingScopeError(error2)).toBe(false); + }); + }); + + describe('isGatingError', () => { + it('returns true for gating errors', () => { + const error1 = newStatutsCodeError({ + statusCode: 403, + error: { category: 'GATED' }, + }); + expect(isGatingError(error1)).toBe(true); + }); + + it('returns false for non gating errors', () => { + const error1 = newStatutsCodeError({ + statusCode: 400, + error: { category: 'GATED' }, + }); + const error2 = newStatutsCodeError({ name: 'NonStatusCodeError' }); + expect(isGatingError(error1)).toBe(false); + expect(isGatingError(error2)).toBe(false); + }); + }); + + describe('isApiUploadValidationError', () => { + it('returns true for api upload validation errors', () => { + const error1 = newStatutsCodeError({ + statusCode: 400, + response: { body: { message: 'upload validation error' } }, + }); + const error2 = newStatutsCodeError({ + statusCode: 400, + response: { body: { errors: [] } }, + }); + expect(isApiUploadValidationError(error1)).toBe(true); + expect(isApiUploadValidationError(error2)).toBe(true); + }); + + it('returns false for non api upload validation errors', () => { + const error1 = newStatutsCodeError({ + statusCode: 400, + response: { body: null }, + }); + const error2 = newStatutsCodeError({ name: 'NonStatusCodeError' }); + expect(isApiUploadValidationError(error1)).toBe(false); + expect(isApiUploadValidationError(error2)).toBe(false); + }); + }); + + describe('isSpecifiedHubSpotAuthError', () => { + it('returns true for matching HubSpot auth errors', () => { + const error1 = newError({ name: 'HubSpotAuthError', statusCode: 123 }); + expect(isSpecifiedHubSpotAuthError(error1, { statusCode: 123 })).toBe( + true + ); + }); + + it('returns false for non matching HubSpot auth errors', () => { + const error1 = newError({ name: 'HubSpotAuthError', statusCode: 123 }); + expect(isSpecifiedHubSpotAuthError(error1, { statusCode: 124 })).toBe( + false + ); + }); + }); + + describe('throwStatusCodeError', () => { + it('throws status code error', () => { + const error = newStatutsCodeError() as StatusCodeError; + expect(() => throwStatusCodeError(error)).toThrow(); + }); + }); + + describe('throwApiStatusCodeError', () => { + it('throws api status code error', () => { + const error = newStatutsCodeError() as StatusCodeError; + expect(() => throwApiStatusCodeError(error)).toThrow(); + }); + }); + + describe('throwApiError', () => { + it('throws api error', () => { + const error = newStatutsCodeError() as StatusCodeError; + expect(() => throwApiError(error)).toThrow(); + }); + }); + + describe('throwApiUploadError', () => { + it('throws api upload error', () => { + const error = newStatutsCodeError() as StatusCodeError; + expect(() => throwApiUploadError(error)).toThrow(); + }); + }); +}); diff --git a/errors/__tests__/standardErrors.ts b/errors/__tests__/standardErrors.ts new file mode 100644 index 00000000..b399c1cb --- /dev/null +++ b/errors/__tests__/standardErrors.ts @@ -0,0 +1,81 @@ +import { + isSystemError, + isFatalError, + throwErrorWithMessage, + throwTypeErrorWithMessage, + throwAuthErrorWithMessage, + throwError, +} from '../standardErrors'; +import { BaseError, StatusCodeError } from '../../types/Error'; +import { HubSpotAuthError } from '../../models/HubSpotAuthError'; + +export const newError = (overrides = {}): BaseError => { + return { + name: 'Error', + message: 'An error ocurred', + errno: 1, + code: 'error_code', + syscall: 'error_syscall', + errors: [], + ...overrides, + }; +}; + +describe('standardErrors', () => { + describe('isSystemError', () => { + it('returns true for system errors', () => { + const error = newError(); + expect(isSystemError(error)).toBe(true); + }); + + it('returns false for non system errors', () => { + const error1 = newError({ errno: null }); + const error2 = newError({ code: null }); + const error3 = newError({ syscall: null }); + expect(isSystemError(error1)).toBe(false); + expect(isSystemError(error2)).toBe(false); + expect(isSystemError(error3)).toBe(false); + }); + }); + + describe('isFatalError', () => { + it('returns true for fatal errors', () => { + const cause = newError() as StatusCodeError; + const error = new HubSpotAuthError('A fatal auth error', { cause }); + expect(isFatalError(error)).toBe(true); + }); + + it('returns false for non fatal errors', () => { + const error = newError(); + expect(isFatalError(error)).toBe(false); + }); + }); + + describe('throwErrorWithMessage', () => { + it('throws error with message', () => { + const error = newError() as BaseError; + expect(() => throwErrorWithMessage('', {}, error)).toThrow(); + }); + }); + + describe('throwTypeErrorWithMessage', () => { + it('throws type error with message', () => { + const error = newError() as BaseError; + expect(() => throwTypeErrorWithMessage('', {}, error)).toThrow(); + }); + }); + + describe('throwAuthErrorWithMessage', () => { + it('throws auth error with message', () => { + const error = newError() as StatusCodeError; + expect(() => throwAuthErrorWithMessage('', {}, error)).toThrow(); + }); + }); + + describe('throwError', () => { + it('throws error', () => { + const error = newError() as BaseError; + expect(() => throwError(error)).toThrow(); + }); + }); +}); diff --git a/errors/apiErrors.ts b/errors/apiErrors.ts index 4a841c05..275c6ad6 100644 --- a/errors/apiErrors.ts +++ b/errors/apiErrors.ts @@ -1,55 +1,60 @@ -import { StatusCodeError, StatusCodeErrorContext } from '../types/Error'; +import { + GenericError, + StatusCodeError, + StatusCodeErrorContext, +} from '../types/Error'; import { HTTP_METHOD_VERBS, HTTP_METHOD_PREPOSITIONS } from '../constants/api'; import { i18n } from '../utils/lang'; import { throwError } from './standardErrors'; -import { HubSpotAuthError } from './HubSpotAuthError'; +import { HubSpotAuthError } from '../models/HubSpotAuthError'; -function isApiStatusCodeError(err: StatusCodeError) { +export function isApiStatusCodeError(err: GenericError): boolean { return ( err.name === 'StatusCodeError' || - (err.statusCode && err.statusCode >= 100 && err.statusCode < 600) + (!!err.statusCode && err.statusCode >= 100 && err.statusCode < 600) ); } -export function isMissingScopeError(err: StatusCodeError): boolean { - return Boolean( - err.name === 'StatusCodeError' && - err.statusCode === 403 && - err.error && - err.error.category === 'MISSING_SCOPES' +export function isMissingScopeError(err: GenericError): boolean { + return ( + isApiStatusCodeError(err) && + err.statusCode === 403 && + !!err.error && + err.error.category === 'MISSING_SCOPES' ); } -export function isGatingError(err: StatusCodeError): boolean { - return Boolean( - err.name === 'StatusCodeError' && - err.statusCode === 403 && - err.error && - err.error.category === 'GATED' +export function isGatingError(err: GenericError): boolean { + return ( + isApiStatusCodeError(err) && + err.statusCode === 403 && + !!err.error && + err.error.category === 'GATED' ); } -function isApiUploadValidationError(err: StatusCodeError): boolean { - return Boolean( +export function isApiUploadValidationError(err: GenericError): boolean { + return ( + isApiStatusCodeError(err) && err.statusCode === 400 && - err.response && - err.response.body && - (err.response.body.message || err.response.body.errors) + !!err.response && + !!err.response.body && + !!(err.response.body.message || !!err.response.body.errors) ); } export function isSpecifiedHubSpotAuthError( - err: HubSpotAuthError, + err: GenericError, { statusCode, category, subCategory }: Partial ): boolean { const statusCodeErr = !statusCode || err.statusCode === statusCode; const categoryErr = !category || err.category === category; const subCategoryErr = !subCategory || err.subCategory === subCategory; - return ( + return Boolean( err.name === 'HubSpotAuthError' && - statusCodeErr && - categoryErr && - subCategoryErr + statusCodeErr && + categoryErr && + subCategoryErr ); } @@ -89,6 +94,9 @@ function logValidationErrors(error: StatusCodeError) { } } +/** + * @throws + */ export function throwStatusCodeError( error: StatusCodeError, context: StatusCodeErrorContext = {} @@ -106,11 +114,14 @@ export function throwStatusCodeError( throw new Error(errorData, { cause: error }); } +/** + * @throws + */ export function throwApiStatusCodeError( error: StatusCodeError, - context: StatusCodeErrorContext + context: StatusCodeErrorContext = {} ): never { - const i18nKey = 'errors.api'; + const i18nKey = 'errors.errorTypes.api'; const { statusCode } = error; const { method } = error.options || {}; const { projectName } = context; @@ -213,9 +224,12 @@ export function throwApiStatusCodeError( throwError(new Error(errorMessage.join(' '), { cause: error })); } +/** + * @throws + */ export function throwApiError( error: StatusCodeError, - context: StatusCodeErrorContext + context: StatusCodeErrorContext = {} ): never { if (isApiStatusCodeError(error)) { throwApiStatusCodeError(error, context); @@ -223,9 +237,12 @@ export function throwApiError( throwError(error); } +/** + * @throws + */ export function throwApiUploadError( error: StatusCodeError, - context: StatusCodeErrorContext + context: StatusCodeErrorContext = {} ): never { if (isApiUploadValidationError(error)) { logValidationErrors(error); diff --git a/errors/standardErrors.ts b/errors/standardErrors.ts index be5c4c24..be1d78cc 100644 --- a/errors/standardErrors.ts +++ b/errors/standardErrors.ts @@ -1,4 +1,4 @@ -import { HubSpotAuthError } from './HubSpotAuthError'; +import { HubSpotAuthError } from '../models/HubSpotAuthError'; import { i18n } from '../utils/lang'; import { throwStatusCodeError } from './apiErrors'; diff --git a/lang/en.lyaml b/lang/en.lyaml index b3b58924..ca340df5 100644 --- a/lang/en.lyaml +++ b/lang/en.lyaml @@ -134,8 +134,8 @@ en: logging: creatingFile: "Creating file at {{ path }}" processFieldsJs: - converting: "Converting \"{{ src }}\" to \"{{ dest }}\"." - converted: "Finished converting \"{{ src }}\" to \"{{ dest }}\"." + converting: 'Converting "{{ src }}" to "{{ dest }}".' + converted: 'Finished converting "{{ src }}" to "{{ dest }}".' errors: hubdb: invalidJsonPath: "The HubDB table file must be a '.json' file" @@ -242,6 +242,6 @@ en: fetchTaskStatus: "There was an error fetching the task status while syncing sandboxes." fetchTypes: "There was an error fetching sandbox types." processFieldsJs: - fieldsJsNotReturnArray: "There was an error loading JS file \"{{ path }}\". Expected type \"Array\". Make sure that your function returns an array" - fieldsJsNotFunction: "There was an error loading JS file \"{{ path }}\". Expected type \"Function\". Make sure that your default export is a function." + fieldsJsNotReturnArray: 'There was an error loading JS file "{{ path }}". Expected type "Array". Make sure that your function returns an array' + fieldsJsNotFunction: 'There was an error loading JS file "{{ path }}". Expected type "Function". Make sure that your default export is a function.' invalidMjsFile: ".mjs files are only supported when using Node 13.2.0+" diff --git a/lib/__tests__/modules.ts b/lib/__tests__/modules.ts index 370e7002..fb992527 100644 --- a/lib/__tests__/modules.ts +++ b/lib/__tests__/modules.ts @@ -10,7 +10,7 @@ const isHubSpot = true; // TODO: Replace two missing tests -describe('cli-lib/modules', () => { +describe('modules', () => { describe('validateSrcAndDestPaths()', () => { const emptyLocal = { isLocal, path: '' }; const emptyHubSpot = { isHubSpot, path: '' }; diff --git a/lib/__tests__/path.ts b/lib/__tests__/path.ts index d1b3d72a..df77b1f3 100644 --- a/lib/__tests__/path.ts +++ b/lib/__tests__/path.ts @@ -1,7 +1,7 @@ import path, { PlatformPath } from 'path'; import { splitHubSpotPath, splitLocalPath } from '../path'; -describe('cli-lib/path', () => { +describe('path', () => { describe('splitHubSpotPath()', () => { const testSplit = ( filepath: string, diff --git a/errors/HubSpotAuthError.ts b/models/HubSpotAuthError.ts similarity index 100% rename from errors/HubSpotAuthError.ts rename to models/HubSpotAuthError.ts diff --git a/types/Error.ts b/types/Error.ts index f6ead280..ed2cf8ac 100644 --- a/types/Error.ts +++ b/types/Error.ts @@ -1,5 +1,9 @@ import { HttpMethod } from './Api'; +export interface GenericError extends Error { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + [key: string]: any; +} // See https://nodejs.org/api/errors.html#class-systemerror export interface BaseError extends Error { name: string; From 11a8cb913825687f66f9e9833ef9fffbf125adbd Mon Sep 17 00:00:00 2001 From: Branden Rodgers Date: Mon, 16 Oct 2023 15:11:58 -0400 Subject: [PATCH 2/2] dont need the casting --- errors/__tests__/standardErrors.ts | 6 +++--- types/Error.ts | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/errors/__tests__/standardErrors.ts b/errors/__tests__/standardErrors.ts index b399c1cb..e18852d0 100644 --- a/errors/__tests__/standardErrors.ts +++ b/errors/__tests__/standardErrors.ts @@ -53,14 +53,14 @@ describe('standardErrors', () => { describe('throwErrorWithMessage', () => { it('throws error with message', () => { - const error = newError() as BaseError; + const error = newError(); expect(() => throwErrorWithMessage('', {}, error)).toThrow(); }); }); describe('throwTypeErrorWithMessage', () => { it('throws type error with message', () => { - const error = newError() as BaseError; + const error = newError(); expect(() => throwTypeErrorWithMessage('', {}, error)).toThrow(); }); }); @@ -74,7 +74,7 @@ describe('standardErrors', () => { describe('throwError', () => { it('throws error', () => { - const error = newError() as BaseError; + const error = newError(); expect(() => throwError(error)).toThrow(); }); }); diff --git a/types/Error.ts b/types/Error.ts index ed2cf8ac..3d5a5c81 100644 --- a/types/Error.ts +++ b/types/Error.ts @@ -4,6 +4,7 @@ export interface GenericError extends Error { // eslint-disable-next-line @typescript-eslint/no-explicit-any [key: string]: any; } + // See https://nodejs.org/api/errors.html#class-systemerror export interface BaseError extends Error { name: string;