From 2bb5441ad765002d51a1a676ff8d23253505e026 Mon Sep 17 00:00:00 2001 From: ckohen Date: Sun, 10 Oct 2021 23:04:45 -0700 Subject: [PATCH 1/2] feat(Errors): show data sent when an error occurs Ported from https://github.com/discordjs/discord.js/pull/5701 Co-authored-by: Vlad Frangu --- .../rest/__tests__/DiscordAPIError.test.ts | 16 ++++++++++ packages/rest/src/lib/RequestManager.ts | 2 +- .../rest/src/lib/errors/DiscordAPIError.ts | 12 +++++++ packages/rest/src/lib/errors/HTTPError.ts | 8 +++++ packages/rest/src/lib/handlers/IHandler.ts | 9 ++++-- .../src/lib/handlers/SequentialHandler.ts | 31 +++++++++++++------ 6 files changed, 66 insertions(+), 12 deletions(-) diff --git a/packages/rest/__tests__/DiscordAPIError.test.ts b/packages/rest/__tests__/DiscordAPIError.test.ts index f297820..07da9ec 100644 --- a/packages/rest/__tests__/DiscordAPIError.test.ts +++ b/packages/rest/__tests__/DiscordAPIError.test.ts @@ -7,6 +7,10 @@ test('Unauthorized', () => { 401, 'PATCH', 'https://discord.com/api/v9/guilds/:id', + { + attachments: undefined, + body: undefined, + }, ); expect(error.code).toBe(0); @@ -15,6 +19,8 @@ test('Unauthorized', () => { expect(error.name).toBe('DiscordAPIError[0]'); expect(error.status).toBe(401); expect(error.url).toBe('https://discord.com/api/v9/guilds/:id'); + expect(error.requestBody.attachments).toBe(undefined); + expect(error.requestBody.json).toBe(undefined); }); test('Invalid Form Body Error (error.{property}._errors.{index})', () => { @@ -30,6 +36,12 @@ test('Invalid Form Body Error (error.{property}._errors.{index})', () => { 400, 'PATCH', 'https://discord.com/api/v9/users/@me', + { + attachments: undefined, + body: { + username: 'a', + }, + }, ); expect(error.code).toBe(50035); @@ -40,6 +52,8 @@ test('Invalid Form Body Error (error.{property}._errors.{index})', () => { expect(error.name).toBe('DiscordAPIError[50035]'); expect(error.status).toBe(400); expect(error.url).toBe('https://discord.com/api/v9/users/@me'); + expect(error.requestBody.attachments).toBe(undefined); + expect(error.requestBody.json).toStrictEqual({ username: 'a' }); }); test('Invalid FormFields Error (error.errors.{property}.{property}.{index}.{property}._errors.{index})', () => { @@ -57,6 +71,7 @@ test('Invalid FormFields Error (error.errors.{property}.{property}.{index}.{prop 400, 'POST', 'https://discord.com/api/v9/channels/:id', + {}, ); expect(error.code).toBe(50035); @@ -84,6 +99,7 @@ test('Invalid FormFields Error (error.errors.{property}.{property}._errors.{inde 400, 'PATCH', 'https://discord.com/api/v9/guilds/:id', + {}, ); expect(error.code).toBe(50035); diff --git a/packages/rest/src/lib/RequestManager.ts b/packages/rest/src/lib/RequestManager.ts index a8697b5..bc6c67e 100644 --- a/packages/rest/src/lib/RequestManager.ts +++ b/packages/rest/src/lib/RequestManager.ts @@ -163,7 +163,7 @@ export class RequestManager extends EventEmitter { const { url, fetchOptions } = this.resolveRequest(request); // Queue the request - return handler.queueRequest(routeId, url, fetchOptions); + return handler.queueRequest(routeId, url, fetchOptions, { body: request.body, attachments: request.attachments }); } /** diff --git a/packages/rest/src/lib/errors/DiscordAPIError.ts b/packages/rest/src/lib/errors/DiscordAPIError.ts index 7d0ab6a..48df0cf 100644 --- a/packages/rest/src/lib/errors/DiscordAPIError.ts +++ b/packages/rest/src/lib/errors/DiscordAPIError.ts @@ -1,3 +1,5 @@ +import type { InternalRequest, RawAttachment } from '../RequestManager'; + interface DiscordErrorFieldInformation { code: string; message: string; @@ -15,6 +17,11 @@ export interface DiscordErrorData { errors?: DiscordError; } +export interface RequestBody { + attachments: RawAttachment[] | undefined; + json: unknown | undefined; +} + function isErrorGroupWrapper(error: any): error is DiscordErrorGroupWrapper { return Reflect.has(error, '_errors'); } @@ -28,12 +35,14 @@ function isErrorResponse(error: any): error is DiscordErrorFieldInformation { * @extends Error */ export class DiscordAPIError extends Error { + public requestBody: RequestBody; /** * @param rawError The error reported by Discord * @param code The error code reported by Discord * @param status The status code of the response * @param method The method of the request that erred * @param url The url of the request that erred + * @param bodyData The unparsed data for the request that erred */ public constructor( public rawError: DiscordErrorData, @@ -41,8 +50,11 @@ export class DiscordAPIError extends Error { public status: number, public method: string, public url: string, + bodyData: Pick, ) { super(DiscordAPIError.getMessage(rawError)); + + this.requestBody = { attachments: bodyData.attachments, json: bodyData.body }; } /** diff --git a/packages/rest/src/lib/errors/HTTPError.ts b/packages/rest/src/lib/errors/HTTPError.ts index 578e908..dc5e1bc 100644 --- a/packages/rest/src/lib/errors/HTTPError.ts +++ b/packages/rest/src/lib/errors/HTTPError.ts @@ -1,13 +1,18 @@ +import type { InternalRequest } from '../RequestManager'; +import type { RequestBody } from './DiscordAPIError'; + /** * Represents a HTTP error */ export class HTTPError extends Error { + public requestBody: RequestBody; /** * @param message The error message * @param name The name of the error * @param status The status code of the response * @param method The method of the request that erred * @param url The url of the request that erred + * @param bodyData The unparsed data for the request that erred */ public constructor( message: string, @@ -15,7 +20,10 @@ export class HTTPError extends Error { public status: number, public method: string, public url: string, + bodyData: Pick, ) { super(message); + + this.requestBody = { attachments: bodyData.attachments, json: bodyData.body }; } } diff --git a/packages/rest/src/lib/handlers/IHandler.ts b/packages/rest/src/lib/handlers/IHandler.ts index 89a0deb..cd04871 100644 --- a/packages/rest/src/lib/handlers/IHandler.ts +++ b/packages/rest/src/lib/handlers/IHandler.ts @@ -1,6 +1,11 @@ import type { RequestInit } from 'node-fetch'; -import type { RouteData } from '../RequestManager'; +import type { InternalRequest, RouteData } from '../RequestManager'; export interface IHandler { - queueRequest(routeId: RouteData, url: string, options: RequestInit): Promise; + queueRequest( + routeId: RouteData, + url: string, + options: RequestInit, + bodyData: Pick, + ): Promise; } diff --git a/packages/rest/src/lib/handlers/SequentialHandler.ts b/packages/rest/src/lib/handlers/SequentialHandler.ts index 7890d87..bd4e665 100644 --- a/packages/rest/src/lib/handlers/SequentialHandler.ts +++ b/packages/rest/src/lib/handlers/SequentialHandler.ts @@ -4,7 +4,7 @@ import AbortController from 'abort-controller'; import fetch, { RequestInit, Response } from 'node-fetch'; import { DiscordAPIError, DiscordErrorData } from '../errors/DiscordAPIError'; import { HTTPError } from '../errors/HTTPError'; -import type { RequestManager, RouteData } from '../RequestManager'; +import type { InternalRequest, RequestManager, RouteData } from '../RequestManager'; import { RESTEvents } from '../utils/constants'; import { parseResponse } from '../utils/utils'; @@ -85,8 +85,14 @@ export class SequentialHandler { * @param routeId The generalized api route with literal ids for major parameters * @param url The url to do the request on * @param options All the information needed to make a request + * @param bodyData The data taht was used to form the body, passed to any errors generated */ - public async queueRequest(routeId: RouteData, url: string, options: RequestInit): Promise { + public async queueRequest( + routeId: RouteData, + url: string, + options: RequestInit, + bodyData: Pick, + ): Promise { // Wait for any previous requests to be completed before this one is run await this.#asyncQueue.wait(); try { @@ -108,7 +114,7 @@ export class SequentialHandler { await sleep(this.timeToReset); } // Make the request, and return the results - return await this.runRequest(routeId, url, options); + return await this.runRequest(routeId, url, options, bodyData); } finally { // Allow the next request to fire this.#asyncQueue.shift(); @@ -120,9 +126,16 @@ export class SequentialHandler { * @param routeId The generalized api route with literal ids for major parameters * @param url The fully resolved url to make the request to * @param options The node-fetch options needed to make the request + * @param bodyData The data that was used to form the body, passed to any errors generated * @param retries The number of retries this request has already attempted (recursion) */ - private async runRequest(routeId: RouteData, url: string, options: RequestInit, retries = 0): Promise { + private async runRequest( + routeId: RouteData, + url: string, + options: RequestInit, + bodyData: Pick, + retries = 0, + ): Promise { const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), this.manager.options.timeout); let res: Response; @@ -132,7 +145,7 @@ export class SequentialHandler { } catch (error: unknown) { // Retry the specified number of times for possible timed out requests if (error instanceof Error && error.name === 'AbortError' && retries !== this.manager.options.retries) { - return this.runRequest(routeId, url, options, ++retries); + return this.runRequest(routeId, url, options, bodyData, ++retries); } throw error; @@ -193,14 +206,14 @@ export class SequentialHandler { // Wait the retryAfter amount of time before retrying the request await sleep(retryAfter); // Since this is not a server side issue, the next request should pass, so we don't bump the retries counter - return this.runRequest(routeId, url, options, retries); + return this.runRequest(routeId, url, options, bodyData, retries); } else if (res.status >= 500 && res.status < 600) { // Retry the specified number of times for possible server side issues if (retries !== this.manager.options.retries) { - return this.runRequest(routeId, url, options, ++retries); + return this.runRequest(routeId, url, options, bodyData, ++retries); } // We are out of retries, throw an error - throw new HTTPError(res.statusText, res.constructor.name, res.status, method, url); + throw new HTTPError(res.statusText, res.constructor.name, res.status, method, url, bodyData); } else { // Handle possible malformed requests if (res.status >= 400 && res.status < 500) { @@ -211,7 +224,7 @@ export class SequentialHandler { // The request will not succeed for some reason, parse the error returned from the api const data = (await parseResponse(res)) as DiscordErrorData; // throw the API error - throw new DiscordAPIError(data, data.code, res.status, method, url); + throw new DiscordAPIError(data, data.code, res.status, method, url, bodyData); } return null; } From b77830d26a271f970618dfc66f7cd77445e945b8 Mon Sep 17 00:00:00 2001 From: Vlad Frangu Date: Mon, 18 Oct 2021 16:49:41 +0300 Subject: [PATCH 2/2] Apply suggestions from code review --- packages/rest/src/lib/errors/DiscordAPIError.ts | 3 ++- packages/rest/src/lib/errors/HTTPError.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/rest/src/lib/errors/DiscordAPIError.ts b/packages/rest/src/lib/errors/DiscordAPIError.ts index 48df0cf..4a8487e 100644 --- a/packages/rest/src/lib/errors/DiscordAPIError.ts +++ b/packages/rest/src/lib/errors/DiscordAPIError.ts @@ -36,13 +36,14 @@ function isErrorResponse(error: any): error is DiscordErrorFieldInformation { */ export class DiscordAPIError extends Error { public requestBody: RequestBody; + /** * @param rawError The error reported by Discord * @param code The error code reported by Discord * @param status The status code of the response * @param method The method of the request that erred * @param url The url of the request that erred - * @param bodyData The unparsed data for the request that erred + * @param bodyData The unparsed data for the request that errored */ public constructor( public rawError: DiscordErrorData, diff --git a/packages/rest/src/lib/errors/HTTPError.ts b/packages/rest/src/lib/errors/HTTPError.ts index dc5e1bc..825d012 100644 --- a/packages/rest/src/lib/errors/HTTPError.ts +++ b/packages/rest/src/lib/errors/HTTPError.ts @@ -6,13 +6,14 @@ import type { RequestBody } from './DiscordAPIError'; */ export class HTTPError extends Error { public requestBody: RequestBody; + /** * @param message The error message * @param name The name of the error * @param status The status code of the response * @param method The method of the request that erred * @param url The url of the request that erred - * @param bodyData The unparsed data for the request that erred + * @param bodyData The unparsed data for the request that errored */ public constructor( message: string,