From 9dcc17b68783e7a8cd190f361bd5e9d0cad549f8 Mon Sep 17 00:00:00 2001 From: Brendan Chen Date: Wed, 30 Apr 2025 17:33:54 -0700 Subject: [PATCH 1/5] add test to check that connection is reused --- .../senders/AppleNotificationSenderTests.test.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/test/notifications/senders/AppleNotificationSenderTests.test.ts b/test/notifications/senders/AppleNotificationSenderTests.test.ts index 948be7d..0ccf20d 100644 --- a/test/notifications/senders/AppleNotificationSenderTests.test.ts +++ b/test/notifications/senders/AppleNotificationSenderTests.test.ts @@ -91,7 +91,7 @@ describe("AppleNotificationSender", () => { }); describe("sendNotificationImmediately", () => { - it('makes the connection to the http server if the notification should be sent', async () => { + it('makes the connection to the http server if sending a notification for the first time', async () => { const notificationArguments: NotificationAlertArguments = { title: 'Test notification', body: 'This notification will send', @@ -103,6 +103,20 @@ describe("AppleNotificationSender", () => { expect(result).toBe(true); }); + it('reuses the existing connection if sending another notification', async () => { + const notificationArguments: NotificationAlertArguments = { + title: 'Test notification', + body: 'This notification will send', + } + + const result1 = await notificationSender.sendNotificationImmediately('1', notificationArguments); + const result2 = await notificationSender.sendNotificationImmediately('1', notificationArguments); + + expect(http2.connect).toHaveBeenCalledTimes(1); + expect(result1).toBe(true); + expect(result2).toBe(true); + }); + it('throws an error if the bundle ID is not set correctly', async () => { process.env = { ...process.env, From 075b35e7ad72f41f0507d39d591c2d11354bbde5 Mon Sep 17 00:00:00 2001 From: Brendan Chen Date: Wed, 30 Apr 2025 17:40:17 -0700 Subject: [PATCH 2/5] reuse the client when sending a notification --- src/notifications/senders/AppleNotificationSender.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/notifications/senders/AppleNotificationSender.ts b/src/notifications/senders/AppleNotificationSender.ts index 4aac769..61c0b73 100644 --- a/src/notifications/senders/AppleNotificationSender.ts +++ b/src/notifications/senders/AppleNotificationSender.ts @@ -1,5 +1,6 @@ import jwt from "jsonwebtoken"; import http2 from "http2"; +import { ClientHttp2Session } from "node:http2"; interface APNsUrl { fullUrl: string; @@ -17,6 +18,8 @@ export class AppleNotificationSender { private apnsToken: string | undefined = undefined; private _lastRefreshedTimeMs: number | undefined = undefined; + private client: ClientHttp2Session | undefined = undefined; + constructor(private shouldActuallySendNotifications = true) { this.sendNotificationImmediately = this.sendNotificationImmediately.bind(this); this.lastReloadedTimeForAPNsIsTooRecent = this.lastReloadedTimeForAPNsIsTooRecent.bind(this); @@ -95,7 +98,10 @@ export class AppleNotificationSender { "apns-topic": bundleId, }; try { - const client = http2.connect(host); + if (!this.client) { + this.client = http2.connect(host); + } + const client = this.client; const req = client.request(headers); req.setEncoding('utf8'); From 06d2a7bb277138efab0efeb56240d1bba1e5f711 Mon Sep 17 00:00:00 2001 From: Brendan Chen Date: Wed, 30 Apr 2025 17:49:39 -0700 Subject: [PATCH 3/5] extract code to open and close the apns connection --- .../senders/AppleNotificationSender.ts | 39 +++++++++++++------ 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/src/notifications/senders/AppleNotificationSender.ts b/src/notifications/senders/AppleNotificationSender.ts index 61c0b73..ebe20f0 100644 --- a/src/notifications/senders/AppleNotificationSender.ts +++ b/src/notifications/senders/AppleNotificationSender.ts @@ -86,7 +86,9 @@ export class AppleNotificationSender { throw new Error("APNS_BUNDLE_ID environment variable is not set correctly"); } - const { path, host } = AppleNotificationSender.getAPNsFullUrlToUse(deviceId); + this.openConnectionIfNoneExists(); + + const { path } = AppleNotificationSender.getAPNsFullUrlToUse(deviceId); const headers = { ':method': 'POST', @@ -98,9 +100,7 @@ export class AppleNotificationSender { "apns-topic": bundleId, }; try { - if (!this.client) { - this.client = http2.connect(host); - } + if (!this.client) { return false } const client = this.client; const req = client.request(headers); req.setEncoding('utf8'); @@ -137,15 +137,21 @@ export class AppleNotificationSender { } } - public static getAPNsFullUrlToUse(deviceId: string): APNsUrl { - // Construct the fetch request - const devBaseUrl = "https://api.development.push.apple.com" - const prodBaseUrl = "https://api.push.apple.com" + private openConnectionIfNoneExists() { + const host = AppleNotificationSender.getAPNsHostToUse(); - let hostToUse = devBaseUrl; - if (process.env.APNS_IS_PRODUCTION === "1") { - hostToUse = prodBaseUrl; + if (!this.client) { + this.client = http2.connect(host); } + } + + private closeConnectionIfExists() { + this.client?.close(); + this.client = undefined; + } + + public static getAPNsFullUrlToUse(deviceId: string): APNsUrl { + let hostToUse = this.getAPNsHostToUse(); const path = "/3/device/" + deviceId; const fullUrl = hostToUse + path; @@ -157,4 +163,15 @@ export class AppleNotificationSender { }; } + public static getAPNsHostToUse() { + // Construct the fetch request + const devBaseUrl = "https://api.development.push.apple.com" + const prodBaseUrl = "https://api.push.apple.com" + + let hostToUse = devBaseUrl; + if (process.env.APNS_IS_PRODUCTION === "1") { + hostToUse = prodBaseUrl; + } + return hostToUse; + } } From 31e9c78ebd8939d4646d158359a216177e6e15dc Mon Sep 17 00:00:00 2001 From: Brendan Chen Date: Wed, 30 Apr 2025 18:09:29 -0700 Subject: [PATCH 4/5] add test for closing connection on emitting closure events --- .../AppleNotificationSenderTests.test.ts | 59 ++++++++++++++----- 1 file changed, 43 insertions(+), 16 deletions(-) diff --git a/test/notifications/senders/AppleNotificationSenderTests.test.ts b/test/notifications/senders/AppleNotificationSenderTests.test.ts index 0ccf20d..67c38f3 100644 --- a/test/notifications/senders/AppleNotificationSenderTests.test.ts +++ b/test/notifications/senders/AppleNotificationSenderTests.test.ts @@ -5,29 +5,36 @@ import { AppleNotificationSender, NotificationAlertArguments } from "../../../src/notifications/senders/AppleNotificationSender"; +import { ClientHttp2Session } from "node:http2"; jest.mock("http2"); const sampleKeyBase64 = "LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tCk1JR1RBZ0VBTUJNR0J5cUdTTTQ5QWdFR0NDcUdTTTQ5QXdFSEJIa3dkd0lCQVFRZ3NybVNBWklhZ09mQ1A4c0IKV2kyQ0JYRzFPbzd2MWJpc3BJWkN3SXI0UkRlZ0NnWUlLb1pJemowREFRZWhSQU5DQUFUWkh4VjJ3UUpMTUJxKwp5YSt5ZkdpM2cyWlV2NmhyZmUrajA4eXRla1BIalhTMHF6Sm9WRUx6S0hhNkVMOVlBb1pEWEJ0QjZoK2ZHaFhlClNPY09OYmFmCi0tLS0tRU5EIFBSSVZBVEUgS0VZLS0tLS0K"; -function mockHttp2Connect(status: number) { - class MockClient extends EventEmitter { - request = jest.fn((_) => { - const mockRequest: any = new EventEmitter(); - mockRequest.setEncoding = jest.fn(); - mockRequest.write = jest.fn(); - mockRequest.end = jest.fn(() => { - setTimeout(() => { - mockRequest.emit('response', { ':status': status }); - }, 10); - }); - return mockRequest; - }); - - close() {}; +class MockClient extends EventEmitter { + constructor( + private status: number, + ) { + super() } - (http2.connect as jest.Mock) = jest.fn(() => new MockClient()); + request = jest.fn((_) => { + const mockRequest: any = new EventEmitter(); + mockRequest.setEncoding = jest.fn(); + mockRequest.write = jest.fn(); + mockRequest.end = jest.fn(() => { + setTimeout(() => { + mockRequest.emit('response', { ':status': this.status }); + }, 10); + }); + return mockRequest; + }); + + close = jest.fn(() => {}); +} + +function mockHttp2Connect(status: number) { + (http2.connect as jest.Mock) = jest.fn(() => new MockClient(status)); } describe("AppleNotificationSender", () => { @@ -159,5 +166,25 @@ describe("AppleNotificationSender", () => { expect(http2.connect).not.toHaveBeenCalled(); expect(result).toBe(true); }); + + it("registers a handler to close the connection if `close` event fired", async () => { + const connectionCloseEvents = ['close', 'goaway', 'error']; + + await Promise.all(connectionCloseEvents.map(async (event) => { + const mockClient = new MockClient(200); + notificationSender = new AppleNotificationSender(true, mockClient as unknown as ClientHttp2Session); + + const notificationArguments: NotificationAlertArguments = { + title: 'Test notification', + body: '' + }; + + const result = await notificationSender.sendNotificationImmediately('1', notificationArguments); + + mockClient.emit(event); + + expect(mockClient.close).toHaveBeenCalled(); + })); + }); }); }); From f74376c8eda1edde78726b07a0d35e27aaa030bd Mon Sep 17 00:00:00 2001 From: Brendan Chen Date: Wed, 30 Apr 2025 18:17:27 -0700 Subject: [PATCH 5/5] add test and code for client closure events --- .../senders/AppleNotificationSender.ts | 25 +++++++++++++++---- .../AppleNotificationSenderTests.test.ts | 4 +-- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/notifications/senders/AppleNotificationSender.ts b/src/notifications/senders/AppleNotificationSender.ts index ebe20f0..ba03989 100644 --- a/src/notifications/senders/AppleNotificationSender.ts +++ b/src/notifications/senders/AppleNotificationSender.ts @@ -18,12 +18,20 @@ export class AppleNotificationSender { private apnsToken: string | undefined = undefined; private _lastRefreshedTimeMs: number | undefined = undefined; - private client: ClientHttp2Session | undefined = undefined; - - constructor(private shouldActuallySendNotifications = true) { + constructor( + private shouldActuallySendNotifications = true, + private client: ClientHttp2Session | undefined = undefined, + ) { this.sendNotificationImmediately = this.sendNotificationImmediately.bind(this); this.lastReloadedTimeForAPNsIsTooRecent = this.lastReloadedTimeForAPNsIsTooRecent.bind(this); this.reloadAPNsTokenIfTimePassed = this.reloadAPNsTokenIfTimePassed.bind(this); + this.openConnectionIfNoneExists = this.openConnectionIfNoneExists.bind(this); + this.closeConnectionIfExists = this.closeConnectionIfExists.bind(this); + this.registerClosureEventsForClient = this.registerClosureEventsForClient.bind(this); + + if (this.client !== undefined) { + this.registerClosureEventsForClient(); + } } get lastRefreshedTimeMs(): number | undefined { @@ -101,8 +109,7 @@ export class AppleNotificationSender { }; try { if (!this.client) { return false } - const client = this.client; - const req = client.request(headers); + const req = this.client.request(headers); req.setEncoding('utf8'); await new Promise((resolve, reject) => { @@ -142,9 +149,17 @@ export class AppleNotificationSender { if (!this.client) { this.client = http2.connect(host); + this.registerClosureEventsForClient(); } } + private registerClosureEventsForClient() { + this.client?.on('close', this.closeConnectionIfExists); + this.client?.on('error', this.closeConnectionIfExists); + this.client?.on('goaway', this.closeConnectionIfExists); + this.client?.on('timeout', this.closeConnectionIfExists); + } + private closeConnectionIfExists() { this.client?.close(); this.client = undefined; diff --git a/test/notifications/senders/AppleNotificationSenderTests.test.ts b/test/notifications/senders/AppleNotificationSenderTests.test.ts index 67c38f3..bd7f9ef 100644 --- a/test/notifications/senders/AppleNotificationSenderTests.test.ts +++ b/test/notifications/senders/AppleNotificationSenderTests.test.ts @@ -168,7 +168,7 @@ describe("AppleNotificationSender", () => { }); it("registers a handler to close the connection if `close` event fired", async () => { - const connectionCloseEvents = ['close', 'goaway', 'error']; + const connectionCloseEvents = ['close', 'goaway', 'error', 'timeout']; await Promise.all(connectionCloseEvents.map(async (event) => { const mockClient = new MockClient(200); @@ -179,7 +179,7 @@ describe("AppleNotificationSender", () => { body: '' }; - const result = await notificationSender.sendNotificationImmediately('1', notificationArguments); + await notificationSender.sendNotificationImmediately('1', notificationArguments); mockClient.emit(event);