Skip to content

Commit

Permalink
fix: standardised Snyk catalog errors handling
Browse files Browse the repository at this point in the history
  • Loading branch information
metju90 committed Dec 12, 2023
1 parent 0342435 commit 2e5451c
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 107 deletions.
38 changes: 13 additions & 25 deletions src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
ScmReportOptions,
} from './interfaces/analysis-options.interface';
import { generateErrorWithDetail, getURL } from './utils/httpUtils';
import { JsonApiError } from './interfaces/json-api';
import { JsonApiErrorObject } from './interfaces/json-api';

type ResultSuccess<T> = { type: 'success'; value: T };

Expand Down Expand Up @@ -52,10 +52,10 @@ function generateError<E>(
messages: { [c: number]: string },
apiName: string,
errorMessage?: string,
jsonApiError?: JsonApiError,
errors?: JsonApiErrorObject[],
): ResultError<E> {
if (jsonApiError) {
return generateErrorWithDetail<E>(jsonApiError, errorCode, apiName);
if (errors) {
return generateErrorWithDetail<E>(errors[0], errorCode, apiName);
}

if (!isSubsetErrorCode<E>(errorCode, messages)) {
Expand Down Expand Up @@ -222,7 +222,7 @@ export async function getFilters({
if (res.success) {
return { type: 'success', value: res.body };
}
return generateError<GenericErrorTypes>(res.errorCode, GENERIC_ERROR_MESSAGES, apiName, undefined, res.jsonApiError);
return generateError<GenericErrorTypes>(res.errorCode, GENERIC_ERROR_MESSAGES, apiName, undefined, res.errors);
}

function commonHttpHeaders(options: ConnectionOptions) {
Expand Down Expand Up @@ -294,7 +294,7 @@ export async function createBundle(
CREATE_BUNDLE_ERROR_MESSAGES,
'createBundle',
undefined,
res.jsonApiError,
res.errors,
);
}

Expand Down Expand Up @@ -338,7 +338,7 @@ export async function checkBundle(options: CheckBundleOptions): Promise<Result<R
CHECK_BUNDLE_ERROR_MESSAGES,
'checkBundle',
undefined,
res.jsonApiError,
res.errors,
);
}

Expand Down Expand Up @@ -394,7 +394,7 @@ export async function extendBundle(
EXTEND_BUNDLE_ERROR_MESSAGES,
'extendBundle',
undefined,
res.jsonApiError,
res.errors,
);
}

Expand Down Expand Up @@ -475,7 +475,7 @@ export async function getAnalysis(
GET_ANALYSIS_ERROR_MESSAGES,
'getAnalysis',
undefined,
res.jsonApiError,
res.errors,
);
}

Expand Down Expand Up @@ -552,13 +552,7 @@ export async function initReport(options: UploadReportOptions): Promise<Result<s

const res = await makeRequest<InitUploadResponseDto>(config);
if (res.success) return { type: 'success', value: res.body.reportId };
return generateError<ReportErrorCodes>(
res.errorCode,
REPORT_ERROR_MESSAGES,
'initReport',
undefined,
res.jsonApiError,
);
return generateError<ReportErrorCodes>(res.errorCode, REPORT_ERROR_MESSAGES, 'initReport', undefined, res.errors);
}

/**
Expand Down Expand Up @@ -588,7 +582,7 @@ export async function getReport(options: GetReportOptions): Promise<Result<Uploa
REPORT_ERROR_MESSAGES,
'getReport',
res.error?.message,
res.jsonApiError,
res.errors,
);
}

Expand Down Expand Up @@ -620,13 +614,7 @@ export async function initScmReport(options: ScmUploadReportOptions): Promise<Re

const res = await makeRequest<InitScmUploadResponseDto>(config);
if (res.success) return { type: 'success', value: res.body.testId };
return generateError<ReportErrorCodes>(
res.errorCode,
REPORT_ERROR_MESSAGES,
'initReport',
undefined,
res.jsonApiError,
);
return generateError<ReportErrorCodes>(res.errorCode, REPORT_ERROR_MESSAGES, 'initReport', undefined, res.errors);
}

/**
Expand Down Expand Up @@ -658,6 +646,6 @@ export async function getScmReport(
REPORT_ERROR_MESSAGES,
'getReport',
res.error?.message,
res.jsonApiError,
res.errors,
);
}
24 changes: 23 additions & 1 deletion src/interfaces/json-api.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,31 @@
export type JsonApiError = {
enum Classification {
UNEXPECTED = 'UNEXPECTED',
ACTIONABLE = 'ACTIONABLE',
UNSUPPORTED = 'UNSUPPORTED',
}

type JsonApiErrorSource = { pointer: string } | { parameter: string } | { header: string };

export type JsonApiErrorObject = {
id?: string;
links?: {
about?: string;
};
status: string;
code: string;
title: string;
detail: string;
source?: JsonApiErrorSource;
meta: {
[x: string]: any;

// Allow consumers to probe if this specific type of JsonApi
// response originates from an error catalog error.
isErrorCatalogError: boolean;

classification?: Classification;

// Logs related to the error that was thrown
logs?: string[];
};
};
16 changes: 5 additions & 11 deletions src/needle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ import needle from 'needle';
import * as querystring from 'querystring';
import https from 'https';
import { URL } from 'url';
import { emitter } from './emitter';
import { JsonApiErrorObject } from './interfaces/json-api';

import { emitter } from './emitter';
import { ErrorCodes, NETWORK_ERRORS, MAX_RETRY_ATTEMPTS, REQUEST_RETRY_DELAY } from './constants';
import { JsonApiError } from './interfaces/json-api';
import { isJsonApiErrors } from './utils/httpUtils';

const sleep = (duration: number) => new Promise(resolve => setTimeout(resolve, duration));

Expand Down Expand Up @@ -52,7 +51,7 @@ export type FailedResponse = {
success: false;
errorCode: number;
error: Error | undefined;
jsonApiError?: JsonApiError | undefined;
errors?: JsonApiErrorObject[] | undefined;
};

export async function makeRequest<T = void>(
Expand Down Expand Up @@ -98,7 +97,6 @@ export async function makeRequest<T = void>(
do {
let errorCode: number | undefined;
let error: Error | undefined;
let jsonApiError: JsonApiError | undefined;
let response: needle.NeedleResponse | undefined;

try {
Expand All @@ -116,11 +114,7 @@ export async function makeRequest<T = void>(
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access
const errorMessage = response?.body?.error;
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-member-access
const errors = response?.body?.errors;

if (isJsonApiErrors(errors)) {
jsonApiError = errors[0];
}
const errors = response?.body?.errors as JsonApiErrorObject[] | undefined;

if (errorMessage) {
error = error ?? new Error(errorMessage);
Expand All @@ -143,7 +137,7 @@ export async function makeRequest<T = void>(
await sleep(REQUEST_RETRY_DELAY);
} else {
attempts = 0;
return { success: false, errorCode, error, jsonApiError };
return { success: false, errorCode, error, errors };
}
} while (attempts > 0);

Expand Down
28 changes: 6 additions & 22 deletions src/utils/httpUtils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ORG_ID_REGEXP } from '../constants';
import { JsonApiError } from '../interfaces/json-api';
import { JsonApiErrorObject } from '../interfaces/json-api';
import { ResultError } from '../http';

export function getURL(baseURL: string, path: string, orgId?: string): string {
Expand All @@ -20,27 +20,11 @@ function isValidOrg(orgId?: string): boolean {
return orgId !== undefined && ORG_ID_REGEXP.test(orgId);
}

export function isJsonApiErrors(input: unknown): input is JsonApiError[] {
if (!Array.isArray(input) || input.length < 1) {
return false;
}

for (const element of input) {
if (
typeof element !== 'object' ||
!('status' in element) ||
!('code' in element) ||
!('title' in element) ||
!('detail' in element)
) {
return false;
}
}

return true;
}

export function generateErrorWithDetail<E>(error: JsonApiError, statusCode: number, apiName: string): ResultError<E> {
export function generateErrorWithDetail<E>(
error: JsonApiErrorObject,
statusCode: number,
apiName: string,
): ResultError<E> {
const errorLink = error.links?.about;
const detail = `${error.title}${error.detail ? `: ${error.detail}` : ''}${
errorLink ? ` (more info: ${errorLink})` : ``
Expand Down
2 changes: 1 addition & 1 deletion tests/api.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ describe('Requests to public API', () => {
if (response.value.status === AnalysisStatus.complete && response.value.type === 'sarif') {
expect(response.value.sarif.runs[0].results?.length).toBeGreaterThan(0);

expect(response.value.coverage).toIncludeSameMembers([
expect(response.value.coverage).toIncludeAllMembers([
{
files: 2,
isSupported: true,
Expand Down
31 changes: 24 additions & 7 deletions tests/http.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ const jsonApiError = {
links: {
about: 'https://snyk.io',
},
meta: {
isErrorCatalogError: true,
},
};

const error = new Error('uh oh');
Expand Down Expand Up @@ -79,7 +82,9 @@ describe('HTTP', () => {
});

it('should return error with detail for json api type errors on failed response', async () => {
jest.spyOn(needle, 'makeRequest').mockResolvedValue({ success: false, errorCode: 422, error, jsonApiError });
jest
.spyOn(needle, 'makeRequest')
.mockResolvedValue({ success: false, errorCode: 422, error, errors: [jsonApiError] });
const spy = jest.spyOn(httpUtils, 'generateErrorWithDetail');

await getFilters({ baseURL, source, attempts: 1, extraHeaders: {} });
Expand Down Expand Up @@ -108,7 +113,9 @@ describe('HTTP', () => {
});

it('should return error with detail for json api type errors on failed response', async () => {
jest.spyOn(needle, 'makeRequest').mockResolvedValue({ success: false, errorCode: 422, error, jsonApiError });
jest
.spyOn(needle, 'makeRequest')
.mockResolvedValue({ success: false, errorCode: 422, error, errors: [jsonApiError] });
const spy = jest.spyOn(httpUtils, 'generateErrorWithDetail');

await getAnalysis(options);
Expand Down Expand Up @@ -138,7 +145,9 @@ describe('HTTP', () => {
});

it('should return error with detail for json api type errors on failed response', async () => {
jest.spyOn(needle, 'makeRequest').mockResolvedValue({ success: false, errorCode: 422, error, jsonApiError });
jest
.spyOn(needle, 'makeRequest')
.mockResolvedValue({ success: false, errorCode: 422, error, errors: [jsonApiError] });
const spy = jest.spyOn(httpUtils, 'generateErrorWithDetail');

await createBundle(options);
Expand Down Expand Up @@ -168,7 +177,9 @@ describe('HTTP', () => {
});

it('should return error with detail for json api type errors on failed response', async () => {
jest.spyOn(needle, 'makeRequest').mockResolvedValue({ success: false, errorCode: 422, error, jsonApiError });
jest
.spyOn(needle, 'makeRequest')
.mockResolvedValue({ success: false, errorCode: 422, error, errors: [jsonApiError] });
const spy = jest.spyOn(httpUtils, 'generateErrorWithDetail');

await checkBundle(options);
Expand Down Expand Up @@ -198,7 +209,9 @@ describe('HTTP', () => {
});

it('should return error with detail for json api type errors on failed response', async () => {
jest.spyOn(needle, 'makeRequest').mockResolvedValue({ success: false, errorCode: 422, error, jsonApiError });
jest
.spyOn(needle, 'makeRequest')
.mockResolvedValue({ success: false, errorCode: 422, error, errors: [jsonApiError] });
const spy = jest.spyOn(httpUtils, 'generateErrorWithDetail');

await extendBundle(options);
Expand Down Expand Up @@ -229,7 +242,9 @@ describe('HTTP', () => {
});

it('should return error with detail for json api type errors on failed response', async () => {
jest.spyOn(needle, 'makeRequest').mockResolvedValue({ success: false, errorCode: 422, error, jsonApiError });
jest
.spyOn(needle, 'makeRequest')
.mockResolvedValue({ success: false, errorCode: 422, error, errors: [jsonApiError] });
const spy = jest.spyOn(httpUtils, 'generateErrorWithDetail');

await initReport(options);
Expand Down Expand Up @@ -257,7 +272,9 @@ describe('HTTP', () => {
});

it('should return error with detail for json api type errors on failed response', async () => {
jest.spyOn(needle, 'makeRequest').mockResolvedValue({ success: false, errorCode: 422, error, jsonApiError });
jest
.spyOn(needle, 'makeRequest')
.mockResolvedValue({ success: false, errorCode: 422, error, errors: [jsonApiError] });
const spy = jest.spyOn(httpUtils, 'generateErrorWithDetail');

await getReport(options);
Expand Down
Loading

0 comments on commit 2e5451c

Please sign in to comment.