Skip to content
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.

Sign up for GitHub

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

feat: add error handling for limit errors #210

Merged
merged 5 commits into from
Nov 10, 2023
Merged

feat: add error handling for limit errors #210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
89ee52f
feat: add error handling for json api errors
nick-watson Nov 7, 2023
a40c069
fix: refactor types and error handling
nick-watson Nov 9, 2023
7bdc8aa
fix: handle detail edge cases
nick-watson Nov 9, 2023
1003a9d
fix: handle empty array
nick-watson Nov 9, 2023
d8b5afd
feat: add handling for filter and report apis
nick-watson Nov 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 77 additions & 23 deletions src/http.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,29 @@ import pick from 'lodash.pick';
import { gzip } from 'zlib';
import { promisify } from 'util';

import { ErrorCodes, GenericErrorTypes, DEFAULT_ERROR_MESSAGES, MAX_RETRY_ATTEMPTS } from './constants';
import { DEFAULT_ERROR_MESSAGES, ErrorCodes, GenericErrorTypes, MAX_RETRY_ATTEMPTS } from './constants';

import { BundleFiles, SupportedFiles } from './interfaces/files.interface';
import { AnalysisResult, ReportResult } from './interfaces/analysis-result.interface';
import { FailedResponse, makeRequest, Payload } from './needle';
import {
AnalysisOptions,
AnalysisContext,
AnalysisOptions,
ReportOptions,
ScmReportOptions,
} from './interfaces/analysis-options.interface';
import { getURL } from './utils/httpUtils';
import { generateErrorWithDetail, getURL } from './utils/httpUtils';
import { JsonApiError } from './interfaces/json-api';

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

export type ResultError<E> = {
type: 'error';
error: {
statusCode: E;
statusText: string;
apiName: string;
detail?: string | undefined;
novalex marked this conversation as resolved.
Show resolved Hide resolved
};
};

Expand All @@ -41,24 +44,26 @@ export interface ConnectionOptions {
// The trick to typecast union type alias
// eslint-disable-next-line @typescript-eslint/no-explicit-any
function isSubsetErrorCode<T>(code: any, messages: { [c: number]: string }): code is T {
if (code in messages) {
return true;
}
return false;
return code in messages;
}

function generateError<E>(
errorCode: number,
messages: { [c: number]: string },
apiName: string,
error?: string,
errorMessage?: string,
jsonApiError?: JsonApiError,
): ResultError<E> {
if (jsonApiError) {
return generateErrorWithDetail<E>(jsonApiError, errorCode, apiName);
}

if (!isSubsetErrorCode<E>(errorCode, messages)) {
throw { errorCode, messages, apiName };
throw { statusCode: errorCode, statusText: errorMessage || 'unknown error occurred', apiName };
novalex marked this conversation as resolved.
Show resolved Hide resolved
}

const statusCode = errorCode;
const statusText = error ?? messages[errorCode];
const statusText = errorMessage ?? messages[errorCode];

return {
type: 'error',
Expand Down Expand Up @@ -92,8 +97,7 @@ interface StartSessionOptions {
export async function compressAndEncode(payload: unknown): Promise<Buffer> {
// encode payload and compress;
const deflate = promisify(gzip);
const compressedPayload = await deflate(Buffer.from(JSON.stringify(payload)).toString('base64'));
return compressedPayload;
return await deflate(Buffer.from(JSON.stringify(payload)).toString('base64'));
}

export function startSession(options: StartSessionOptions): StartSessionResponseDto {
Expand Down Expand Up @@ -204,7 +208,7 @@ export async function getFilters(
if (res.success) {
return { type: 'success', value: res.body };
}
return generateError<GenericErrorTypes>(res.errorCode, GENERIC_ERROR_MESSAGES, apiName);
return generateError<GenericErrorTypes>(res.errorCode, GENERIC_ERROR_MESSAGES, apiName, undefined, res.jsonApiError);
}

function commonHttpHeaders(options: ConnectionOptions) {
Expand Down Expand Up @@ -271,7 +275,13 @@ export async function createBundle(
if (res.success) {
return { type: 'success', value: res.body };
}
return generateError<CreateBundleErrorCodes>(res.errorCode, CREATE_BUNDLE_ERROR_MESSAGES, 'createBundle');
return generateError<CreateBundleErrorCodes>(
res.errorCode,
CREATE_BUNDLE_ERROR_MESSAGES,
'createBundle',
undefined,
res.jsonApiError,
);
}

export type CheckBundleErrorCodes =
Expand Down Expand Up @@ -309,7 +319,13 @@ export async function checkBundle(options: CheckBundleOptions): Promise<Result<R
});

if (res.success) return { type: 'success', value: res.body };
return generateError<CheckBundleErrorCodes>(res.errorCode, CHECK_BUNDLE_ERROR_MESSAGES, 'checkBundle');
return generateError<CheckBundleErrorCodes>(
res.errorCode,
CHECK_BUNDLE_ERROR_MESSAGES,
'checkBundle',
undefined,
res.jsonApiError,
);
}

export type ExtendBundleErrorCodes =
Expand Down Expand Up @@ -359,7 +375,13 @@ export async function extendBundle(
isJson: false,
});
if (res.success) return { type: 'success', value: res.body };
return generateError<ExtendBundleErrorCodes>(res.errorCode, EXTEND_BUNDLE_ERROR_MESSAGES, 'extendBundle');
return generateError<ExtendBundleErrorCodes>(
res.errorCode,
EXTEND_BUNDLE_ERROR_MESSAGES,
'extendBundle',
undefined,
res.jsonApiError,
);
}

// eslint-disable-next-line no-shadow
Expand Down Expand Up @@ -431,8 +453,16 @@ export async function getAnalysis(
};

const res = await makeRequest<GetAnalysisResponseDto>(config);
if (res.success) return { type: 'success', value: res.body };
return generateError<GetAnalysisErrorCodes>(res.errorCode, GET_ANALYSIS_ERROR_MESSAGES, 'getAnalysis');
if (res.success) {
return { type: 'success', value: res.body };
}
return generateError<GetAnalysisErrorCodes>(
res.errorCode,
GET_ANALYSIS_ERROR_MESSAGES,
'getAnalysis',
undefined,
res.jsonApiError,
);
}

export type ReportErrorCodes =
Expand Down Expand Up @@ -508,7 +538,13 @@ 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');
return generateError<ReportErrorCodes>(
res.errorCode,
REPORT_ERROR_MESSAGES,
'initReport',
undefined,
res.jsonApiError,
);
}

/**
Expand All @@ -533,7 +569,13 @@ export async function getReport(options: GetReportOptions): Promise<Result<Uploa

const res = await makeRequest<UploadReportResponseDto>(config);
if (res.success) return { type: 'success', value: res.body };
return generateError<ReportErrorCodes>(res.errorCode, REPORT_ERROR_MESSAGES, 'getReport', res.error?.message);
return generateError<ReportErrorCodes>(
res.errorCode,
REPORT_ERROR_MESSAGES,
'getReport',
res.error?.message,
res.jsonApiError,
);
}

/**
Expand Down Expand Up @@ -564,7 +606,13 @@ 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');
return generateError<ReportErrorCodes>(
res.errorCode,
REPORT_ERROR_MESSAGES,
'initReport',
undefined,
res.jsonApiError,
);
}

/**
Expand All @@ -591,5 +639,11 @@ export async function getScmReport(

const res = await makeRequest<UploadReportResponseDto>(config);
if (res.success) return { type: 'success', value: res.body };
return generateError<ReportErrorCodes>(res.errorCode, REPORT_ERROR_MESSAGES, 'getReport', res.error?.message);
return generateError<ReportErrorCodes>(
res.errorCode,
REPORT_ERROR_MESSAGES,
'getReport',
res.error?.message,
res.jsonApiError,
);
}
9 changes: 9 additions & 0 deletions src/interfaces/json-api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export type JsonApiError = {
links?: {
about?: string;
};
status: string;
code: string;
title: string;
detail: string;
};
15 changes: 14 additions & 1 deletion src/needle.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import { URL } from 'url';
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 @@ -45,10 +47,12 @@ interface SuccessResponse<T> {
success: true;
body: T;
}

export type FailedResponse = {
success: false;
errorCode: number;
error: Error | undefined;
jsonApiError?: JsonApiError | undefined;
};

export async function makeRequest<T = void>(
Expand Down Expand Up @@ -94,7 +98,9 @@ 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 {
response = await needle(method, url, data, options);
emitter.apiRequestLog(`<= Response: ${response.statusCode} ${JSON.stringify(response.body)}`);
Expand All @@ -109,6 +115,13 @@ 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];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This behaviour (only handling first error) is not documented anywhere.
I don't see cases where we have more than one error from the API TBH, but know the JSON API requires arrays.
If someone were to change the API to actually return more than one error they will probably not realise the client can only handle one though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I guess this is a limitation right now, where would you think we can document it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The good thing is if we have more than 1 error, nothing breaks. They will be ignored.

Perhaps a single comment from the service that throws the error?

}

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

Expand Down
39 changes: 39 additions & 0 deletions src/utils/httpUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { ORG_ID_REGEXP } from '../constants';
import { JsonApiError } from '../interfaces/json-api';
import { ResultError } from '../http';

export function getURL(baseURL: string, path: string, orgId?: string): string {
if (routeToGateway(baseURL)) {
Expand All @@ -17,3 +19,40 @@ function routeToGateway(baseURL: string): boolean {
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> {
const errorLink = error.links?.about;
const detail = `${error.title}${error.detail ? `: ${error.detail}` : ''}${
errorLink ? ` (more info: ${errorLink})` : ``
}`;
const statusText = error.title;
return {
type: 'error',
error: {
apiName,
statusCode: statusCode as unknown as E,
statusText,
detail,
},
};
}
Loading