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

refactor(rest): switch api to fetch-like and provide strategies #9416

Merged
merged 1 commit into from
May 6, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

## Installation

**Node.js 16.9.0 or newer is required.**
**Node.js 18.12.0 or newer is required.**

```sh
npm install @discordjs/core
Expand Down
4 changes: 2 additions & 2 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
"devDependencies": {
"@favware/cliff-jumper": "^2.0.0",
"@microsoft/api-extractor": "^7.34.8",
"@types/node": "16.18.25",
"@types/node": "18.15.11",
"@vitest/coverage-c8": "^0.31.0",
"cross-env": "^7.0.3",
"esbuild-plugin-version-injector": "^1.1.0",
Expand All @@ -78,7 +78,7 @@
"vitest": "^0.31.0"
},
"engines": {
"node": ">=16.9.0"
"node": ">=18.12.0"
},
"publishConfig": {
"access": "public"
Expand Down
2 changes: 1 addition & 1 deletion packages/discord.js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
"@discordjs/builders": "workspace:^",
"@discordjs/collection": "workspace:^",
"@discordjs/formatters": "workspace:^",
"@discordjs/rest": "workspace:^",
"@discordjs/rest": "^1.7.1",
"@discordjs/util": "workspace:^",
"@discordjs/ws": "workspace:^",
"@sapphire/snowflake": "^3.4.2",
Expand Down
4 changes: 2 additions & 2 deletions packages/proxy-container/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
"tslib": "^2.5.0"
},
"devDependencies": {
"@types/node": "16.18.25",
"@types/node": "18.15.11",
"cross-env": "^7.0.3",
"eslint": "^8.39.0",
"eslint-config-neon": "^0.1.46",
Expand All @@ -60,7 +60,7 @@
"typescript": "^5.0.4"
},
"engines": {
"node": ">=16.9.0"
"node": ">=18.12.0"
},
"publishConfig": {
"access": "public"
Expand Down
2 changes: 1 addition & 1 deletion packages/proxy/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

## Installation

**Node.js 16.9.0 or newer is required.**
**Node.js 18.12.0 or newer is required.**

```sh
npm install @discordjs/proxy
Expand Down
4 changes: 2 additions & 2 deletions packages/proxy/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
"devDependencies": {
"@favware/cliff-jumper": "^2.0.0",
"@microsoft/api-extractor": "^7.34.8",
"@types/node": "16.18.25",
"@types/node": "18.15.11",
"@types/supertest": "^2.0.12",
"@vitest/coverage-c8": "^0.31.0",
"cross-env": "^7.0.3",
Expand All @@ -79,7 +79,7 @@
"vitest": "^0.31.0"
},
"engines": {
"node": ">=16.9.0"
"node": ">=18.12.0"
},
"publishConfig": {
"access": "public"
Expand Down
18 changes: 10 additions & 8 deletions packages/proxy/src/util/responseHelpers.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,29 @@
import type { ServerResponse } from 'node:http';
import { Readable } from 'node:stream';
import { pipeline } from 'node:stream/promises';
import { DiscordAPIError, HTTPError, RateLimitError } from '@discordjs/rest';
import type { Dispatcher } from 'undici';
import { DiscordAPIError, HTTPError, RateLimitError, type ResponseLike } from '@discordjs/rest';

/**
* Populates a server response with the data from a Discord 2xx REST response
*
* @param res - The server response to populate
* @param data - The data to populate the response with
*/
export async function populateSuccessfulResponse(res: ServerResponse, data: Dispatcher.ResponseData): Promise<void> {
res.statusCode = data.statusCode;
export async function populateSuccessfulResponse(res: ServerResponse, data: ResponseLike): Promise<void> {
res.statusCode = data.status;

for (const header of Object.keys(data.headers)) {
for (const [header, value] of data.headers) {
// Strip ratelimit headers
if (header.startsWith('x-ratelimit')) {
if (/^x-ratelimit/i.test(header)) {
continue;
}

res.setHeader(header, data.headers[header]!);
res.setHeader(header, value);
}

await pipeline(data.body, res);
if (data.body) {
await pipeline(data.body instanceof Readable ? data.body : Readable.fromWeb(data.body), res);
}
}

/**
Expand Down
21 changes: 20 additions & 1 deletion packages/rest/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

## Installation

**Node.js 16.9.0 or newer is required.**
**Node.js 18.12.0 or newer is required.**

```sh
npm install @discordjs/rest
Expand Down Expand Up @@ -80,6 +80,25 @@ try {
}
```

Send a basic message in an edge environment:

```js
import { REST } from '@discordjs/rest';
import { Routes } from 'discord-api-types/v10';

const rest = new REST({ version: '10', makeRequest: fetch }).setToken(TOKEN);

try {
await rest.post(Routes.channelMessages(CHANNEL_ID), {
body: {
content: 'A message via REST from the edge!',
},
});
} catch (error) {
console.error(error);
}
```

## Links

- [Website][website] ([source][website-source])
Expand Down
3 changes: 2 additions & 1 deletion packages/rest/__tests__/BurstHandler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import { performance } from 'node:perf_hooks';
import { MockAgent, setGlobalDispatcher } from 'undici';
import type { Interceptable, MockInterceptor } from 'undici/types/mock-interceptor';
import { beforeEach, afterEach, test, expect, vitest } from 'vitest';
import { beforeEach, afterEach, test, expect } from 'vitest';
import { DiscordAPIError, REST, BurstHandlerMajorIdKey } from '../src/index.js';
import { BurstHandler } from '../src/lib/handlers/BurstHandler.js';
import { genPath } from './util.js';
Expand Down Expand Up @@ -46,6 +46,7 @@ test('Interaction callback creates burst handler', async () => {
auth: false,
body: { type: 4, data: { content: 'Reply' } },
}),
// TODO: This should be ArrayBuffer, there is a bug in undici request
).toBeInstanceOf(Uint8Array);
expect(api.requestManager.handlers.get(callbackKey)).toBeInstanceOf(BurstHandler);
});
Expand Down
42 changes: 32 additions & 10 deletions packages/rest/__tests__/REST.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ import { URLSearchParams } from 'node:url';
import { DiscordSnowflake } from '@sapphire/snowflake';
import type { Snowflake } from 'discord-api-types/v10';
import { Routes } from 'discord-api-types/v10';
import type { FormData } from 'undici';
import { type FormData, fetch } from 'undici';
import { File as UndiciFile, MockAgent, setGlobalDispatcher } from 'undici';
import type { Interceptable, MockInterceptor } from 'undici/types/mock-interceptor.js';
import { beforeEach, afterEach, test, expect } from 'vitest';
import { beforeEach, afterEach, test, expect, vitest } from 'vitest';
import { REST } from '../src/index.js';
import { genPath } from './util.js';

Expand All @@ -16,6 +16,10 @@ const newSnowflake: Snowflake = DiscordSnowflake.generate().toString();

const api = new REST().setToken('A-Very-Fake-Token');

const makeRequestMock = vitest.fn(fetch);

const fetchApi = new REST({ makeRequest: makeRequestMock }).setToken('A-Very-Fake-Token');

// @discordjs/rest uses the `content-type` header to detect whether to parse
// the response as JSON or as an ArrayBuffer.
const responseOptions: MockInterceptor.MockResponseOptions = {
Expand Down Expand Up @@ -114,6 +118,22 @@ test('simple POST', async () => {
expect(await api.post('/simplePost')).toStrictEqual({ test: true });
});

test('simple POST with fetch', async () => {
mockPool
.intercept({
path: genPath('/fetchSimplePost'),
method: 'POST',
})
.reply(() => ({
data: { test: true },
statusCode: 200,
responseOptions,
}));

expect(await fetchApi.post('/fetchSimplePost')).toStrictEqual({ test: true });
expect(makeRequestMock).toHaveBeenCalledTimes(1);
});

test('simple PUT 2', async () => {
mockPool
.intercept({
Expand Down Expand Up @@ -159,11 +179,11 @@ test('getAuth', async () => {
path: genPath('/getAuth'),
method: 'GET',
})
.reply((from) => ({
data: { auth: (from.headers as unknown as Record<string, string | undefined>).Authorization ?? null },
statusCode: 200,
.reply(
200,
(from) => ({ auth: (from.headers as unknown as Record<string, string | undefined>).Authorization ?? null }),
responseOptions,
}))
)
.times(3);

// default
Expand All @@ -190,11 +210,13 @@ test('getReason', async () => {
path: genPath('/getReason'),
method: 'GET',
})
.reply((from) => ({
data: { reason: (from.headers as unknown as Record<string, string | undefined>)['X-Audit-Log-Reason'] ?? null },
statusCode: 200,
.reply(
200,
(from) => ({
reason: (from.headers as unknown as Record<string, string | undefined>)['X-Audit-Log-Reason'] ?? null,
}),
responseOptions,
}))
)
.times(3);

// default
Expand Down
8 changes: 4 additions & 4 deletions packages/rest/__tests__/RequestHandler.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable id-length */
/* eslint-disable promise/prefer-await-to-then */
import { performance } from 'node:perf_hooks';
import { setInterval, clearInterval, setTimeout } from 'node:timers';
import { setInterval, clearInterval } from 'node:timers';
import { MockAgent, setGlobalDispatcher } from 'undici';
import type { Interceptable, MockInterceptor } from 'undici/types/mock-interceptor.js';
import { beforeEach, afterEach, test, expect, vitest } from 'vitest';
Expand Down Expand Up @@ -492,7 +492,7 @@ test('server responding too slow', async () => {

const promise = api2.get('/slow');

await expect(promise).rejects.toThrowError('Request aborted');
await expect(promise).rejects.toThrowError('aborted');
}, 1_000);

test('Unauthorized', async () => {
Expand Down Expand Up @@ -570,8 +570,8 @@ test('abort', async () => {
controller.abort();

// Abort mid-execution:
await expect(bP2).rejects.toThrowError('Request aborted');
await expect(bP2).rejects.toThrowError('aborted');

// Abort scheduled:
await expect(cP2).rejects.toThrowError('Request aborted');
await expect(cP2).rejects.toThrowError('Request aborted manually');
});
Original file line number Diff line number Diff line change
@@ -1,22 +1,37 @@
import { Blob, Buffer } from 'node:buffer';
import { URLSearchParams } from 'node:url';
import { test, expect } from 'vitest';
import { resolveBody, parseHeader } from '../src/lib/utils/utils.js';
import { MockAgent, setGlobalDispatcher } from 'undici';
import type { Interceptable, MockInterceptor } from 'undici/types/mock-interceptor.js';
import { beforeEach, afterEach, test, expect, vitest } from 'vitest';
import { REST } from '../src/index.js';
import { makeRequest, resolveBody } from '../src/strategies/undiciRequest.js';
import { genPath } from './util.js';

test('GIVEN string parseHeader returns string', () => {
const header = 'application/json';
const makeRequestMock = vitest.fn(makeRequest);

expect(parseHeader(header)).toEqual(header);
});
const api = new REST({ makeRequest: makeRequestMock }).setToken('A-Very-Fake-Token');

// @discordjs/rest uses the `content-type` header to detect whether to parse
// the response as JSON or as an ArrayBuffer.
const responseOptions: MockInterceptor.MockResponseOptions = {
headers: {
'content-type': 'application/json',
},
};

let mockAgent: MockAgent;
let mockPool: Interceptable;

test('GIVEN string[] parseHeader returns string', () => {
const header = ['application/json', 'wait sorry I meant text/html'];
beforeEach(() => {
mockAgent = new MockAgent();
mockAgent.disableNetConnect(); // prevent actual requests to Discord
setGlobalDispatcher(mockAgent); // enabled the mock client to intercept requests

expect(parseHeader(header)).toEqual(header.join(';'));
mockPool = mockAgent.get('https://discord.com');
});

test('GIVEN undefined parseHeader return undefined', () => {
expect(parseHeader(undefined)).toBeUndefined();
afterEach(async () => {
await mockAgent.close();
});

test('resolveBody', async () => {
Expand All @@ -43,7 +58,7 @@ test('resolveBody', async () => {
}
},
};
await expect(resolveBody(iterable)).resolves.toStrictEqual(new Uint8Array([1, 2, 3, 1, 2, 3, 1, 2, 3]));
await expect(resolveBody(iterable)).resolves.toStrictEqual(Buffer.from([1, 2, 3, 1, 2, 3, 1, 2, 3]));

const asyncIterable: AsyncIterable<Uint8Array> = {
[Symbol.asyncIterator]() {
Expand All @@ -66,3 +81,19 @@ test('resolveBody', async () => {
// @ts-expect-error: This test is ensuring that this throws
await expect(resolveBody(true)).rejects.toThrow(TypeError);
});

test('use passed undici request', async () => {
mockPool
.intercept({
path: genPath('/simplePost'),
method: 'POST',
})
.reply(() => ({
data: { test: true },
statusCode: 200,
responseOptions,
}));

expect(await api.post('/simplePost')).toStrictEqual({ test: true });
expect(makeRequestMock).toHaveBeenCalledTimes(1);
});
17 changes: 12 additions & 5 deletions packages/rest/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,16 @@
"module": "./dist/index.mjs",
"typings": "./dist/index.d.ts",
"exports": {
"types": "./dist/index.d.ts",
"import": "./dist/index.mjs",
"require": "./dist/index.js"
".": {
"types": "./dist/index.d.ts",
"import": "./dist/index.mjs",
"require": "./dist/index.js"
},
"./*": {
"types": "./dist/strategies/*.d.ts",
"import": "./dist/strategies/*.mjs",
"require": "./dist/strategies/*.js"
}
},
"directories": {
"lib": "src",
Expand Down Expand Up @@ -66,7 +73,7 @@
"devDependencies": {
"@favware/cliff-jumper": "^2.0.0",
"@microsoft/api-extractor": "^7.34.8",
"@types/node": "16.18.25",
"@types/node": "18.15.11",
"@vitest/coverage-c8": "^0.31.0",
"cross-env": "^7.0.3",
"esbuild-plugin-version-injector": "^1.1.0",
Expand All @@ -80,7 +87,7 @@
"vitest": "^0.31.0"
},
"engines": {
"node": ">=16.9.0"
"node": ">=18.12.0"
},
"publishConfig": {
"access": "public"
Expand Down