From 70047b27684a8cc9256f80cb7cd921ad844665e3 Mon Sep 17 00:00:00 2001 From: Will Schurman Date: Thu, 29 Sep 2022 12:26:33 -0700 Subject: [PATCH 1/4] feat: add exponential backoff and retries for push/send --- package.json | 4 ++- src/ExpoClient.ts | 43 +++++++++++++++++++++++--------- src/__tests__/ExpoClient-test.ts | 35 +++++++++++++++----------- tsconfig.json | 1 + yarn.lock | 30 ++++++++++++++++++++++ 5 files changed, 85 insertions(+), 28 deletions(-) diff --git a/package.json b/package.json index 0e0ee0d..3203f0f 100644 --- a/package.json +++ b/package.json @@ -40,11 +40,13 @@ "homepage": "https://github.com/expo/expo-server-sdk-node#readme", "dependencies": { "node-fetch": "^2.6.0", - "promise-limit": "^2.7.0" + "promise-limit": "^2.7.0", + "promise-retry": "^2.0.1" }, "devDependencies": { "@types/jest": "^27.0.1", "@types/node-fetch": "^2.5.12", + "@types/promise-retry": "^1.1.3", "eslint": "^7.32.0", "eslint-config-universe": "^7.0.1", "fetch-mock": "^9.11.0", diff --git a/src/ExpoClient.ts b/src/ExpoClient.ts index 2d07376..0bc73ad 100644 --- a/src/ExpoClient.ts +++ b/src/ExpoClient.ts @@ -4,11 +4,12 @@ * Use this if you are running Node on your server backend when you are working with Expo * https://expo.io */ -import * as assert from 'assert'; +import assert from 'assert'; import { Agent } from 'http'; import fetch, { Headers, Response as FetchResponse } from 'node-fetch'; -import * as promiseLimit from 'promise-limit'; -import * as zlib from 'zlib'; +import promiseLimit from 'promise-limit'; +import promiseRetry from 'promise-retry'; +import zlib from 'zlib'; const BASE_URL = 'https://exp.host'; const BASE_API_URL = `${BASE_URL}/--/api/v2`; @@ -74,13 +75,30 @@ export class Expo { async sendPushNotificationsAsync(messages: ExpoPushMessage[]): Promise { const actualMessagesCount = Expo._getActualMessageCount(messages); - const data = await this.requestAsync(`${BASE_API_URL}/push/send`, { - httpMethod: 'post', - body: messages, - shouldCompress(body) { - return body.length > 1024; + const data = await promiseRetry( + async (retry): Promise => { + try { + return await this.requestAsync(`${BASE_API_URL}/push/send`, { + httpMethod: 'post', + body: messages, + shouldCompress(body) { + return body.length > 1024; + }, + }); + } catch (e: any) { + // if Expo servers rate limit, retry with exponential backoff + if (e.statusCode === 429) { + return retry(e); + } + throw e; + } }, - }); + { + retries: 2, + factor: 2, + minTimeout: 1000, + } + ); if (!Array.isArray(data) || data.length !== actualMessagesCount) { const apiError: ExtensibleError = new Error( @@ -236,7 +254,7 @@ export class Expo { } if (result.errors) { - const apiError = this.getErrorFromResult(result); + const apiError = this.getErrorFromResult(response, result); throw apiError; } @@ -258,7 +276,7 @@ export class Expo { return apiError; } - return this.getErrorFromResult(result); + return this.getErrorFromResult(response, result); } private async getTextResponseErrorAsync(response: FetchResponse, text: string): Promise { @@ -274,13 +292,14 @@ export class Expo { * Returns an error for the first API error in the result, with an optional `others` field that * contains any other errors. */ - private getErrorFromResult(result: ApiResult): Error { + private getErrorFromResult(response: FetchResponse, result: ApiResult): Error { assert(result.errors && result.errors.length > 0, `Expected at least one error from Expo`); const [errorData, ...otherErrorData] = result.errors!; const error: ExtensibleError = this.getErrorFromResultError(errorData); if (otherErrorData.length) { error.others = otherErrorData.map((data) => this.getErrorFromResultError(data)); } + error.statusCode = response.status; return error; } diff --git a/src/__tests__/ExpoClient-test.ts b/src/__tests__/ExpoClient-test.ts index ccac852..76b7e8e 100644 --- a/src/__tests__/ExpoClient-test.ts +++ b/src/__tests__/ExpoClient-test.ts @@ -6,21 +6,6 @@ afterEach(() => { (fetch as any).reset(); }); -test('limits the number of concurrent requests', async () => { - (fetch as any).mock('https://exp.host/--/api/v2/push/send', { data: [] }); - expect((fetch as any).calls().length).toBe(0); - - const client = new ExpoClient({ maxConcurrentRequests: 1 }); - const sendPromise1 = client.sendPushNotificationsAsync([]); - const sendPromise2 = client.sendPushNotificationsAsync([]); - - expect((fetch as any).calls().length).toBe(1); - await sendPromise1; - expect((fetch as any).calls().length).toBe(2); - await sendPromise2; - expect((fetch as any).calls().length).toBe(2); -}); - describe('sending push notification messages', () => { test('sends requests to the Expo API server without a supplied access token', async () => { const mockTickets = [ @@ -187,6 +172,26 @@ describe('sending push notification messages', () => { others: expect.arrayContaining([expect.any(Error)]), }); }); + + test('handles 429 too many request by applying exponential backoff', async () => { + (fetch as any).mock( + 'https://exp.host/--/api/v2/push/send', + { + status: 429, + body: { + errors: [{ code: 'RATE_LIMIT_ERROR', message: `Rate limit exceeded` }], + }, + }, + { repeat: 3 } + ); + + const client = new ExpoClient(); + const rejection = expect(client.sendPushNotificationsAsync([])).rejects; + await rejection.toThrowError(`Rate limit exceeded`); + await rejection.toMatchObject({ code: 'RATE_LIMIT_ERROR' }); + + expect((fetch as any).done()).toBeTruthy(); + }, 10000); }); describe('retrieving push notification receipts', () => { diff --git a/tsconfig.json b/tsconfig.json index f86c6d6..b1c84ce 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -6,6 +6,7 @@ "sourceMap": true, "declaration": true, "types": ["jest", "node"], + "esModuleInterop": true, "outDir": "./build", "rootDir": "./src", "strict": true diff --git a/yarn.lock b/yarn.lock index efb7edd..066298a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -676,6 +676,18 @@ resolved "https://registry.yarnpkg.com/@types/prettier/-/prettier-2.3.2.tgz#fc8c2825e4ed2142473b4a81064e6e081463d1b3" integrity sha512-eI5Yrz3Qv4KPUa/nSIAi0h+qX0XyewOliug5F2QAtuRg6Kjg6jfmxe1GIwoIRhZspD1A0RP8ANrPwvEXXtRFog== +"@types/promise-retry@^1.1.3": + version "1.1.3" + resolved "https://registry.yarnpkg.com/@types/promise-retry/-/promise-retry-1.1.3.tgz#baab427419da9088a1d2f21bf56249c21b3dd43c" + integrity sha512-LxIlEpEX6frE3co3vCO2EUJfHIta1IOmhDlcAsR4GMMv9hev1iTI9VwberVGkePJAuLZs5rMucrV8CziCfuJMw== + dependencies: + "@types/retry" "*" + +"@types/retry@*": + version "0.12.2" + resolved "https://registry.yarnpkg.com/@types/retry/-/retry-0.12.2.tgz#ed279a64fa438bb69f2480eda44937912bb7480a" + integrity sha512-XISRgDJ2Tc5q4TRqvgJtzsRkFYNJzZrhTdtMoGVBttwzzQJkPnS3WWTFc7kuDRoPtPakl+T+OfdEUjYJj7Jbow== + "@types/stack-utils@^2.0.0": version "2.0.1" resolved "https://registry.yarnpkg.com/@types/stack-utils/-/stack-utils-2.0.1.tgz#20f18294f797f2209b5f65c8e3b5c8e8261d127c" @@ -1314,6 +1326,11 @@ enquirer@^2.3.5: dependencies: ansi-colors "^4.1.1" +err-code@^2.0.2: + version "2.0.3" + resolved "https://registry.yarnpkg.com/err-code/-/err-code-2.0.3.tgz#23c2f3b756ffdfc608d30e27c9a941024807e7f9" + integrity sha512-2bmlRpNKBxT/CRmPOlyISQpNj+qSeYvcym/uT0Jx2bMOlKLtSy1ZmLuVxSEKKyor/N5yhvp/ZiG1oE3DEYMSFA== + error-ex@^1.3.1: version "1.3.2" resolved "https://registry.yarnpkg.com/error-ex/-/error-ex-1.3.2.tgz#b4ac40648107fdcdcfae242f428bea8a14d4f1bf" @@ -3158,6 +3175,14 @@ promise-limit@^2.7.0: resolved "https://registry.yarnpkg.com/promise-limit/-/promise-limit-2.7.0.tgz#eb5737c33342a030eaeaecea9b3d3a93cb592b26" integrity sha512-7nJ6v5lnJsXwGprnGXga4wx6d1POjvi5Qmf1ivTRxTjH4Z/9Czja/UCMLVmB9N93GeWOU93XaFaEt6jbuoagNw== +promise-retry@^2.0.1: + version "2.0.1" + resolved "https://registry.yarnpkg.com/promise-retry/-/promise-retry-2.0.1.tgz#ff747a13620ab57ba688f5fc67855410c370da22" + integrity sha512-y+WKFlBR8BGXnsNlIHFGPZmyDf3DFMoLhaflAnyZgV6rG6xu+JwesTo2Q9R6XwYmtmwAFCkAk3e35jEdoeh/3g== + dependencies: + err-code "^2.0.2" + retry "^0.12.0" + prompts@^2.0.1: version "2.4.1" resolved "https://registry.yarnpkg.com/prompts/-/prompts-2.4.1.tgz#befd3b1195ba052f9fd2fde8a486c4e82ee77f61" @@ -3283,6 +3308,11 @@ resolve@^2.0.0-next.3: is-core-module "^2.2.0" path-parse "^1.0.6" +retry@^0.12.0: + version "0.12.0" + resolved "https://registry.yarnpkg.com/retry/-/retry-0.12.0.tgz#1b42a6266a21f07421d1b0b54b7dc167b01c013b" + integrity sha512-9LkiTwjUh6rT555DtE9rTX+BKByPfrMzEAtnlEtdEwr3Nkffwiihqe2bWADg+OQRjt9gl6ICdmB/ZFDCGAtSow== + reusify@^1.0.4: version "1.0.4" resolved "https://registry.yarnpkg.com/reusify/-/reusify-1.0.4.tgz#90da382b1e126efc02146e90845a88db12925d76" From 52b4d8802bae08512f1116a40f9f4db2b472bd6a Mon Sep 17 00:00:00 2001 From: Will Schurman Date: Mon, 3 Oct 2022 10:58:41 -0700 Subject: [PATCH 2/4] Address some comments --- src/ExpoClient.ts | 60 ++++++++++++++++---------------- src/__tests__/ExpoClient-test.ts | 36 +++++++++++++++++-- 2 files changed, 64 insertions(+), 32 deletions(-) diff --git a/src/ExpoClient.ts b/src/ExpoClient.ts index 0bc73ad..972c1ee 100644 --- a/src/ExpoClient.ts +++ b/src/ExpoClient.ts @@ -75,30 +75,32 @@ export class Expo { async sendPushNotificationsAsync(messages: ExpoPushMessage[]): Promise { const actualMessagesCount = Expo._getActualMessageCount(messages); - const data = await promiseRetry( - async (retry): Promise => { - try { - return await this.requestAsync(`${BASE_API_URL}/push/send`, { - httpMethod: 'post', - body: messages, - shouldCompress(body) { - return body.length > 1024; - }, - }); - } catch (e: any) { - // if Expo servers rate limit, retry with exponential backoff - if (e.statusCode === 429) { - return retry(e); + const data = await this.limitConcurrentRequests(async () => { + return await promiseRetry( + async (retry): Promise => { + try { + return await this.requestAsync(`${BASE_API_URL}/push/send`, { + httpMethod: 'post', + body: messages, + shouldCompress(body) { + return body.length > 1024; + }, + }); + } catch (e: any) { + // if Expo servers rate limit, retry with exponential backoff + if (e.statusCode === 429) { + return retry(e); + } + throw e; } - throw e; + }, + { + retries: 2, + factor: 2, + minTimeout: 1000, } - }, - { - retries: 2, - factor: 2, - minTimeout: 1000, - } - ); + ); + }); if (!Array.isArray(data) || data.length !== actualMessagesCount) { const apiError: ExtensibleError = new Error( @@ -229,14 +231,12 @@ export class Expo { requestHeaders.set('Content-Type', 'application/json'); } - const response = await this.limitConcurrentRequests(() => - fetch(url, { - method: options.httpMethod, - body: requestBody, - headers: requestHeaders, - agent: this.httpAgent, - }) - ); + const response = await fetch(url, { + method: options.httpMethod, + body: requestBody, + headers: requestHeaders, + agent: this.httpAgent, + }); if (response.status !== 200) { const apiError = await this.parseErrorResponseAsync(response); diff --git a/src/__tests__/ExpoClient-test.ts b/src/__tests__/ExpoClient-test.ts index 76b7e8e..7e05c32 100644 --- a/src/__tests__/ExpoClient-test.ts +++ b/src/__tests__/ExpoClient-test.ts @@ -173,7 +173,7 @@ describe('sending push notification messages', () => { }); }); - test('handles 429 too many request by applying exponential backoff', async () => { + test('handles 429 Too Many Requests by applying exponential backoff', async () => { (fetch as any).mock( 'https://exp.host/--/api/v2/push/send', { @@ -186,12 +186,44 @@ describe('sending push notification messages', () => { ); const client = new ExpoClient(); - const rejection = expect(client.sendPushNotificationsAsync([])).rejects; + const ticketPromise = client.sendPushNotificationsAsync([]); + + const rejection = expect(ticketPromise).rejects; await rejection.toThrowError(`Rate limit exceeded`); await rejection.toMatchObject({ code: 'RATE_LIMIT_ERROR' }); expect((fetch as any).done()).toBeTruthy(); }, 10000); + + test('handles 429 Too Many Requests and succeeds when a retry succeeds', async () => { + const mockTickets = [ + { status: 'ok', id: 'XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX' }, + { status: 'ok', id: 'YYYYYYYY-YYYY-YYYY-YYYY-YYYYYYYYYYYY' }, + ]; + (fetch as any) + .mock( + 'https://exp.host/--/api/v2/push/send', + { + status: 429, + body: { + errors: [{ code: 'RATE_LIMIT_ERROR', message: `Rate limit exceeded` }], + }, + }, + { repeat: 2 } + ) + .mock( + 'https://exp.host/--/api/v2/push/send', + { data: mockTickets }, + { overwriteRoutes: false } + ); + + const client = new ExpoClient(); + await expect(client.sendPushNotificationsAsync([{ to: 'a' }, { to: 'b' }])).resolves.toEqual( + mockTickets + ); + + expect((fetch as any).done()).toBeTruthy(); + }, 10000); }); describe('retrieving push notification receipts', () => { From 14c2409ba40337ec5ab5ed43cc64901bba24ca43 Mon Sep 17 00:00:00 2001 From: Will Schurman Date: Wed, 12 Oct 2022 11:54:19 -0700 Subject: [PATCH 3/4] Remove timeout --- src/__tests__/ExpoClient-test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/__tests__/ExpoClient-test.ts b/src/__tests__/ExpoClient-test.ts index 7e05c32..a895952 100644 --- a/src/__tests__/ExpoClient-test.ts +++ b/src/__tests__/ExpoClient-test.ts @@ -193,7 +193,7 @@ describe('sending push notification messages', () => { await rejection.toMatchObject({ code: 'RATE_LIMIT_ERROR' }); expect((fetch as any).done()).toBeTruthy(); - }, 10000); + }); test('handles 429 Too Many Requests and succeeds when a retry succeeds', async () => { const mockTickets = [ @@ -223,7 +223,7 @@ describe('sending push notification messages', () => { ); expect((fetch as any).done()).toBeTruthy(); - }, 10000); + }); }); describe('retrieving push notification receipts', () => { From d1dac8122fadec05a5a092361e67c09c9bda2c12 Mon Sep 17 00:00:00 2001 From: Will Schurman Date: Tue, 18 Oct 2022 09:41:46 -0700 Subject: [PATCH 4/4] Address comments --- src/ExpoClient.ts | 4 +++- src/ExpoClientValues.ts | 1 + src/__tests__/ExpoClient-test.ts | 4 ++++ 3 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 src/ExpoClientValues.ts diff --git a/src/ExpoClient.ts b/src/ExpoClient.ts index 972c1ee..3ac546e 100644 --- a/src/ExpoClient.ts +++ b/src/ExpoClient.ts @@ -11,6 +11,8 @@ import promiseLimit from 'promise-limit'; import promiseRetry from 'promise-retry'; import zlib from 'zlib'; +import { requestRetryMinTimeout } from './ExpoClientValues'; + const BASE_URL = 'https://exp.host'; const BASE_API_URL = `${BASE_URL}/--/api/v2`; @@ -97,7 +99,7 @@ export class Expo { { retries: 2, factor: 2, - minTimeout: 1000, + minTimeout: requestRetryMinTimeout, } ); }); diff --git a/src/ExpoClientValues.ts b/src/ExpoClientValues.ts new file mode 100644 index 0000000..bebbcd3 --- /dev/null +++ b/src/ExpoClientValues.ts @@ -0,0 +1 @@ +export const requestRetryMinTimeout = 1000; diff --git a/src/__tests__/ExpoClient-test.ts b/src/__tests__/ExpoClient-test.ts index a895952..de4d319 100644 --- a/src/__tests__/ExpoClient-test.ts +++ b/src/__tests__/ExpoClient-test.ts @@ -2,6 +2,10 @@ import fetch from 'node-fetch'; import ExpoClient, { ExpoPushMessage } from '../ExpoClient'; +jest.mock('../ExpoClientValues', () => ({ + requestRetryMinTimeout: 1, +})); + afterEach(() => { (fetch as any).reset(); });