Skip to content

Commit

Permalink
fix: do not expect specific error for clients without IPv6 (#120)
Browse files Browse the repository at this point in the history
  • Loading branch information
michelkaporin authored Jan 17, 2022
1 parent 9b436fa commit a4ffee2
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 5 deletions.
1 change: 0 additions & 1 deletion src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ export const NETWORK_ERRORS = {
ECONNRESET: ErrorCodes.connectionRefused,
ENETUNREACH: ErrorCodes.connectionRefused,
ENOTFOUND: ErrorCodes.dnsNotFound,
EADDRNOTAVAIL: ErrorCodes.dnsNotFound, // happens when DNS cannot resolve the IPv6
};

export const DEFAULT_ERROR_MESSAGES: { [P in ErrorCodes]: string } = {
Expand Down
2 changes: 1 addition & 1 deletion src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export async function getIpFamily(authHost: string): Promise<IpFamily> {
0,
);

const ipv6Incompatible = (<FailedResponse>res).errorCode === ErrorCodes.dnsNotFound;
const ipv6Incompatible = (<FailedResponse>res).error;

return ipv6Incompatible ? undefined : family;
}
Expand Down
8 changes: 6 additions & 2 deletions src/needle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ interface SuccessResponse<T> {
export type FailedResponse = {
success: false;
errorCode: number;
error: Error | undefined;
};

export async function makeRequest<T = void>(
Expand Down Expand Up @@ -83,8 +84,10 @@ export async function makeRequest<T = void>(
};

emitter.apiRequestLog(`=> HTTP ${method?.toUpperCase()} ${url} ${data ?? ''}`.slice(0, 399));

do {
let errorCode: number | undefined;
let error: Error | undefined;
let response: needle.NeedleResponse | undefined;
try {
response = await needle(method, url, data, options);
Expand All @@ -93,6 +96,7 @@ export async function makeRequest<T = void>(
if (success) return { success, body: response.body as T };
errorCode = response.statusCode;
} catch (err) {
error = err; // do not swallow the error, pass further to the caller instead
errorCode = NETWORK_ERRORS[err.code || err.errno];
emitter.apiRequestLog(`Requested url --> ${url} | error --> ${err}`);
}
Expand All @@ -114,9 +118,9 @@ export async function makeRequest<T = void>(
await sleep(REQUEST_RETRY_DELAY);
} else {
attempts = 0;
return { success: false, errorCode };
return { success: false, errorCode, error };
}
} while (attempts > 0);

return { success: false, errorCode: ErrorCodes.serviceUnavailable };
return { success: false, errorCode: ErrorCodes.serviceUnavailable, error: undefined };
}
2 changes: 1 addition & 1 deletion tests/analysis.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ describe('Functional test of analysis', () => {

const sarifResults = extendedBundle.analysisResults.sarif;

expect(sarifResults.runs[0].tool.driver.rules).toHaveLength(7);
expect(sarifResults.runs[0].tool.driver.rules).toHaveLength(8);
expect(sarifResults.runs[0].results).toHaveLength(15);
const getRes = (path: string) =>
sarifResults.runs[0].results!.find(
Expand Down
33 changes: 33 additions & 0 deletions tests/http.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import needle from 'needle';

describe('HTTP', () => {
const authHost = 'https://dev.snyk.io';

beforeEach(() => {
jest.resetModuleRegistry();
})

it('should respolve IPv6, if http request succeeds', async () => {
jest.mock('needle', () => jest.fn(() => ({
response: {
statusCode: 401,
body: {}
}
})));

const http = await import('../src');
const family = await http.getIpFamily(authHost);

expect(family).toBe(6);
});

it('shouldn not resolve IPv6, if http request throws an error', async () => {
const errorFn = () => { throw new Error(); };
jest.mock('needle', () => jest.fn(errorFn));

const http = await import('../src');
const family = await http.getIpFamily(authHost);

expect(family).toBe(undefined);
});
});

0 comments on commit a4ffee2

Please sign in to comment.