diff --git a/src/snyk/common/error/errorReporter.ts b/src/snyk/common/error/errorReporter.ts index 73140a30b..393de7f89 100644 --- a/src/snyk/common/error/errorReporter.ts +++ b/src/snyk/common/error/errorReporter.ts @@ -84,8 +84,9 @@ export class ErrorReporter { Object.keys(tags).forEach(tag => scope.setTag(tag, tags[tag] as string)); return Sentry.captureException(e); }); + } else { + return Sentry.captureException(e); } - return Sentry.captureException(e); } } diff --git a/src/snyk/snykCode/error/snykCodeErrorHandler.ts b/src/snyk/snykCode/error/snykCodeErrorHandler.ts index 719f8a5aa..cdcc57e37 100644 --- a/src/snyk/snykCode/error/snykCodeErrorHandler.ts +++ b/src/snyk/snykCode/error/snykCodeErrorHandler.ts @@ -9,6 +9,16 @@ import { TagKeys, Tags } from '../../common/error/errorReporter'; import { ILog } from '../../common/logger/interfaces'; import { IContextService } from '../../common/services/contextService'; +type SnykCodeErrorResponseType = { + apiName: string; + errorCode: string; + messages: { [key: number]: unknown }; +}; + +class SnykCodeErrorResponse { + constructor(public error: SnykCodeErrorResponseType) {} +} + export interface ISnykCodeErrorHandler { resetTransientErrors(): void; get connectionRetryLimitExhausted(): boolean; @@ -39,10 +49,52 @@ export class SnykCodeErrorHandler extends ErrorHandler implements ISnykCodeError this.transientErrors = 0; } + resetRequestId(): void { + this._requestId = undefined; + } + get connectionRetryLimitExhausted(): boolean { return this._connectionRetryLimitExhausted; } + private isAuthenticationError(errorStatusCode: PropertyKey): boolean { + return errorStatusCode === constants.ErrorCodes.unauthorizedUser; + } + + private isBundleError(error: errorType): boolean { + // checkBundle API call returns 404 sometimes that gets propagated as an Error to us from 'code-client', treat as a transient error [ROAD-683] + return error instanceof Error && error.message === 'Failed to get remote bundle'; + } + + private async authenticationErrorHandler(): Promise { + await this.configuration.setToken(''); + await this.contextService.setContext(SNYK_CONTEXT.LOGGEDIN, false); + this.loadingBadge.setLoadingBadge(true); + } + + private isErrorRetryable(errorStatusCode: PropertyKey): boolean { + switch (errorStatusCode) { + case constants.ErrorCodes.badGateway: + case constants.ErrorCodes.serviceUnavailable: + case constants.ErrorCodes.serverError: + case constants.ErrorCodes.timeout: + case constants.ErrorCodes.dnsNotFound: + case constants.ErrorCodes.connectionRefused: + return true; + + default: + return false; + } + } + + private extractErrorResponse(error: errorType) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + if (!(error instanceof Error) && error?.apiName) { + const { apiName, errorCode, messages } = error as { [key: string]: string }; + return new SnykCodeErrorResponse({ apiName, errorCode, messages }); + } + } + async processError( error: errorType, options: { [key: string]: unknown } = {}, @@ -53,8 +105,12 @@ export class SnykCodeErrorHandler extends ErrorHandler implements ISnykCodeError // happens in the error handler we just log it this._requestId = requestId; + const errorResponse = this.extractErrorResponse(error); + + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const updatedError = errorResponse ? errorResponse : error; - return this.processErrorInternal(error, options, callback).catch(err => + return this.processErrorInternal(updatedError, options, callback).catch(err => ErrorHandler.handle(err, this.logger, 'Snyk Code error handler failed with error.', { [TagKeys.CodeRequestId]: this._requestId, }), @@ -66,56 +122,21 @@ export class SnykCodeErrorHandler extends ErrorHandler implements ISnykCodeError options: { [key: string]: unknown } = {}, callback: (error: Error) => void, ): Promise { - const defaultErrorHandler = () => { - // no need to retry in the case of serverError - this._connectionRetryLimitExhausted = true; - this.generalErrorHandler(error, options, callback); - }; - - const errorHandlers: { [P in constants.ErrorCodes]: () => Promise | void } = { - [constants.ErrorCodes.serverError]: defaultErrorHandler, - [constants.ErrorCodes.badGateway]: async () => { - return this.connectionErrorHandler(error, options, callback); - }, - [constants.ErrorCodes.serviceUnavailable]: async () => { - return this.connectionErrorHandler(error, options, callback); - }, - [constants.ErrorCodes.timeout]: async () => { - return this.connectionErrorHandler(error, options, callback); - }, - [constants.ErrorCodes.dnsNotFound]: async () => { - return this.connectionErrorHandler(error, options, callback); - }, - [constants.ErrorCodes.connectionRefused]: async () => { - return this.connectionErrorHandler(error, options, callback); - }, - [constants.ErrorCodes.loginInProgress]: async () => Promise.resolve(), - [constants.ErrorCodes.badRequest]: async () => Promise.resolve(), - [constants.ErrorCodes.unauthorizedUser]: async () => { - return this.authenticationErrorHandler(); - }, - [constants.ErrorCodes.unauthorizedBundleAccess]: async () => Promise.resolve(), - [constants.ErrorCodes.notFound]: async () => Promise.resolve(), - [constants.ErrorCodes.bigPayload]: async () => Promise.resolve(), - }; - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access - const errorStatusCode = error?.statusCode; - if (errorHandlers.hasOwnProperty(errorStatusCode as PropertyKey)) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access - await errorHandlers[errorStatusCode](); - } else if (error instanceof Error && error.message === 'Failed to get remote bundle') { - // checkBundle API call returns 404 sometimes that gets propagated as an Error to us from 'code-client', treat as a transient error [ROAD-683] - await this.connectionErrorHandler(error, options, callback); - } else { - defaultErrorHandler(); + const errorStatusCode = (error?.statusCode as PropertyKey) || (error?.error?.errorCode as PropertyKey); + + if (this.isAuthenticationError(errorStatusCode)) { + return await this.authenticationErrorHandler(); } - } - private async authenticationErrorHandler(): Promise { - await this.configuration.setToken(''); - await this.contextService.setContext(SNYK_CONTEXT.LOGGEDIN, false); - this.loadingBadge.setLoadingBadge(true); + if (this.isErrorRetryable(errorStatusCode) || this.isBundleError(error)) { + return await this.retryHandler(error, options, callback); + } + + this._connectionRetryLimitExhausted = true; + this.generalErrorHandler(error, options, callback); + + return Promise.resolve(); } private generalErrorHandler( @@ -124,18 +145,19 @@ export class SnykCodeErrorHandler extends ErrorHandler implements ISnykCodeError callback: (error: errorType) => void, ): void { this.transientErrors = 0; - this._requestId = undefined; - callback(error); + this.capture(error, options, { [TagKeys.CodeRequestId]: this._requestId }); + this.resetRequestId(); } - private async connectionErrorHandler( + private async retryHandler( error: errorType, options: { [key: string]: unknown }, callback: (error: Error) => void, ): Promise { this.logger.error(`Connection error to Snyk Code. Try count: ${this.transientErrors + 1}.`); + if (this.transientErrors > MAX_CONNECTION_RETRIES) { this._connectionRetryLimitExhausted = true; this.generalErrorHandler(error, options, callback); @@ -143,13 +165,19 @@ export class SnykCodeErrorHandler extends ErrorHandler implements ISnykCodeError } this.transientErrors += 1; + setTimeout(() => { this.baseSnykModule.runCodeScan().catch(err => this.capture(err, options)); }, CONNECTION_ERROR_RETRY_INTERVAL); + return Promise.resolve(); } capture(error: errorType, options: { [key: string]: unknown }, tags?: Tags): void { + if (error instanceof SnykCodeErrorResponse) { + error = new Error(JSON.stringify(error?.error)); + } + let msg = error instanceof Error ? error?.message : ''; if (Object.keys(options).length > 0) { msg += `. ${JSON.stringify(options)}`; diff --git a/src/test/unit/snykCode/error/snykCodeErrorHandler.test.ts b/src/test/unit/snykCode/error/snykCodeErrorHandler.test.ts index fc2cf26d5..01503ac67 100644 --- a/src/test/unit/snykCode/error/snykCodeErrorHandler.test.ts +++ b/src/test/unit/snykCode/error/snykCodeErrorHandler.test.ts @@ -5,7 +5,6 @@ import { IBaseSnykModule } from '../../../../snyk/base/modules/interfaces'; import { ILoadingBadge } from '../../../../snyk/base/views/loadingBadge'; import { IConfiguration } from '../../../../snyk/common/configuration/configuration'; import { CONNECTION_ERROR_RETRY_INTERVAL, MAX_CONNECTION_RETRIES } from '../../../../snyk/common/constants/general'; -import { ErrorReporter } from '../../../../snyk/common/error/errorReporter'; import { IContextService } from '../../../../snyk/common/services/contextService'; import { SnykCodeErrorHandler } from '../../../../snyk/snykCode/error/snykCodeErrorHandler'; import { LoggerMock } from '../../mocks/logger.mock'; @@ -46,6 +45,28 @@ suite('Snyk Code Error Handler', () => { }); }); + test('Handles Snyk Code api error response and retries appropriately', async function () { + this.timeout(CONNECTION_ERROR_RETRY_INTERVAL + 2000); + const error = { + apiName: 'getAnalysis', + messages: { + 500: 'Unexpected server error', + }, + errorCode: 500, + }; + + await handler.processError(error, undefined, '123456789', () => null); + + strictEqual(handler.connectionRetryLimitExhausted, false); + // assert + return new Promise((resolve, _) => { + setTimeout(() => { + strictEqual(runCodeScanFake.called, true); + resolve(); + }, CONNECTION_ERROR_RETRY_INTERVAL); + }); + }); + test('Logs analytic events once retries are exhausted', async function () { this.timeout(CONNECTION_ERROR_RETRY_INTERVAL + 2000); const error = new Error('Failed to get remote bundle');