Skip to content

Commit

Permalink
refactor(rest): switch api to fetch-like and provide strategies (#9416)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: NodeJS v18+ is required when using node due to the use of global `fetch`
BREAKING CHANGE: The raw method of REST now returns a web compatible `Respone` object.
BREAKING CHANGE: The `parseResponse` utility method has been updated to operate on a web compatible `Response` object.
BREAKING CHANGE: Many underlying internals have changed, some of which were exported.
BREAKING CHANGE: `DefaultRestOptions` used to contain a default `agent`, which is now set to `null` instead.
ckohen authored May 6, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent fc5b9c5 commit cdaa0a3
Showing 28 changed files with 315 additions and 201 deletions.
2 changes: 1 addition & 1 deletion packages/core/README.md
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions packages/core/package.json
Original file line number Diff line number Diff line change
@@ -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",
@@ -78,7 +78,7 @@
"vitest": "^0.31.0"
},
"engines": {
"node": ">=16.9.0"
"node": ">=18.12.0"
},
"publishConfig": {
"access": "public"
2 changes: 1 addition & 1 deletion packages/discord.js/package.json
Original file line number Diff line number Diff line change
@@ -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",
4 changes: 2 additions & 2 deletions packages/proxy-container/package.json
Original file line number Diff line number Diff line change
@@ -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",
@@ -60,7 +60,7 @@
"typescript": "^5.0.4"
},
"engines": {
"node": ">=16.9.0"
"node": ">=18.12.0"
},
"publishConfig": {
"access": "public"
2 changes: 1 addition & 1 deletion packages/proxy/README.md
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions packages/proxy/package.json
Original file line number Diff line number Diff line change
@@ -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",
@@ -79,7 +79,7 @@
"vitest": "^0.31.0"
},
"engines": {
"node": ">=16.9.0"
"node": ">=18.12.0"
},
"publishConfig": {
"access": "public"
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);
}
}

/**
21 changes: 20 additions & 1 deletion packages/rest/README.md
Original file line number Diff line number Diff line change
@@ -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
@@ -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])
3 changes: 2 additions & 1 deletion packages/rest/__tests__/BurstHandler.test.ts
Original file line number Diff line number Diff line change
@@ -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';
@@ -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);
});
42 changes: 32 additions & 10 deletions packages/rest/__tests__/REST.test.ts
Original file line number Diff line number Diff line change
@@ -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';

@@ -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 = {
@@ -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({
@@ -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
@@ -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
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';
@@ -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 () => {
@@ -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 () => {
@@ -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]() {
@@ -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
@@ -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",
@@ -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",
@@ -80,7 +87,7 @@
"vitest": "^0.31.0"
},
"engines": {
"node": ">=16.9.0"
"node": ">=18.12.0"
},
"publishConfig": {
"access": "public"
Loading

0 comments on commit cdaa0a3

Please sign in to comment.