diff --git a/.eslintrc.json b/.eslintrc.json index 191f3675..ee5214e9 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -2,12 +2,10 @@ "plugins": ["@typescript-eslint", "prettier", "import"], "extends": [ "eslint:recommended", - "airbnb-base", "plugin:@typescript-eslint/recommended", "plugin:@typescript-eslint/recommended-requiring-type-checking", "plugin:prettier/recommended", - "prettier", - "prettier/@typescript-eslint" + "prettier" ], "globals": { "Atomics": "readonly", @@ -29,14 +27,14 @@ "require-jsdoc": "off", "space-before-function-paren": "off", "comma-dangle": "off", - "object-curly-spacing": "warn", + "object-curly-spacing": ["error", "always"], "padded-blocks": "off", "camelcase": "warn", "object-property-newline": "off", "prefer-const": "warn", "import/no-absolute-path": "off", "no-prototype-builtins": "off", - "indent": "warn", + "indent": ["warn", 2], "quote-props": ["warn", "as-needed"], "lines-between-class-members": "off", "prefer-destructuring": ["warn", {"object": true, "array": false}], diff --git a/package.json b/package.json index 5e5b8dc2..8f364d55 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,6 @@ "@typescript-eslint/eslint-plugin": "^4.31.2", "@typescript-eslint/parser": "^4.0.1", "eslint": "^7.32.0", - "eslint-config-airbnb-base": "^14.2.1", "eslint-config-prettier": "^8.3.0", "eslint-plugin-import": "^2.24.2", "eslint-plugin-prettier": "^4.0.0", @@ -76,7 +75,6 @@ "multimatch": "^5.0.0", "needle": "^3.0.0", "p-map": "^3.0.0", - "queue": "^6.0.2", "uuid": "^8.3.2", "yaml": "^1.10.2" } diff --git a/src/analysis.ts b/src/analysis.ts index 88c9b83b..ce4c4f30 100644 --- a/src/analysis.ts +++ b/src/analysis.ts @@ -14,7 +14,6 @@ import { ConnectionOptions, GetAnalysisOptions, } from './http'; -import { fromEntries } from './lib/utils'; import { createBundleFromFolders, FileBundle, remoteBundleFactory } from './bundles'; import { emitter } from './emitter'; import { AnalysisResult, AnalysisResultLegacy, AnalysisResultSarif, AnalysisFiles, Suggestion } from './interfaces/analysis-result.interface'; @@ -78,12 +77,11 @@ export async function analyzeBundle(options: GetAnalysisOptions): Promise { - const filePath = resolveBundleFilePath(baseDir, path); - return [filePath, positions]; - }), - ); + return Object.entries(files).reduce((obj, [path, positions]) => { + const filePath = resolveBundleFilePath(baseDir, path); + obj[filePath] = positions; + return obj; + }, {}); } return files; } @@ -216,11 +214,11 @@ const moveSuggestionIndexes = ( suggestions: { [index: string]: T }, ): { [index: string]: T } => { const entries = Object.entries(suggestions); - return fromEntries( - entries.map(([i, s]) => { - return [`${parseInt(i, 10) + suggestionIndex + 1}`, s]; - }), - ); + + return entries.reduce((obj, [i, s]) => { + obj[`${parseInt(i, 10) + suggestionIndex + 1}`] = s; + return obj; + }, {}); }; function mergeLegacyResults( @@ -241,11 +239,10 @@ function mergeLegacyResults( const newSuggestions = moveSuggestionIndexes(suggestionIndex, newAnalysisResults.suggestions); const suggestions = { ...oldAnalysisResults.suggestions, ...newSuggestions }; - const newFiles = fromEntries( - Object.entries(newAnalysisResults.files).map(([fn, s]) => { - return [fn, moveSuggestionIndexes(suggestionIndex, s)]; - }), - ); + const newFiles = Object.entries(newAnalysisResults.files).reduce((obj, [fn, s]) => { + obj[fn] = moveSuggestionIndexes(suggestionIndex, s); + return obj; + }, {}); // expand relative file names to absolute ones only for legacy results const changedFiles = [...limitToFiles, ...removedFiles].map(path => resolveBundleFilePath(baseDir, path)); diff --git a/src/constants.ts b/src/constants.ts index 9a8fa5e5..f572b60b 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -42,6 +42,14 @@ export enum ErrorCodes { timeout = 504, } +export const NETWORK_ERRORS = { + ETIMEDOUT: ErrorCodes.timeout, + ECONNREFUSED: ErrorCodes.connectionRefused, + ECONNRESET: ErrorCodes.connectionRefused, + ENETUNREACH: ErrorCodes.connectionRefused, + ENOTFOUND: ErrorCodes.dnsNotFound, +}; + export const DEFAULT_ERROR_MESSAGES: { [P in ErrorCodes]: string } = { [ErrorCodes.serverError]: 'Unexpected server error', // 500 [ErrorCodes.badGateway]: 'Bad gateway', // 502 diff --git a/src/http.ts b/src/http.ts index 280219a0..96de369c 100644 --- a/src/http.ts +++ b/src/http.ts @@ -1,7 +1,7 @@ import { v4 as uuidv4 } from 'uuid'; import pick from 'lodash.pick'; -import { ErrorCodes, GenericErrorTypes, DEFAULT_ERROR_MESSAGES } from './constants'; +import { ErrorCodes, GenericErrorTypes, DEFAULT_ERROR_MESSAGES, MAX_RETRY_ATTEMPTS } from './constants'; import { BundleFiles, SupportedFiles } from './interfaces/files.interface'; import { AnalysisResult } from './interfaces/analysis-result.interface'; @@ -25,32 +25,6 @@ export interface ConnectionOptions { source: string; } -export function determineErrorCode(error: any): ErrorCodes { - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - const { response }: { response: any | undefined } = error; - if (response) { - return response.statusCode; - } - - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - const { errno, code }: { errno: string | undefined; code: string | number | undefined } = error; - if (errno === 'ECONNREFUSED') { - // set connectionRefused item - return ErrorCodes.connectionRefused; - } - if (errno === 'ECONNRESET') { - // set connectionRefused item - return ErrorCodes.connectionRefused; - } - - if (code === 'ENOTFOUND') { - return ErrorCodes.dnsNotFound; - } - - // We must be strict here and if none of our existing logic recognized this error, just throw it up. - throw error; -} - // The trick to typecast union type alias function isSubsetErrorCode(code: any, messages: { [c: number]: string }): code is T { if (code in messages) { @@ -59,11 +33,9 @@ function isSubsetErrorCode(code: any, messages: { [c: number]: string }): cod return false; } -function generateError(error: any, messages: { [c: number]: string }, apiName: string): ResultError { - const errorCode = determineErrorCode(error); - +function generateError(errorCode: number, messages: { [c: number]: string }, apiName: string): ResultError { if (!isSubsetErrorCode(errorCode, messages)) { - throw error; + throw { errorCode, messages, apiName }; } const statusCode = errorCode; @@ -116,17 +88,17 @@ export type IpFamily = 6 | undefined; */ export async function getIpFamily(authHost: string): Promise { const family = 6; - try { - // Dispatch a FORCED IPv6 request to test client's ISP and network capability - await makeRequest({ + + // Dispatch a FORCED IPv6 request to test client's ISP and network capability + const res = await makeRequest( + { url: `${authHost}/verify/callback`, method: 'post', family, // family param forces the handler to dispatch a request using IP at "family" version - }); - return family; - } catch (e) { - return undefined; - } + }, + 0, + ); + return res.success ? family : undefined; } type CheckSessionErrorCodes = GenericErrorTypes | ErrorCodes.unauthorizedUser | ErrorCodes.loginInProgress; @@ -153,54 +125,44 @@ export async function checkSession(options: CheckSessionOptions): Promise({ + url: `${options.authHost}/api/v1/verify/callback`, + body: { + token: options.draftToken, + }, + family: options.ipFamily, + method: 'post', + }); - if (success) { - const responseBody = response.body as IApiTokenResponse; - defaultValue.value = (responseBody.ok && responseBody.api) || ''; - } + if (res.success) { + return { ...defaultValue, value: (res.body.ok && res.body.api) || '' }; + } else if ([ErrorCodes.loginInProgress, ErrorCodes.badRequest, ErrorCodes.unauthorizedUser].includes(res.errorCode)) { return defaultValue; - } catch (err) { - if ( - [ErrorCodes.loginInProgress, ErrorCodes.badRequest, ErrorCodes.unauthorizedUser].includes( - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - err.response?.status, - ) - ) { - return defaultValue; - } - - return generateError(err, CHECK_SESSION_ERROR_MESSAGES, 'checkSession'); } + + return generateError(res.errorCode, CHECK_SESSION_ERROR_MESSAGES, 'checkSession'); } -export async function getFilters(baseURL: string, source: string): Promise> { +export async function getFilters( + baseURL: string, + source: string, + attempts = MAX_RETRY_ATTEMPTS, +): Promise> { const apiName = 'filters'; - try { - const { success, response } = await makeRequest({ + const res = await makeRequest( + { headers: { source }, url: `${baseURL}/${apiName}`, method: 'get', - }); - - if (success) { - const responseBody = response.body as SupportedFiles; - return { type: 'success', value: responseBody }; - } + }, + attempts, + ); - return generateError({ response }, GENERIC_ERROR_MESSAGES, apiName); - } catch (error) { - return generateError(error, GENERIC_ERROR_MESSAGES, 'filters'); + if (res.success) { + return { type: 'success', value: res.body }; } + return generateError(res.errorCode, GENERIC_ERROR_MESSAGES, apiName); } function prepareTokenHeaders(sessionToken: string) { @@ -240,27 +202,21 @@ interface CreateBundleOptions extends ConnectionOptions { export async function createBundle( options: CreateBundleOptions, ): Promise> { - try { - const payload: Payload = { - headers: { - ...prepareTokenHeaders(options.sessionToken), - source: options.source, - }, - url: `${options.baseURL}/bundle`, - method: 'post', - body: options.files, - }; - - const { response, success } = await makeRequest(payload); - - if (success) { - return { type: 'success', value: response.body as RemoteBundle }; - } + const payload: Payload = { + headers: { + ...prepareTokenHeaders(options.sessionToken), + source: options.source, + }, + url: `${options.baseURL}/bundle`, + method: 'post', + body: options.files, + }; - return generateError({ response }, CREATE_BUNDLE_ERROR_MESSAGES, 'createBundle'); - } catch (error) { - return generateError(error, CREATE_BUNDLE_ERROR_MESSAGES, 'createBundle'); + const res = await makeRequest(payload); + if (res.success) { + return { type: 'success', value: res.body }; } + return generateError(res.errorCode, CREATE_BUNDLE_ERROR_MESSAGES, 'createBundle'); } export type CheckBundleErrorCodes = @@ -281,22 +237,17 @@ interface CheckBundleOptions extends ConnectionOptions { } export async function checkBundle(options: CheckBundleOptions): Promise> { - try { - const { response, success } = await makeRequest({ - headers: { - ...prepareTokenHeaders(options.sessionToken), - source: options.source, - }, - url: `${options.baseURL}/bundle/${options.bundleHash}`, - method: 'get', - }); - - if (success) return { type: 'success', value: response.body as RemoteBundle }; + const res = await makeRequest({ + headers: { + ...prepareTokenHeaders(options.sessionToken), + source: options.source, + }, + url: `${options.baseURL}/bundle/${options.bundleHash}`, + method: 'get', + }); - return generateError({ response }, CHECK_BUNDLE_ERROR_MESSAGES, 'checkBundle'); - } catch (error) { - return generateError(error, CHECK_BUNDLE_ERROR_MESSAGES, 'checkBundle'); - } + if (res.success) return { type: 'success', value: res.body }; + return generateError(res.errorCode, CHECK_BUNDLE_ERROR_MESSAGES, 'checkBundle'); } export type ExtendBundleErrorCodes = @@ -325,23 +276,18 @@ interface ExtendBundleOptions extends ConnectionOptions { export async function extendBundle( options: ExtendBundleOptions, ): Promise> { - try { - const { response, success } = await makeRequest({ - headers: { - ...prepareTokenHeaders(options.sessionToken), - source: options.source, - }, - url: `${options.baseURL}/bundle/${options.bundleHash}`, - method: 'put', - body: pick(options, ['files', 'removedFiles']), - }); - - if (success) return { type: 'success', value: response.body as RemoteBundle }; + const res = await makeRequest({ + headers: { + ...prepareTokenHeaders(options.sessionToken), + source: options.source, + }, + url: `${options.baseURL}/bundle/${options.bundleHash}`, + method: 'put', + body: pick(options, ['files', 'removedFiles']), + }); - return generateError({ response }, EXTEND_BUNDLE_ERROR_MESSAGES, 'extendBundle'); - } catch (error) { - return generateError(error, EXTEND_BUNDLE_ERROR_MESSAGES, 'extendBundle'); - } + if (res.success) return { type: 'success', value: res.body }; + return generateError(res.errorCode, EXTEND_BUNDLE_ERROR_MESSAGES, 'extendBundle'); } // eslint-disable-next-line no-shadow @@ -408,18 +354,13 @@ export async function getAnalysis( type: 'file', hash: options.bundleHash, limitToFiles: options.limitToFiles || [], - ...(options.shard ? {shard: options.shard} : null), + ...(options.shard ? { shard: options.shard } : null), }, ...pick(options, ['severity', 'prioritized', 'legacy']), }, }; - try { - const { response, success } = await makeRequest(config); - if (success) return { type: 'success', value: response.body as GetAnalysisResponseDto }; - - return generateError({ response }, GET_ANALYSIS_ERROR_MESSAGES, 'getAnalysis'); - } catch (error) { - return generateError(error, GET_ANALYSIS_ERROR_MESSAGES, 'getAnalysis'); - } + const res = await makeRequest(config); + if (res.success) return { type: 'success', value: res.body }; + return generateError(res.errorCode, GET_ANALYSIS_ERROR_MESSAGES, 'getAnalysis'); } diff --git a/src/lib/utils.ts b/src/lib/utils.ts deleted file mode 100644 index 1d26d192..00000000 --- a/src/lib/utils.ts +++ /dev/null @@ -1,13 +0,0 @@ -// We are using the same types that Object.fromEntries has -// eslint-disable-next-line import/prefer-default-export, @typescript-eslint/no-explicit-any -export function fromEntries(entries: Iterable): any; -export function fromEntries( - entries: Iterable, -): { - [k in PropertyKey]: T; -} { - return [...entries].reduce((obj, [key, val]) => { - obj[key] = val; // eslint-disable-line no-param-reassign - return obj; - }, {}); -} diff --git a/src/needle.ts b/src/needle.ts index f3d25d6d..54876f6f 100644 --- a/src/needle.ts +++ b/src/needle.ts @@ -1,3 +1,4 @@ +/* eslint-disable camelcase */ import http, { OutgoingHttpHeaders } from 'http'; import needle from 'needle'; import * as querystring from 'querystring'; @@ -5,11 +6,9 @@ import https from 'https'; import { URL } from 'url'; import { emitter } from './emitter'; -import { - ErrorCodes, - MAX_RETRY_ATTEMPTS, - REQUEST_RETRY_DELAY, -} from './constants'; +import { ErrorCodes, NETWORK_ERRORS, MAX_RETRY_ATTEMPTS, REQUEST_RETRY_DELAY } from './constants'; + +const sleep = (duration: number) => new Promise(resolve => setTimeout(resolve, duration)); // Snyk CLI allow passing --insecure flag which allows self-signed certificates // It updates global namespace property ignoreUnknownCA and we can use it in order @@ -19,6 +18,8 @@ export declare interface Global extends NodeJS.Global { } declare const global: Global; +const TIMEOUT_DEFAULT = 10000; + const agentOptions = { keepAlive: true, keepAliveMsecs: 1000, @@ -37,11 +38,21 @@ export interface Payload { qs?: querystring.ParsedUrlQueryInput; timeout?: number; family?: number; - attempts?: number; - retryDelay?: number; } -export async function makeRequest(payload: Payload): Promise<{ success: boolean; response: needle.NeedleResponse }> { +interface SuccessResponse { + success: true; + body: T; +} +type FailedResponse = { + success: false; + errorCode: number; +}; + +export async function makeRequest( + payload: Payload, + attempts = MAX_RETRY_ATTEMPTS, +): Promise | FailedResponse> { const data = JSON.stringify(payload.body); const parsedUrl = new URL(payload.url); @@ -60,9 +71,9 @@ export async function makeRequest(payload: Payload): Promise<{ success: boolean; const options: needle.NeedleOptions = { headers: payload.headers, - open_timeout: 0, // No timeout - response_timeout: payload.timeout, - read_timeout: payload.timeout, + open_timeout: TIMEOUT_DEFAULT, // No timeout + response_timeout: payload.timeout || TIMEOUT_DEFAULT, + read_timeout: payload.timeout || TIMEOUT_DEFAULT, family: payload.family, json: true, compressed: true, // sets 'Accept-Encoding' to 'gzip, deflate, br' @@ -72,35 +83,40 @@ export async function makeRequest(payload: Payload): Promise<{ success: boolean; }; emitter.apiRequestLog(`=> HTTP ${method?.toUpperCase()} ${url} ${data ?? ''}`.slice(0, 399)); - let attempts = payload.attempts ?? MAX_RETRY_ATTEMPTS; - let retryDelay = payload.retryDelay ?? REQUEST_RETRY_DELAY; - let response, success; do { + let errorCode: number | undefined; + let response: needle.NeedleResponse | undefined; try { response = await needle(method, url, data, options); emitter.apiRequestLog(`<= Response: ${response.statusCode} ${JSON.stringify(response.body)}`); - success = !!(response.statusCode && response.statusCode >= 200 && response.statusCode < 300); - if (success) return { success, response }; + const success = !!(response.statusCode && response.statusCode >= 200 && response.statusCode < 300); + if (success) return { success, body: response.body as T }; + errorCode = response.statusCode; + } catch (err) { + errorCode = NETWORK_ERRORS[err.code || err.errno]; + emitter.apiRequestLog(`Requested url --> ${url} | error --> ${err}`); + } + + errorCode = errorCode ?? ErrorCodes.serviceUnavailable; - // Try to avoid breaking requests due to temporary network errors - if (attempts > 1 && response.statusCode && [ + // Try to avoid breaking requests due to temporary network errors + if ( + attempts > 1 && + [ ErrorCodes.serviceUnavailable, ErrorCodes.badGateway, ErrorCodes.connectionRefused, ErrorCodes.timeout, ErrorCodes.dnsNotFound, - ].includes(response.statusCode)) { - attempts--; - await new Promise(resolve => setTimeout(resolve, retryDelay)); - } else { - attempts = 0; - } - } catch (err) { - emitter.apiRequestLog(`Request error --> ${err}`.slice(0, 399)); - throw err; + ].includes(errorCode) + ) { + attempts--; + await sleep(REQUEST_RETRY_DELAY); + } else { + attempts = 0; + return { success: false, errorCode }; } } while (attempts > 0); - if (response) return { success, response }; - // Following line should be unreachable - throw new Error('Unknown network error'); + + return { success: false, errorCode: ErrorCodes.serviceUnavailable }; } diff --git a/tests/api.spec.ts b/tests/api.spec.ts index be429141..a056c4c3 100644 --- a/tests/api.spec.ts +++ b/tests/api.spec.ts @@ -2,7 +2,7 @@ import pick from 'lodash.pick'; import { baseURL, sessionToken, source, TEST_TIMEOUT } from './constants/base'; import { bundleFiles, bundleFilesFull } from './constants/sample'; -import { fromEntries } from '../src/lib/utils'; +import { emitter } from '../src/emitter'; import { getFilters, createBundle, checkBundle, extendBundle, getAnalysis, AnalysisStatus } from '../src/http'; import { BundleFiles } from '../src/interfaces/files.interface'; @@ -42,7 +42,7 @@ describe('Requests to public API', () => { '.erb', '.es', '.es6', - '.go', + '.go', '.h', '.haml', '.hpp', @@ -55,7 +55,7 @@ describe('Requests to public API', () => { '.php', '.phtml', '.py', - '.rb', + '.rb', '.rhtml', '.slim', '.ts', @@ -70,12 +70,25 @@ describe('Requests to public API', () => { expect(response.value.extensions.length).toBeGreaterThan(0); }); + it('test network issues', async () => { + + const response = await getFilters('https://faketest.snyk.io', 'test-source', 1); + expect(response.type).toEqual('error'); + if (response.type !== 'error') return; + expect(response.error).toMatchObject({ + apiName: 'filters', + statusCode: 452, + statusText: '[Connection issue] Could not resolve domain', + }); + }); + it( 'creates bundle successfully', async () => { - const files: BundleFiles = fromEntries( - [...(await bundleFiles).entries()].map(([i, d]) => [d.bundlePath, `${i}`]), - ); + const files: BundleFiles = [...(await bundleFiles).entries()].reduce((obj, [i, d]) => { + obj[d.bundlePath] = `${i}`; + return obj; + }, {}); const response = await createBundle({ baseURL, @@ -201,9 +214,9 @@ describe('Requests to public API', () => { expect(response.type).toEqual('error'); if (response.type !== 'error') return; expect(response.error).toEqual({ - apiName: "extendBundle", + apiName: 'extendBundle', statusCode: 404, - statusText: "Parent bundle has expired" + statusText: 'Parent bundle has expired', }); }, TEST_TIMEOUT, @@ -230,7 +243,9 @@ describe('Requests to public API', () => { TEST_TIMEOUT, ); - it('test successful workflow', async () => { + it( + 'test successful workflow', + async () => { // Create a bundle first const files: BundleFiles = (await bundleFilesFull).reduce((r, d) => { r[d.bundlePath] = pick(d, ['hash', 'content']); diff --git a/tests/utils.spec.ts b/tests/utils.spec.ts deleted file mode 100644 index 346c62ad..00000000 --- a/tests/utils.spec.ts +++ /dev/null @@ -1,60 +0,0 @@ -import { fromEntries } from '../src/lib/utils'; -describe('fromEntries', () => { - it('Object transformations', async () => { - // Arrange - const obj = { a: 1, b: 2, c: 3 }; - const expected = { a: 2, b: 4, c: 6 }; - - // Action - const fromEntriesRes = fromEntries(Object.entries(obj).map(([key, val]) => [key, val * 2])); - - // Assert - expect(fromEntriesRes).toMatchObject(expected); - }); - - it('Converting an Array to an Object', async () => { - // Arrange - const arr = [ - ['0', 'a'], - ['1', 'b'], - ['2', 'c'], - ]; - const expected = { 0: 'a', 1: 'b', 2: 'c' }; - - // Action - const fromEntriesRes = fromEntries(arr); - - // Assert - expect(fromEntriesRes).toMatchObject(expected); - }); - - it('Converting a Map to an Object', async () => { - // Arrange - const map = new Map([ - ['foo', 12], - ['baz', 42], - ]); - const expected = { foo: 12, baz: 42 }; - - // Action - const fromEntriesRes = fromEntries(map); - - // Assert - expect(fromEntriesRes).toMatchObject(expected); - }); - - it('Duplicate key', async () => { - // Arrange - const arr = [ - ['foo', 1], - ['foo', 2], - ]; - const expected = { foo: 2 }; - - // Action - const fromEntriesRes = fromEntries(arr); - - // Assert - expect(fromEntriesRes).toMatchObject(expected); - }); -});