From bd9eca7d69111a4317f14bf332614096f8f3bcbc Mon Sep 17 00:00:00 2001 From: Yukai Huang Date: Fri, 17 Feb 2023 15:47:52 +0800 Subject: [PATCH 1/2] feat: add wrapResponseErrors option --- nodejs/src/error.ts | 16 ++++++++++-- nodejs/src/index.ts | 63 ++++++++++++++++++++++++++++----------------- 2 files changed, 53 insertions(+), 26 deletions(-) diff --git a/nodejs/src/error.ts b/nodejs/src/error.ts index 565e5b9..08f3279 100644 --- a/nodejs/src/error.ts +++ b/nodejs/src/error.ts @@ -18,11 +18,23 @@ class HttpResponseError extends HackMDError { class MissingRequiredArgument extends HackMDError {} class InternalServerError extends HttpResponseError {} - +class TooManyRequestsError extends HttpResponseError { + public constructor ( + message: string, + readonly code: number, + readonly statusText: string, + readonly userLimit: number, + readonly userRemaining: number, + readonly resetAfter?: number, + ) { + super(message, code, statusText) + } +} export { HackMDError, HttpResponseError, MissingRequiredArgument, - InternalServerError + InternalServerError, + TooManyRequestsError, } diff --git a/nodejs/src/index.ts b/nodejs/src/index.ts index ec72a5c..dd0ea18 100644 --- a/nodejs/src/index.ts +++ b/nodejs/src/index.ts @@ -12,10 +12,14 @@ const defaultOption: RequestOptions = { type OptionReturnType = Opt extends { unwrapData: false } ? AxiosResponse : Opt extends { unwrapData: true } ? T : T +export type APIClientOptions = { + wrapResponseErrors: boolean +} + export class API { private axios: AxiosInstance - constructor (readonly accessToken: string, public hackmdAPIEndpointURL: string = "https://api.hackmd.io/v1") { + constructor (readonly accessToken: string, public hackmdAPIEndpointURL: string = "https://api.hackmd.io/v1", public options: APIClientOptions = { wrapResponseErrors: true }) { if (!accessToken) { throw new HackMDErrors.MissingRequiredArgument('Missing access token when creating HackMD client') } @@ -37,30 +41,41 @@ export class API { } ) - this.axios.interceptors.response.use( - (response: AxiosResponse) => { - return response - }, - async (err: AxiosError) => { - if (!err.response) { - return Promise.reject(err) - } - - if (err.response.status >= 500) { - throw new HackMDErrors.InternalServerError( - `HackMD internal error (${err.response.status} ${err.response.statusText})`, - err.response.status, - err.response.statusText, - ) - } else { - throw new HackMDErrors.HttpResponseError( - `Received an error response (${err.response.status} ${err.response.statusText}) from HackMD`, - err.response.status, - err.response.statusText, - ) + if (options.wrapResponseErrors) { + this.axios.interceptors.response.use( + (response: AxiosResponse) => { + return response + }, + async (err: AxiosError) => { + if (!err.response) { + return Promise.reject(err) + } + + if (err.response.status >= 500) { + throw new HackMDErrors.InternalServerError( + `HackMD internal error (${err.response.status} ${err.response.statusText})`, + err.response.status, + err.response.statusText, + ) + } else if (err.response.status === 429) { + throw new HackMDErrors.TooManyRequestsError( + `Too many requests (${err.response.status} ${err.response.statusText})`, + err.response.status, + err.response.statusText, + parseInt(err.response.headers['x-ratelimit-limit'], 10), + parseInt(err.response.headers['x-ratelimit-remaining'], 10), + parseInt(err.response.headers['x-ratelimit-userreset'], 10), + ) + } else { + throw new HackMDErrors.HttpResponseError( + `Received an error response (${err.response.status} ${err.response.statusText}) from HackMD`, + err.response.status, + err.response.statusText, + ) + } } - } - ) + ) + } } async getMe (options = defaultOption as Opt): Promise> { From 90ee847c8f7807472e7b8f0e861e46cd1c62c2be Mon Sep 17 00:00:00 2001 From: Yukai Huang Date: Fri, 17 Feb 2023 16:10:14 +0800 Subject: [PATCH 2/2] fix: add tests --- nodejs/src/index.ts | 4 ++-- nodejs/tests/api.spec.ts | 52 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/nodejs/src/index.ts b/nodejs/src/index.ts index dd0ea18..eef0246 100644 --- a/nodejs/src/index.ts +++ b/nodejs/src/index.ts @@ -62,8 +62,8 @@ export class API { `Too many requests (${err.response.status} ${err.response.statusText})`, err.response.status, err.response.statusText, - parseInt(err.response.headers['x-ratelimit-limit'], 10), - parseInt(err.response.headers['x-ratelimit-remaining'], 10), + parseInt(err.response.headers['x-ratelimit-userlimit'], 10), + parseInt(err.response.headers['x-ratelimit-userremaining'], 10), parseInt(err.response.headers['x-ratelimit-userreset'], 10), ) } else { diff --git a/nodejs/tests/api.spec.ts b/nodejs/tests/api.spec.ts index 5944c6b..d56a846 100644 --- a/nodejs/tests/api.spec.ts +++ b/nodejs/tests/api.spec.ts @@ -1,5 +1,7 @@ import { server } from './mock' import { API } from '../src' +import { rest } from 'msw' +import { TooManyRequestsError } from '../src/error' let client: API @@ -34,3 +36,53 @@ test('getMe unwrapped', async () => { expect(response).toHaveProperty('userPath') expect(response).toHaveProperty('photo') }) + +test('should throw axios error object if set wrapResponseErrors to false', async () => { + const customCilent = new API(process.env.HACKMD_ACCESS_TOKEN!, undefined, { + wrapResponseErrors: false, + }) + + server.use( + rest.get('https://api.hackmd.io/v1/me', (req, res, ctx) => { + return res(ctx.status(429)) + }), + ) + + try { + await customCilent.getMe() + } catch (error: any) { + expect(error).toHaveProperty('response') + expect(error.response).toHaveProperty('status', 429) + } +}) + +test.only('should throw HackMD error object', async () => { + server.use( + rest.get('https://api.hackmd.io/v1/me', (req, res, ctx) => { + return res( + ctx.status(429), + ctx.set({ + 'X-RateLimit-UserLimit': '100', + 'x-RateLimit-UserRemaining': '0', + 'x-RateLimit-UserReset': String( + new Date().getTime() + 1000 * 60 * 60 * 24, + ), + }), + ) + }), + ) + + try { + await client.getMe() + } catch (error: any) { + expect(error).toBeInstanceOf(TooManyRequestsError) + + console.log(JSON.stringify(error)) + + expect(error).toHaveProperty('code', 429) + expect(error).toHaveProperty('statusText', 'Too Many Requests') + expect(error).toHaveProperty('userLimit', 100) + expect(error).toHaveProperty('userRemaining', 0) + expect(error).toHaveProperty('resetAfter') + } +})