Skip to content

fix(javascript): ensure requesters work as in v4 #823

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

Merged
merged 2 commits into from
Jul 21, 2022
Merged
Show file tree
Hide file tree
Changes from all 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 .github/workflows/check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,9 @@ jobs:
if: ${{ steps.cache.outputs.cache-hit != 'true' }}
run: cd clients/algoliasearch-client-javascript && yarn build ${{ matrix.client }}

- name: Run tests for 'client-common'
if: ${{ steps.cache.outputs.cache-hit != 'true' && matrix.client == 'client-common' }}
run: cd clients/algoliasearch-client-javascript && yarn workspace @algolia/client-common test
- name: Run tests for '${{ matrix.client }}'
if: ${{ steps.cache.outputs.cache-hit != 'true' }}
run: cd clients/algoliasearch-client-javascript && yarn workspace @algolia/${{ matrix.client }} test

- name: Store '${{ matrix.client }}' JavaScript utils package
uses: actions/upload-artifact@v3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export function createTransporter({
return stackFrame;
};

const response = await requester.send(payload, request);
const response = await requester.send(payload);

if (isRetryable(response)) {
const stackFrame = pushToStackTrace(response);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
import type { Headers, QueryParameters } from './Transporter';

/**
* The method of the request.
*/
export type Method = 'DELETE' | 'GET' | 'PATCH' | 'POST' | 'PUT';

export type Request = {
method: Method;
/**
* The path of the REST API to send the request to.
*/
path: string;
queryParameters: QueryParameters;
data?: Array<Record<string, any>> | Record<string, any>;
Expand All @@ -20,26 +26,42 @@ export type Request = {
useReadTransporter?: boolean;
};

export type EndRequest = {
method: Method;
export type EndRequest = Pick<Request, 'headers' | 'method'> & {
/**
* The full URL of the REST API.
*/
url: string;
/**
* The connection timeout, in milliseconds.
*/
connectTimeout: number;
/**
* The response timeout, in milliseconds.
*/
responseTimeout: number;
headers: Headers;
data?: string;
};

export type Response = {
/**
* The body of the response.
*/
content: string;
/**
* Whether the API call is timed out or not.
*/
isTimedOut: boolean;
/**
* The HTTP status code of the response.
*/
status: number;
};

export type Requester = {
/**
* Sends the given `request` to the server.
*/
send: (request: EndRequest, originalRequest: Request) => Promise<Response>;
send: (request: EndRequest) => Promise<Response>;
};

export type EchoResponse = Omit<EndRequest, 'data'> &
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export type RequestOptions = Pick<Request, 'cacheable'> & {
queryParameters?: QueryParameters;

/**
* Custom data for the request. This data are
* Custom data for the request. This data is
* going to be merged the transporter data.
*/
data?: Array<Record<string, any>> | Record<string, any>;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import type { Config } from '@jest/types';

const config: Config.InitialOptions = {
preset: 'ts-jest',
roots: ['src/__tests__'],
testEnvironment: 'jsdom',
};

export default config;
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,20 @@
"index.ts"
],
"scripts": {
"build": "tsc",
"clean": "rm -rf dist/"
"clean": "rm -rf dist/",
"test": "jest"
},
"dependencies": {
"@algolia/client-common": "5.0.0-alpha.0"
},
"devDependencies": {
"@types/jest": "28.1.4",
"@types/node": "16.11.45",
"typescript": "4.7.4"
"jest": "28.1.2",
"jest-environment-jsdom": "28.1.2",
"ts-jest": "28.0.5",
"typescript": "4.7.4",
"xhr-mock": "2.5.1"
},
"engines": {
"node": ">= 14.0.0"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,261 @@
import http from 'http';

import type { EndRequest, Headers } from '@algolia/client-common';
import type { MockRequest, MockResponse } from 'xhr-mock';
import mock from 'xhr-mock';

import { createXhrRequester } from '../..';

const requester = createXhrRequester();
const BASE_URL = 'https://algolia-dns.net/foo?x-algolia-header=bar';

function getStringifiedBody(
body: Record<string, any> = { foo: 'bar' }
): string {
return JSON.stringify(body);
}

const headers: Headers = {
'content-type': 'text/plain',
};

const timeoutRequest: EndRequest = {
url: 'missing-url-here',
data: '',
headers: {},
method: 'GET',
responseTimeout: 2000,
connectTimeout: 1000,
};

const requestStub: EndRequest = {
url: BASE_URL,
method: 'POST',
headers,
data: getStringifiedBody(),
responseTimeout: 1000,
connectTimeout: 2000,
};

describe('status code handling', () => {
beforeEach(() => mock.setup());
afterEach(() => mock.teardown());

it('sends requests', async () => {
mock.post(BASE_URL, (req: MockRequest, res: MockResponse): MockResponse => {
expect(req.method()).toEqual('POST');
expect(req.header('content-type')).toEqual('text/plain');
expect(req.body()).toEqual(JSON.stringify({ foo: 'bar' }));

return res.status(200);
});

await requester.send(requestStub);
});

it('resolves status 200', async () => {
const body = getStringifiedBody();

mock.post(BASE_URL, {
status: 200,
body: requestStub.data,
});

const response = await requester.send(requestStub);

expect(response.status).toBe(200);
expect(response.content).toBe(body);
expect(response.isTimedOut).toBe(false);
});

it('resolves status 300', async () => {
const reason = 'Multiple Choices';

mock.post(BASE_URL, {
status: 300,
reason,
});

const response = await requester.send(requestStub);

expect(response.status).toBe(300);
expect(response.content).toBe(''); // No body returned here on xhr
expect(response.isTimedOut).toBe(false);
});

it('resolves status 400', async () => {
const body = getStringifiedBody({
message: 'Invalid Application-Id or API-Key',
});

mock.post(BASE_URL, {
status: 400,
body,
});

const response = await requester.send(requestStub);

expect(response.status).toBe(400);
expect(response.content).toBe(body);
expect(response.isTimedOut).toBe(false);
});

it('handles the protocol', async () => {
const body = getStringifiedBody();

mock.post('http://localhost/', {
status: 200,
body,
});

const response = await requester.send({
...requestStub,
url: 'http://localhost',
});

expect(response.status).toBe(200);
expect(response.content).toBe(body);
expect(response.isTimedOut).toBe(false);
});
});

describe('timeout handling', () => {
let server: http.Server;
// setup http server to test timeout
beforeAll(() => {
server = http.createServer(function (_req, res) {
res.writeHead(200, {
'content-type': 'text/plain',
'access-control-allow-origin': '*',
'x-powered-by': 'nodejs',
});

res.write('{"foo":');

setTimeout(() => {
res.write(' "bar"');
}, 1000);

setTimeout(() => {
res.write('}');
res.end();
}, 5000);
});

server.listen('1111');
});

afterAll((done) => {
server.close(() => done());
});

it('connection timeouts with the given 1 seconds connection timeout', async () => {
const before = Date.now();
const response = await requester.send({
...timeoutRequest,
connectTimeout: 1000,
url: 'http://www.google.com:81',
});

const now = Date.now();

expect(response.content).toBe('Connection timeout');
expect(now - before).toBeGreaterThan(999);
expect(now - before).toBeLessThan(1200);
});

it('connection timeouts with the given 2 seconds connection timeout', async () => {
const before = Date.now();
const response = await requester.send({
...timeoutRequest,
connectTimeout: 2000,
url: 'http://www.google.com:81',
});

const now = Date.now();

expect(response.content).toBe('Connection timeout');
expect(now - before).toBeGreaterThan(1990);
expect(now - before).toBeLessThan(2200);
});

it("socket timeouts if response don't appears before the timeout with 2 seconds timeout", async () => {
const before = Date.now();

const response = await requester.send({
...timeoutRequest,
responseTimeout: 2000,
url: 'http://localhost:1111',
});

const now = Date.now();

expect(response.content).toBe('Socket timeout');
expect(now - before).toBeGreaterThan(1990);
expect(now - before).toBeLessThan(2200);
});

it("socket timeouts if response don't appears before the timeout with 3 seconds timeout", async () => {
const before = Date.now();

const response = await requester.send({
...timeoutRequest,
responseTimeout: 3000,
url: 'http://localhost:1111',
});

const now = Date.now();

expect(response.content).toBe('Socket timeout');
expect(now - before).toBeGreaterThan(2999);
expect(now - before).toBeLessThan(3200);
});

it('do not timeouts if response appears before the timeout', async () => {
const before = Date.now();
const response = await requester.send({
...requestStub,
responseTimeout: 6000,
url: 'http://localhost:1111',
});

const now = Date.now();

expect(response.isTimedOut).toBe(false);
expect(response.status).toBe(200);
expect(response.content).toBe('{"foo": "bar"}');
expect(now - before).toBeGreaterThan(4999);
expect(now - before).toBeLessThan(5200);
}, 10000); // This is a long-running test, default server timeout is set to 5000ms
});

describe('error handling', () => {
it('resolves dns not found', async () => {
const request: EndRequest = {
url: 'https://this-dont-exist.algolia.com',
method: 'POST',
headers,
data: getStringifiedBody(),
responseTimeout: 2000,
connectTimeout: 1000,
};

const response = await requester.send(request);

expect(response.status).toBe(0);
expect(response.content).toBe('Network request failed');
expect(response.isTimedOut).toBe(false);
});

it('resolves general network errors', async () => {
mock.post(BASE_URL, () =>
Promise.reject(new Error('This is a general error'))
);

const response = await requester.send(requestStub);

expect(response.status).toBe(0);
expect(response.content).toBe('Network request failed');
expect(response.isTimedOut).toBe(false);
});
});
Loading