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.

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

Supports [email protected] #165

Merged
merged 10 commits into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@
"clean": "yarn tsc --build --clean"
},
"devDependencies": {
"@lifeomic/eslint-config-standards": "^3.0.0",
"@lifeomic/eslint-config-standards": "^3.2.3",
"@lifeomic/jest-config": "^1.1.2",
"@lifeomic/typescript-config": "^1.0.3",
"@lifeomic/typescript-config": "^3.1.0",
"@swc/core": "^1.2.207",
"@swc/jest": "^0.2.21",
"@types/jest": "^28.1.3",
Expand Down Expand Up @@ -60,7 +60,7 @@
"@aws-sdk/signature-v4": "^3.110.0",
"@aws-sdk/url-parser": "^3.357.0",
"@types/aws-lambda": "^8.10.101",
"axios": "^0.27.2",
"axios": "^1.6.0",
"lodash": "^4.17.21",
"nearley": "2",
"url-parse": "^1.5.10",
Expand Down
4 changes: 2 additions & 2 deletions src/adapters/helpers/apiGateway.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { AxiosRequestHeaders } from 'axios';
import { AxiosRequestHeaders, AxiosHeaders } from 'axios';
import { APIGatewayProxyEvent } from 'aws-lambda';

/**
Expand All @@ -7,7 +7,7 @@ import { APIGatewayProxyEvent } from 'aws-lambda';
* https://github.com/apollographql/apollo-server/issues/5504
*/
export type ToProxyHeaders = Pick<APIGatewayProxyEvent, 'multiValueHeaders' | 'headers'>;
export const toProxyHeaders = (headers: AxiosRequestHeaders = {}): ToProxyHeaders => {
export const toProxyHeaders = (headers: AxiosRequestHeaders = new AxiosHeaders()): ToProxyHeaders => {
const response: ToProxyHeaders = {
multiValueHeaders: {},
headers: {},
Expand Down
4 changes: 2 additions & 2 deletions src/adapters/helpers/chainAdapters.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import axios, { AxiosAdapter } from 'axios';
import axios, { getAdapter } from 'axios';
import type { AlphaOptions, AlphaAdapter } from '../../types';

export type Predicate = (config: AlphaOptions) => any;
Expand All @@ -8,7 +8,7 @@ export const chainAdapters = (
predicate: Predicate,
adapter: AlphaAdapter,
) => {
const nextAdapter = config.adapter || axios.defaults.adapter as AxiosAdapter;
const nextAdapter = getAdapter(config.adapter || axios.defaults.adapter);

config.adapter = async (config) => {
if (predicate(config)) {
Expand Down
4 changes: 2 additions & 2 deletions src/adapters/helpers/requestError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import { InvocationRequest, InvocationResponse } from '@aws-sdk/client-lambda';
import { HandlerRequest, AlphaOptions } from '../../types';
import { AxiosError, AxiosResponse } from 'axios';

export const isAxiosError = (err: any | AxiosError): err is AxiosError =>
export const isAxiosError = (err: any): err is AxiosError =>
(typeof err === 'object') && !!err.isAxiosError;

export const isAlphaRequestError = (err: any | RequestError): err is RequestError =>
export const isAlphaRequestError = (err: any): err is RequestError =>
(typeof err === 'object') && !!err.isAlphaRequestError;

export class RequestError extends Error implements Omit<AxiosError, 'response' | 'toJSON' | 'isAxiosError'> {
Expand Down
2 changes: 1 addition & 1 deletion src/adapters/lambda-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const lambdaHandlerAdapter: AlphaAdapter = async (config) => {
try {
const result = await handler(request.event, request.context as Context) as Payload;
return lambdaResponse(config, request, result);
} catch (error: any | Error) {
} catch (error: any) {
throw new RequestError(error.message as string, config, request, error.response as AxiosResponse);
}
};
Expand Down
22 changes: 16 additions & 6 deletions src/adapters/response-retry.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import isBoolean from 'lodash/isBoolean';
import defaults from 'lodash/defaults';
import _get from 'lodash/get';
import _inRange from 'lodash/inRange';
import { RequestError } from './helpers/requestError';
import type { AlphaOptions } from '../types';
import type { Alpha } from '../alpha';
import { AxiosError } from 'axios';

export interface RetryOptions {
attempts: number;
Expand All @@ -17,11 +18,20 @@ export interface RetryAlphaOptions extends Omit<AlphaOptions, 'retry'> {
retry: RetryOptions;
}

const isRetryableError = (error: any | RequestError) => {
const isServerSideError = (error: RequestError) => {
return (
error.response
&& (
_inRange(_get(error.response, 'StatusCode', 0) as number, 500, 600)
Copy link
Member

Choose a reason for hiding this comment

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

Before the addition of StatusCode checks, was this function failing to detect some failures?

Copy link
Member Author

Choose a reason for hiding this comment

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

this function is refactored from isRetryableError(). it checked the statusCode of http request via Axios API (AxiosResponse) only.

These lines are for checking the status code of lambda function invocation via Lambda API (InvocationResponse). I could remove this one if aws internal error during lambda invocation is not retryable.

|| _inRange(_get(error.response, 'status', 0) as number, 500, 600)
)
);
};

const isRetryableError = (error: RequestError) => {
if (error.isLambdaInvokeTimeout) return true;

return error.code !== 'ECONNABORTED' &&
(!error.response || (error.response.status >= 500 && error.response.status <= 599));
return error.code !== 'ECONNABORTED' && isServerSideError(error);
Copy link
Member

Choose a reason for hiding this comment

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

Should this be ! isServerSideError ?

Copy link
Member

Choose a reason for hiding this comment

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

No, I don't think so. My mistake

};

const DEFAULTS = {
Expand Down Expand Up @@ -57,8 +67,8 @@ const exponentialBackoff = (config: RetryAlphaOptions) => {
export const setup = (client: Alpha) => {
client.interceptors.response.use(
undefined,
async (err: any | AxiosError) => {
if (!('config' in err && err.config.retry)) {
async (err: any) => {
if (!('config' in err && err.config?.retry)) {
return Promise.reject(err);
}

Expand Down
8 changes: 4 additions & 4 deletions src/alpha.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import pick from 'lodash/pick';
import merge from 'lodash/merge';

import axios, { Axios, AxiosAdapter, AxiosResponse } from 'axios';
import axios, { Axios, AxiosAdapter, AxiosHeaders, AxiosResponse } from 'axios';
import cloneDeep from 'lodash/cloneDeep';
import { AlphaOptions, AlphaResponse, HandlerRequest } from './types';
import { Handler } from 'aws-lambda';
Expand Down Expand Up @@ -35,7 +35,7 @@ export class Alpha extends Axios {
constructor(target: string | Handler)
constructor(target: string | Handler, config: AlphaOptions)
constructor (...[target, config]: ConstructorArgs) {
const tmpOptions: AlphaOptions = {};
const tmpOptions: AlphaOptions = { headers: new AxiosHeaders() };
if (typeof target === 'object') {
Object.assign(tmpOptions, target);
} else if (typeof target === 'string') {
Expand Down Expand Up @@ -75,12 +75,12 @@ export class Alpha extends Axios {
if (castResp.status === 301 || castResp.status === 302) {
if (maxRedirects === 0) {
const request = castResp.request as InvocationRequest | HandlerRequest;
throw new RequestError('Exceeded maximum number of redirects.', castResp.config, request, response);
throw new RequestError('Exceeded maximum number of redirects.', castResp.config, request, castResp);
}

const redirect = cloneDeep(config);
redirect.maxRedirects = maxRedirects - 1;
redirect.url = resolve(castResp.headers.location, castResp.config.url as string);
redirect.url = resolve(castResp.headers.location as string, castResp.config.url);
return this.request(redirect);
}

Expand Down
6 changes: 5 additions & 1 deletion src/resolve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,15 @@
import { isAbsoluteURL, LambdaUrl, parseLambdaUrl } from './utils/url';
import { URL } from 'url';

export const resolve = (url: string, base: string) => {
export const resolve = (url: string, base?: string) => {
shawnzhu marked this conversation as resolved.
Show resolved Hide resolved
if (isAbsoluteURL(url)) {
return url;
}

if (!base) {
return url;
}

let lambdaParts = parseLambdaUrl(base);

if (!lambdaParts) {
Expand Down
4 changes: 2 additions & 2 deletions src/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { AxiosPromise, AxiosRequestConfig, AxiosResponse } from 'axios';
import type { AxiosPromise, InternalAxiosRequestConfig, AxiosResponse } from 'axios';
import type { Lambda } from '@aws-sdk/client-lambda';
import type { Context, Handler } from 'aws-lambda';
import { SignatureV4CryptoInit, SignatureV4Init } from '@aws-sdk/signature-v4';
Expand All @@ -21,7 +21,7 @@ export type SignAwsV4Config =
& Omit<SignatureV4Constructor, SignatureV4Optionals>
& Partial<Pick<SignatureV4Constructor, SignatureV4Optionals>>;

export interface AlphaOptions<D = any> extends AxiosRequestConfig<D> {
export interface AlphaOptions<D = any> extends InternalAxiosRequestConfig<D> {
retry?: RetryOptions | boolean;
lambda?: Handler;
context?: Context;
Expand Down
13 changes: 4 additions & 9 deletions test/adapters/helpers/apiGateway.test.ts
Original file line number Diff line number Diff line change
@@ -1,35 +1,30 @@
import { ToProxyHeaders, toProxyHeaders } from '../../../src/adapters/helpers/apiGateway';
import { v4 as uuid } from 'uuid';
import { AxiosRequestHeaders } from 'axios';
import { AxiosRequestHeaders, AxiosHeaders } from 'axios';

// TODO support the use case that request header value could be undefined/null
test('will convert header values', () => {
expect(toProxyHeaders()).toEqual({ multiValueHeaders: {}, headers: {} });
const multiStringHeader = [uuid(), ` ${uuid()}`, `${uuid()} `, uuid()];
const input: AxiosRequestHeaders = {
const input: AxiosRequestHeaders = new AxiosHeaders({
stringHeader: uuid(),
multiStringHeader: multiStringHeader.join(','),
// @ts-ignore
numberHeader: 123456,
// @ts-ignore
booleanHeader: true,
// @ts-ignore
shawnzhu marked this conversation as resolved.
Show resolved Hide resolved
undefinedHeader: undefined,
};
});

const expected: ToProxyHeaders = {
headers: {
stringHeader: input.stringHeader as string,
multiStringHeader: multiStringHeader.join(','),
numberHeader: `${input.numberHeader}`,
booleanHeader: `${input.booleanHeader}`,
undefinedHeader: undefined,
},
multiValueHeaders: {
stringHeader: [input.stringHeader as string],
multiStringHeader: multiStringHeader.map((v) => v.trim()),
numberHeader: [`${input.numberHeader}`],
booleanHeader: [`${input.booleanHeader}`],
undefinedHeader: undefined,
},
};
const actual = toProxyHeaders(input);
Expand Down
16 changes: 9 additions & 7 deletions test/adapters/helpers/chainAdapters.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
import axios from 'axios';
import axios, { AxiosHeaders, getAdapter } from 'axios';
import { chainAdapters } from '../../../src/adapters/helpers/chainAdapters';
import { AlphaAdapter } from '../../../src';

test('will set default adapter', () => {
test('will set default adapter', async () => {
const adapter: AlphaAdapter = jest.fn();
const defaultAdapter = axios.defaults.adapter = jest.fn();
const config = chainAdapters(
{},
{ headers: new AxiosHeaders() },
() => false,
adapter,
);
expect(defaultAdapter).not.toBeCalled();
expect(() => config.adapter!(config)).not.toThrow();
expect(defaultAdapter).toBeCalled();
expect(adapter).not.toBeCalled();
expect(defaultAdapter).not.toHaveBeenCalled();

expect(config.adapter).toHaveLength(1); // an array of adapters
await expect(getAdapter(config.adapter)(config)).resolves.toBeUndefined();
expect(defaultAdapter).toHaveBeenCalled();
expect(adapter).not.toHaveBeenCalled();
});
19 changes: 10 additions & 9 deletions test/alpha.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { AxiosHeaders } from 'axios';
import { Handler } from 'aws-lambda';
import { v4 as uuid } from 'uuid';

import { Alpha, AlphaOptions } from '../src';

const handler: jest.MockedFn<Handler> = jest.fn();
const config: AlphaOptions = {};
const config: AlphaOptions = { headers: new AxiosHeaders() };
shawnzhu marked this conversation as resolved.
Show resolved Hide resolved

const response = {
headers: { 'test-header': 'some value' },
Expand All @@ -31,27 +32,27 @@ test('constructor types', () => {
test('overloaded method signatures', async () => {
const path = `/${uuid()}`;
handler.mockResolvedValue(response);
const options: AlphaOptions = {};
const options: AlphaOptions = config;
const data = {};
const alpha = new Alpha(handler);
await expect(alpha.get(path, options)).resolves.toEqual(expect.objectContaining(expected));
expect(handler).toBeCalledWith(expect.objectContaining({ path, httpMethod: 'GET' }), expect.any(Object), expect.any(Function));
expect(handler).toHaveBeenCalledWith(expect.objectContaining({ path, httpMethod: 'GET' }), expect.any(Object), expect.any(Function));

await expect(alpha.delete(path, options)).resolves.toEqual(expect.objectContaining(expected));
expect(handler).toBeCalledWith(expect.objectContaining({ path, httpMethod: 'DELETE' }), expect.any(Object), expect.any(Function));
expect(handler).toHaveBeenCalledWith(expect.objectContaining({ path, httpMethod: 'DELETE' }), expect.any(Object), expect.any(Function));

await expect(alpha.head(path, options)).resolves.toEqual(expect.objectContaining(expected));
expect(handler).toBeCalledWith(expect.objectContaining({ path, httpMethod: 'HEAD' }), expect.any(Object), expect.any(Function));
expect(handler).toHaveBeenCalledWith(expect.objectContaining({ path, httpMethod: 'HEAD' }), expect.any(Object), expect.any(Function));

await expect(alpha.options(path, options)).resolves.toEqual(expect.objectContaining(expected));
expect(handler).toBeCalledWith(expect.objectContaining({ path, httpMethod: 'OPTIONS' }), expect.any(Object), expect.any(Function));
expect(handler).toHaveBeenCalledWith(expect.objectContaining({ path, httpMethod: 'OPTIONS' }), expect.any(Object), expect.any(Function));

await expect(alpha.post(path, data, options)).resolves.toEqual(expect.objectContaining(expected));
expect(handler).toBeCalledWith(expect.objectContaining({ path, httpMethod: 'POST' }), expect.any(Object), expect.any(Function));
expect(handler).toHaveBeenCalledWith(expect.objectContaining({ path, httpMethod: 'POST' }), expect.any(Object), expect.any(Function));

await expect(alpha.put(path, data, options)).resolves.toEqual(expect.objectContaining(expected));
expect(handler).toBeCalledWith(expect.objectContaining({ path, httpMethod: 'PUT' }), expect.any(Object), expect.any(Function));
expect(handler).toHaveBeenCalledWith(expect.objectContaining({ path, httpMethod: 'PUT' }), expect.any(Object), expect.any(Function));

await expect(alpha.patch(path, data, options)).resolves.toEqual(expect.objectContaining(expected));
expect(handler).toBeCalledWith(expect.objectContaining({ path, httpMethod: 'PATCH' }), expect.any(Object), expect.any(Function));
expect(handler).toHaveBeenCalledWith(expect.objectContaining({ path, httpMethod: 'PATCH' }), expect.any(Object), expect.any(Function));
});
23 changes: 11 additions & 12 deletions test/config.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Alpha } from '../src';
import { Axios } from 'axios';
import { Alpha, type AlphaOptions } from '../src';
import { Axios, AxiosHeaders } from 'axios';
import nock from 'nock';

beforeAll(() => {
Expand All @@ -14,6 +14,12 @@ afterEach(() => {
nock.cleanAll();
});

const defaultOptions: AlphaOptions = {
headers: new AxiosHeaders({
'Accept': 'application/json',
}),
};

test('Creating a client with no options creates a standard Axios client', async () => {
const server = nock('http://localhost')
.get('/some/path')
Expand Down Expand Up @@ -52,9 +58,7 @@ test('Creating a client with configuration options sets the default client optio
params: {
foo: 'bar',
},
headers: {
'Accept': 'application/json',
},
...defaultOptions,
};

const client = new Alpha(options);
Expand All @@ -73,13 +77,7 @@ test('Creating a client with a target and configuration options binds the client
.matchHeader('Accept', 'application/json')
.reply(200, { message: 'hello!' });

const options = {
headers: {
'Accept': 'application/json',
},
};

const client = new Alpha('http://example.com', options);
const client = new Alpha('http://example.com', defaultOptions);
expect(client instanceof Axios).toBe(true);

const response = await client.get('/some/path');
Expand All @@ -95,6 +93,7 @@ test('A custom status validator can be used with the client', async () => {
.reply(302, { location: '/redirect' });

const options = {
...defaultOptions,
validateStatus: (status: number) => status >= 200 && status < 300,
};

Expand Down
Loading
Loading