From f08bedb8b38b6a4eb19753253d3f6adb8ad03aba Mon Sep 17 00:00:00 2001 From: Jonathan Edey Date: Fri, 21 Feb 2025 13:17:59 -0500 Subject: [PATCH 1/6] Promise based option --- src/messaging/messaging.ts | 14 ++++++++++- src/utils/api-request.ts | 38 ++++++++++++++++++++++++++---- test/integration/messaging.spec.ts | 14 +++++++++++ 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/src/messaging/messaging.ts b/src/messaging/messaging.ts index 1c1e2a5887..7658462134 100644 --- a/src/messaging/messaging.ts +++ b/src/messaging/messaging.ts @@ -206,10 +206,22 @@ export class Messaging { MessagingClientErrorCode.INVALID_ARGUMENT, 'dryRun must be a boolean'); } - const http2SessionHandler = this.useLegacyTransport ? undefined : new Http2SessionHandler(`https://${FCM_SEND_HOST}`) + // const http2SessionHandler = this.useLegacyTransport ? undefined : new Http2SessionHandler(`https://${FCM_SEND_HOST}`) + const http2SessionHandler = this.useLegacyTransport ? undefined : new Http2SessionHandler(`https://localhost:3001`); return this.getUrlPath() .then((urlPath) => { + // Try listening for errors here? + if (http2SessionHandler){ + http2SessionHandler.invoke().catch((error) => { + console.log("ERROR TO BE PASSED TO USER:") + console.log(error) + + // Throwing here does nothing since it's still not in the promise that's returned? + throw error + }) + } + const requests: Promise[] = copy.map(async (message) => { validateMessage(message); const request: { message: Message; validate_only?: boolean } = { message }; diff --git a/src/utils/api-request.ts b/src/utils/api-request.ts index c5e284e414..afb52d3f07 100644 --- a/src/utils/api-request.ts +++ b/src/utils/api-request.ts @@ -847,12 +847,19 @@ class AsyncHttp2Call extends AsyncRequestCall { ...this.options.headers }); + // console.log("EMIT SESSION ERROR") + // this.http2ConfigImpl.http2SessionHandler.session.emit('error', "MOCK_SESSION_ERROR") + req.on('response', (headers: IncomingHttp2Headers) => { this.handleHttp2Response(headers, req); + + // console.log("EMIT SESSION ERROR") + // this.http2ConfigImpl.http2SessionHandler.session.emit('error', "MOCK_ERROR") }); // Handle errors req.on('error', (err: any) => { + console.log("GOT REQUEST ERROR") if (req.aborted) { return; } @@ -1315,9 +1322,16 @@ export class ExponentialBackoffPoller extends EventEmitter { export class Http2SessionHandler { private http2Session: http2.ClientHttp2Session + protected promise: Promise + protected resolve: () => void; + protected reject: (_: any) => void; constructor(url: string){ - this.http2Session = this.createSession(url) + this.promise = new Promise((resolve, reject) => { + this.resolve = resolve; + this.reject = reject; + this.http2Session = this.createSession(url) + }); } public createSession(url: string): http2.ClientHttp2Session { @@ -1330,23 +1344,37 @@ export class Http2SessionHandler { const http2Session = http2.connect(url, opts) http2Session.on('goaway', (errorCode, _, opaqueData) => { - throw new FirebaseAppError( + console.log("GOT SESSION GOAWAY EVENT") + this.reject(new FirebaseAppError( AppErrorCodes.NETWORK_ERROR, `Error while making requests: GOAWAY - ${opaqueData.toString()}, Error code: ${errorCode}` - ); + )); }) http2Session.on('error', (error) => { - throw new FirebaseAppError( + console.log("GOT SESSION ERROR EVENT") + this.reject(new FirebaseAppError( AppErrorCodes.NETWORK_ERROR, `Error while making requests: ${error}` - ); + )); }) + + // Session close should be where we resolve the promise since we no longer need to listen for errors + http2Session.on('close', () => { + console.log("GOT SESSION CLOSE EVENT") + this.resolve() + }); + return http2Session } return this.http2Session } + // return the promise tracking events + public invoke(): Promise { + return this.promise + } + get session(): http2.ClientHttp2Session { return this.http2Session } diff --git a/test/integration/messaging.spec.ts b/test/integration/messaging.spec.ts index 409261f6aa..7c563f4c3b 100644 --- a/test/integration/messaging.spec.ts +++ b/test/integration/messaging.spec.ts @@ -101,6 +101,20 @@ describe('admin.messaging', () => { }); }); + it('test uncaught error', () => { + const messages: Message[] = [message, message, message]; + // No longer throws uncatchable error + return getMessaging().sendEach(messages, true) + .then((response) => { + console.log(response.responses) + }) + .catch((error) => { + // This error isn't passed here even when thrown + console.log("CAUGHT ERROR") + console.log(error) + }) + }); + it('sendEach(500)', () => { const messages: Message[] = []; for (let i = 0; i < 500; i++) { From 690660f16ff2992058262cbbc61cf45ef0337091 Mon Sep 17 00:00:00 2001 From: Jonathan Edey Date: Mon, 24 Feb 2025 10:45:43 -0500 Subject: [PATCH 2/6] Throw session errors and bundle with a promise to a BatchResponse if one exists --- src/messaging/messaging.ts | 44 ++++++++++++++++++------------ test/integration/messaging.spec.ts | 15 ++++++++-- 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/src/messaging/messaging.ts b/src/messaging/messaging.ts index 7658462134..5e0c468a82 100644 --- a/src/messaging/messaging.ts +++ b/src/messaging/messaging.ts @@ -209,33 +209,48 @@ export class Messaging { // const http2SessionHandler = this.useLegacyTransport ? undefined : new Http2SessionHandler(`https://${FCM_SEND_HOST}`) const http2SessionHandler = this.useLegacyTransport ? undefined : new Http2SessionHandler(`https://localhost:3001`); + return this.getUrlPath() .then((urlPath) => { - // Try listening for errors here? + // Try listening for errors here? if (http2SessionHandler){ - http2SessionHandler.invoke().catch((error) => { - console.log("ERROR TO BE PASSED TO USER:") - console.log(error) - - // Throwing here does nothing since it's still not in the promise that's returned? - throw error + let batchResponsePromise: Promise[]> + return new Promise((resolve: (result: PromiseSettledResult[]) => void, reject) => { + http2SessionHandler.invoke().catch((error) => { + console.log("ERROR TO BE PASSED TO USER:") + console.log(error) + reject({error, batchResponsePromise}) + }) + + + const requests: Promise[] = copy.map(async (message) => { + validateMessage(message); + const request: { message: Message; validate_only?: boolean } = { message }; + if (dryRun) { + request.validate_only = true; + } + return this.messagingRequestHandler.invokeHttp2RequestHandlerForSendResponse( + FCM_SEND_HOST, urlPath, request, http2SessionHandler); + }); + batchResponsePromise = Promise.allSettled(requests) + batchResponsePromise.then(resolve).catch((error) => { + reject({error, batchResponsePromise}) + }) }) } + // const requests: Promise[] = copy.map(async (message) => { validateMessage(message); const request: { message: Message; validate_only?: boolean } = { message }; if (dryRun) { request.validate_only = true; } - - if (http2SessionHandler){ - return this.messagingRequestHandler.invokeHttp2RequestHandlerForSendResponse( - FCM_SEND_HOST, urlPath, request, http2SessionHandler); - } return this.messagingRequestHandler.invokeHttpRequestHandlerForSendResponse(FCM_SEND_HOST, urlPath, request); }); return Promise.allSettled(requests); + // + }) .then((results) => { const responses: SendResponse[] = []; @@ -253,11 +268,6 @@ export class Messaging { failureCount: responses.length - successCount, }; }) - .finally(() => { - if (http2SessionHandler){ - http2SessionHandler.close() - } - }); } /** diff --git a/test/integration/messaging.spec.ts b/test/integration/messaging.spec.ts index 7c563f4c3b..c7cbb5f7eb 100644 --- a/test/integration/messaging.spec.ts +++ b/test/integration/messaging.spec.ts @@ -101,17 +101,26 @@ describe('admin.messaging', () => { }); }); - it('test uncaught error', () => { + it('test uncaught error', async () => { const messages: Message[] = [message, message, message]; + // No longer throws uncatchable error return getMessaging().sendEach(messages, true) .then((response) => { console.log(response.responses) }) - .catch((error) => { - // This error isn't passed here even when thrown + .catch(async (error) => { + // type err = { + // error: any, + // batchResponsePromise: Promise[]> + // } + // If batchResponsePromise is undefined then no messages were sent + // The promise should eventally return with the result of each request console.log("CAUGHT ERROR") console.log(error) + await error.batchResponsePromise.then((results: any) => { + console.log(results) + }) }) }); From 6cfb7fbb6fe6d6fff03e75714a686974c599a138 Mon Sep 17 00:00:00 2001 From: Jonathan Edey Date: Mon, 10 Mar 2025 13:28:25 -0400 Subject: [PATCH 3/6] Cleaned up implementation and updated session error types --- src/messaging/messaging.ts | 95 +++++++++++++++--------------- src/utils/api-request.ts | 23 ++------ src/utils/error.ts | 1 + test/integration/messaging.spec.ts | 23 -------- 4 files changed, 55 insertions(+), 87 deletions(-) diff --git a/src/messaging/messaging.ts b/src/messaging/messaging.ts index 5e0c468a82..e44414155b 100644 --- a/src/messaging/messaging.ts +++ b/src/messaging/messaging.ts @@ -206,68 +206,69 @@ export class Messaging { MessagingClientErrorCode.INVALID_ARGUMENT, 'dryRun must be a boolean'); } - // const http2SessionHandler = this.useLegacyTransport ? undefined : new Http2SessionHandler(`https://${FCM_SEND_HOST}`) - const http2SessionHandler = this.useLegacyTransport ? undefined : new Http2SessionHandler(`https://localhost:3001`); - + const http2SessionHandler = this.useLegacyTransport ? undefined : new Http2SessionHandler(`https://${FCM_SEND_HOST}`); return this.getUrlPath() .then((urlPath) => { - // Try listening for errors here? - if (http2SessionHandler){ - let batchResponsePromise: Promise[]> + if (http2SessionHandler) { + let batchResponsePromise: Promise[]>; return new Promise((resolve: (result: PromiseSettledResult[]) => void, reject) => { + // Start session listeners http2SessionHandler.invoke().catch((error) => { - console.log("ERROR TO BE PASSED TO USER:") - console.log(error) - reject({error, batchResponsePromise}) - }) - + error.pendingBatchResponse = + batchResponsePromise ? batchResponsePromise.then(this.parseSendResponses) : undefined; + reject(error); + }); + // Start making requests const requests: Promise[] = copy.map(async (message) => { validateMessage(message); - const request: { message: Message; validate_only?: boolean } = { message }; + const request: { message: Message; validate_only?: boolean; } = { message }; if (dryRun) { request.validate_only = true; } return this.messagingRequestHandler.invokeHttp2RequestHandlerForSendResponse( - FCM_SEND_HOST, urlPath, request, http2SessionHandler); + FCM_SEND_HOST, urlPath, request, http2SessionHandler); }); - batchResponsePromise = Promise.allSettled(requests) - batchResponsePromise.then(resolve).catch((error) => { - reject({error, batchResponsePromise}) - }) - }) - } - // - const requests: Promise[] = copy.map(async (message) => { - validateMessage(message); - const request: { message: Message; validate_only?: boolean } = { message }; - if (dryRun) { - request.validate_only = true; - } - return this.messagingRequestHandler.invokeHttpRequestHandlerForSendResponse(FCM_SEND_HOST, urlPath, request); - }); - return Promise.allSettled(requests); - // - - }) - .then((results) => { - const responses: SendResponse[] = []; - results.forEach(result => { - if (result.status === 'fulfilled') { - responses.push(result.value); - } else { // rejected - responses.push({ success: false, error: result.reason }) - } - }) - const successCount: number = responses.filter((resp) => resp.success).length; - return { - responses, - successCount, - failureCount: responses.length - successCount, - }; + // Resolve once all requests have completed + batchResponsePromise = Promise.allSettled(requests); + batchResponsePromise.then(resolve); + }); + } else { + const requests: Promise[] = copy.map(async (message) => { + validateMessage(message); + const request: { message: Message; validate_only?: boolean; } = { message }; + if (dryRun) { + request.validate_only = true; + } + return this.messagingRequestHandler.invokeHttpRequestHandlerForSendResponse( + FCM_SEND_HOST, urlPath, request); + }); + return Promise.allSettled(requests); + } }) + .then(this.parseSendResponses) + .finally(() => { + http2SessionHandler?.close(); + }); + } + + private parseSendResponses(results: PromiseSettledResult[]): BatchResponse { + const responses: SendResponse[] = []; + results.forEach(result => { + if (result.status === 'fulfilled') { + responses.push(result.value); + } else { // rejected + responses.push({ success: false, error: result.reason }); + } + }); + const successCount: number = responses.filter((resp) => resp.success).length; + return { + responses, + successCount, + failureCount: responses.length - successCount, + }; } /** diff --git a/src/utils/api-request.ts b/src/utils/api-request.ts index afb52d3f07..e3d0d8a279 100644 --- a/src/utils/api-request.ts +++ b/src/utils/api-request.ts @@ -847,19 +847,12 @@ class AsyncHttp2Call extends AsyncRequestCall { ...this.options.headers }); - // console.log("EMIT SESSION ERROR") - // this.http2ConfigImpl.http2SessionHandler.session.emit('error', "MOCK_SESSION_ERROR") - req.on('response', (headers: IncomingHttp2Headers) => { this.handleHttp2Response(headers, req); - - // console.log("EMIT SESSION ERROR") - // this.http2ConfigImpl.http2SessionHandler.session.emit('error', "MOCK_ERROR") }); // Handle errors req.on('error', (err: any) => { - console.log("GOT REQUEST ERROR") if (req.aborted) { return; } @@ -1061,6 +1054,7 @@ class Http2RequestConfigImpl extends BaseRequestConfigImpl implements Http2Reque public buildRequestOptions(): https.RequestOptions { const parsed = this.buildUrl(); + // TODO(b/401051826) const protocol = parsed.protocol; return { @@ -1344,33 +1338,28 @@ export class Http2SessionHandler { const http2Session = http2.connect(url, opts) http2Session.on('goaway', (errorCode, _, opaqueData) => { - console.log("GOT SESSION GOAWAY EVENT") this.reject(new FirebaseAppError( - AppErrorCodes.NETWORK_ERROR, - `Error while making requests: GOAWAY - ${opaqueData.toString()}, Error code: ${errorCode}` + AppErrorCodes.HTTP2_SESSION_ERROR, + `Error while making requests: GOAWAY - ${opaqueData?.toString()}, Error code: ${errorCode}` )); }) http2Session.on('error', (error) => { - console.log("GOT SESSION ERROR EVENT") this.reject(new FirebaseAppError( - AppErrorCodes.NETWORK_ERROR, - `Error while making requests: ${error}` + AppErrorCodes.HTTP2_SESSION_ERROR, + `Session error while making requests: ${error}` )); }) - // Session close should be where we resolve the promise since we no longer need to listen for errors http2Session.on('close', () => { - console.log("GOT SESSION CLOSE EVENT") + // Resolve current promise this.resolve() }); - return http2Session } return this.http2Session } - // return the promise tracking events public invoke(): Promise { return this.promise } diff --git a/src/utils/error.ts b/src/utils/error.ts index cf3736adcb..3f4376429b 100644 --- a/src/utils/error.ts +++ b/src/utils/error.ts @@ -377,6 +377,7 @@ export class AppErrorCodes { public static INVALID_APP_OPTIONS = 'invalid-app-options'; public static INVALID_CREDENTIAL = 'invalid-credential'; public static NETWORK_ERROR = 'network-error'; + public static HTTP2_SESSION_ERROR = 'http2-session-error'; public static NETWORK_TIMEOUT = 'network-timeout'; public static NO_APP = 'no-app'; public static UNABLE_TO_PARSE_RESPONSE = 'unable-to-parse-response'; diff --git a/test/integration/messaging.spec.ts b/test/integration/messaging.spec.ts index c7cbb5f7eb..409261f6aa 100644 --- a/test/integration/messaging.spec.ts +++ b/test/integration/messaging.spec.ts @@ -101,29 +101,6 @@ describe('admin.messaging', () => { }); }); - it('test uncaught error', async () => { - const messages: Message[] = [message, message, message]; - - // No longer throws uncatchable error - return getMessaging().sendEach(messages, true) - .then((response) => { - console.log(response.responses) - }) - .catch(async (error) => { - // type err = { - // error: any, - // batchResponsePromise: Promise[]> - // } - // If batchResponsePromise is undefined then no messages were sent - // The promise should eventally return with the result of each request - console.log("CAUGHT ERROR") - console.log(error) - await error.batchResponsePromise.then((results: any) => { - console.log(results) - }) - }) - }); - it('sendEach(500)', () => { const messages: Message[] = []; for (let i = 0; i < 500; i++) { From 99e050850237f88ef48316b4714a795bfab81082 Mon Sep 17 00:00:00 2001 From: Jonathan Edey Date: Mon, 10 Mar 2025 15:07:37 -0400 Subject: [PATCH 4/6] Added unit test --- test/resources/mocks.ts | 18 ++++++----- test/unit/messaging/messaging.spec.ts | 29 ++++++++++++++++++ test/unit/utils/api-request.spec.ts | 43 +++++++++++++++++++++++++-- 3 files changed, 81 insertions(+), 9 deletions(-) diff --git a/test/resources/mocks.ts b/test/resources/mocks.ts index d4afddd879..f9d08023d2 100644 --- a/test/resources/mocks.ts +++ b/test/resources/mocks.ts @@ -322,10 +322,11 @@ export interface MockHttp2Request { } export interface MockHttp2Response { - headers: http2.IncomingHttpHeaders & http2.IncomingHttpStatusHeader, - data: Buffer, + headers?: http2.IncomingHttpHeaders & http2.IncomingHttpStatusHeader, + data?: Buffer, delay?: number, - error?: any + sessionError?: any + streamError?: any, } export class Http2Mocker { @@ -340,12 +341,12 @@ export class Http2Mocker { this.connectStub = sinon.stub(http2, 'connect'); this.connectStub.callsFake((_target: any, options: any) => { const session = this.originalConnect('https://www.example.com', options); - session.request = this.createMockRequest() + session.request = this.createMockRequest(session) return session; }) } - private createMockRequest() { + private createMockRequest(session:http2.ClientHttp2Session) { return (requestHeaders: http2.OutgoingHttpHeaders) => { // Create a mock ClientHttp2Stream to return const mockStream = new stream.Readable({ @@ -365,8 +366,11 @@ export class Http2Mocker { const mockRes = this.mockResponses.shift(); if (mockRes) { this.timeouts.push(setTimeout(() => { - if (mockRes.error) { - mockStream.emit('error', mockRes.error) + if (mockRes.sessionError) { + session.emit('error', mockRes.sessionError) + } + if (mockRes.streamError) { + mockStream.emit('error', mockRes.streamError) } else { mockStream.emit('response', mockRes.headers); diff --git a/test/unit/messaging/messaging.spec.ts b/test/unit/messaging/messaging.spec.ts index 488056488e..41ea43b6fb 100644 --- a/test/unit/messaging/messaging.spec.ts +++ b/test/unit/messaging/messaging.spec.ts @@ -121,6 +121,12 @@ function mockHttp2SendRequestError( } as mocks.MockHttp2Response } +function mockHttp2Error(streamError?: any, sessionError?:any): mocks.MockHttp2Response { + return { + streamError: streamError, + sessionError: sessionError + } as mocks.MockHttp2Response +} function mockErrorResponse( path: string, @@ -906,6 +912,29 @@ describe('Messaging', () => { }); }); + it('should throw error with BatchResponse promise on session error event using HTTP/2', () => { + mockedHttp2Responses.push(mockHttp2SendRequestResponse('projects/projec_id/messages/1')) + const sessionError = 'MOCK_SESSION_ERROR' + mockedHttp2Responses.push(mockHttp2Error( + new Error(`MOCK_STREAM_ERROR caused by ${sessionError}`), + new Error(sessionError) + )); + http2Mocker.http2Stub(mockedHttp2Responses) + + return messaging.sendEach( + [validMessage, validMessage], true + ).catch(async (error) => { + expect(error.errorInfo.code).to.equal('app/http2-session-error') + await error.pendingBatchResponse.then((response: BatchResponse) => { + expect(http2Mocker.requests.length).to.equal(2); + expect(response.failureCount).to.equal(1); + const responses = response.responses; + checkSendResponseSuccess(responses[0], 'projects/projec_id/messages/1'); + checkSendResponseFailure(responses[1], 'app/network-error'); + }) + }); + }) + // This test was added to also verify https://github.com/firebase/firebase-admin-node/issues/1146 it('should be fulfilled when called with different message types using HTTP/2', () => { const messageIds = [ diff --git a/test/unit/utils/api-request.spec.ts b/test/unit/utils/api-request.spec.ts index d64589f8c9..db2b43006e 100644 --- a/test/unit/utils/api-request.spec.ts +++ b/test/unit/utils/api-request.spec.ts @@ -140,12 +140,14 @@ function mockHttp2SendRequestError( } as mocks.MockHttp2Response } -function mockHttp2Error(err: any): mocks.MockHttp2Response { +function mockHttp2Error(streamError?: any, sessionError?:any): mocks.MockHttp2Response { return { - error: err + streamError: streamError, + sessionError: sessionError } as mocks.MockHttp2Response } + /** * Returns a new RetryConfig instance for testing. This is same as the default * RetryConfig, with the backOffFactor set to 0 to avoid delays. @@ -2500,6 +2502,43 @@ describe('Http2Client', () => { http2SessionHandler: http2SessionHandler }).should.eventually.be.rejectedWith(err).and.have.property('code', 'app/network-error'); }); + + it('should fail on session and stream errors', async () => { + const reqData = { request: 'data' }; + const streamError = 'Error while making request: test stream error. Error code: AWFUL_STREAM_ERROR'; + const sessionError = 'Session error while making requests: Error: AWFUL_SESSION_ERROR' + mockedHttp2Responses.push(mockHttp2Error( + { message: 'test stream error', code: 'AWFUL_STREAM_ERROR' }, + new Error('AWFUL_SESSION_ERROR') + )); + http2Mocker.http2Stub(mockedHttp2Responses); + + const client = new Http2Client(); + http2SessionHandler = new Http2SessionHandler(mockHostUrl) + + await client.send({ + method: 'POST', + url: mockUrl, + headers: { + 'authorization': 'Bearer token', + 'My-Custom-Header': 'CustomValue', + }, + data: reqData, + http2SessionHandler: http2SessionHandler, + }).should.eventually.be.rejectedWith(streamError).and.have.property('code', 'app/network-error') + .then(() => { + expect(http2Mocker.requests.length).to.equal(1); + expect(http2Mocker.requests[0].headers[':method']).to.equal('POST'); + expect(http2Mocker.requests[0].headers[':scheme']).to.equal('https:'); + expect(http2Mocker.requests[0].headers[':path']).to.equal(mockPath); + expect(JSON.parse(http2Mocker.requests[0].data)).to.deep.equal(reqData); + expect(http2Mocker.requests[0].headers.authorization).to.equal('Bearer token'); + expect(http2Mocker.requests[0].headers['content-type']).to.contain('application/json'); + expect(http2Mocker.requests[0].headers['My-Custom-Header']).to.equal('CustomValue'); + }); + + await http2SessionHandler.invoke().should.eventually.be.rejectedWith(sessionError) + }); }); describe('AuthorizedHttpClient', () => { From 7f868340fa68b0fc83f5846b1d95c639a21a7b09 Mon Sep 17 00:00:00 2001 From: Jonathan Edey Date: Mon, 10 Mar 2025 15:44:40 -0400 Subject: [PATCH 5/6] rebuild apidocs --- etc/firebase-admin.app.api.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/etc/firebase-admin.app.api.md b/etc/firebase-admin.app.api.md index 97ab9667ce..46cc2256d9 100644 --- a/etc/firebase-admin.app.api.md +++ b/etc/firebase-admin.app.api.md @@ -19,6 +19,8 @@ export class AppErrorCodes { // (undocumented) static DUPLICATE_APP: string; // (undocumented) + static HTTP2_SESSION_ERROR: string; + // (undocumented) static INTERNAL_ERROR: string; // (undocumented) static INVALID_APP_NAME: string; From 7a7faa6a6664834b56283a954d6ec26f05279d7f Mon Sep 17 00:00:00 2001 From: Jonathan Edey Date: Wed, 12 Mar 2025 11:36:27 -0400 Subject: [PATCH 6/6] Wrap session errors in a new `FirebaseMessagingSessionError` --- etc/firebase-admin.app.api.md | 2 -- src/messaging/messaging.ts | 16 +++++++------ src/utils/api-request.ts | 4 ++-- src/utils/error.ts | 34 ++++++++++++++++++++++++++- test/unit/messaging/messaging.spec.ts | 8 ++++--- test/unit/utils/api-request.spec.ts | 1 + 6 files changed, 50 insertions(+), 15 deletions(-) diff --git a/etc/firebase-admin.app.api.md b/etc/firebase-admin.app.api.md index 46cc2256d9..97ab9667ce 100644 --- a/etc/firebase-admin.app.api.md +++ b/etc/firebase-admin.app.api.md @@ -19,8 +19,6 @@ export class AppErrorCodes { // (undocumented) static DUPLICATE_APP: string; // (undocumented) - static HTTP2_SESSION_ERROR: string; - // (undocumented) static INTERNAL_ERROR: string; // (undocumented) static INVALID_APP_NAME: string; diff --git a/src/messaging/messaging.ts b/src/messaging/messaging.ts index e44414155b..8e61cd679a 100644 --- a/src/messaging/messaging.ts +++ b/src/messaging/messaging.ts @@ -17,7 +17,9 @@ import { App } from '../app'; import { deepCopy } from '../utils/deep-copy'; -import { ErrorInfo, MessagingClientErrorCode, FirebaseMessagingError } from '../utils/error'; +import { + ErrorInfo, MessagingClientErrorCode, FirebaseMessagingError, FirebaseMessagingSessionError +} from '../utils/error'; import * as utils from '../utils'; import * as validator from '../utils/validator'; import { validateMessage } from './messaging-internal'; @@ -211,13 +213,13 @@ export class Messaging { return this.getUrlPath() .then((urlPath) => { if (http2SessionHandler) { - let batchResponsePromise: Promise[]>; + let sendResponsePromise: Promise[]>; return new Promise((resolve: (result: PromiseSettledResult[]) => void, reject) => { // Start session listeners http2SessionHandler.invoke().catch((error) => { - error.pendingBatchResponse = - batchResponsePromise ? batchResponsePromise.then(this.parseSendResponses) : undefined; - reject(error); + const pendingBatchResponse = + sendResponsePromise ? sendResponsePromise.then(this.parseSendResponses) : undefined; + reject(new FirebaseMessagingSessionError(error, undefined, pendingBatchResponse)); }); // Start making requests @@ -232,8 +234,8 @@ export class Messaging { }); // Resolve once all requests have completed - batchResponsePromise = Promise.allSettled(requests); - batchResponsePromise.then(resolve); + sendResponsePromise = Promise.allSettled(requests); + sendResponsePromise.then(resolve); }); } else { const requests: Promise[] = copy.map(async (message) => { diff --git a/src/utils/api-request.ts b/src/utils/api-request.ts index e3d0d8a279..ca3140a27d 100644 --- a/src/utils/api-request.ts +++ b/src/utils/api-request.ts @@ -1339,14 +1339,14 @@ export class Http2SessionHandler { http2Session.on('goaway', (errorCode, _, opaqueData) => { this.reject(new FirebaseAppError( - AppErrorCodes.HTTP2_SESSION_ERROR, + AppErrorCodes.NETWORK_ERROR, `Error while making requests: GOAWAY - ${opaqueData?.toString()}, Error code: ${errorCode}` )); }) http2Session.on('error', (error) => { this.reject(new FirebaseAppError( - AppErrorCodes.HTTP2_SESSION_ERROR, + AppErrorCodes.NETWORK_ERROR, `Session error while making requests: ${error}` )); }) diff --git a/src/utils/error.ts b/src/utils/error.ts index 3f4376429b..a80254b5b8 100644 --- a/src/utils/error.ts +++ b/src/utils/error.ts @@ -16,6 +16,7 @@ */ import { FirebaseError as FirebaseErrorInterface } from '../app'; +import { BatchResponse } from '../messaging/messaging-api'; import { deepCopy } from '../utils/deep-copy'; /** @@ -344,6 +345,38 @@ export class FirebaseMessagingError extends PrefixedFirebaseError { } } +export class FirebaseMessagingSessionError extends FirebaseMessagingError { + public pendingBatchResponse?: Promise; + /** + * + * @param info - The error code info. + * @param message - The error message. This will override the default message if provided. + * @param pendingBatchResponse - BatchResponse for pending messages when session error occured. + * @constructor + * @internal + */ + constructor(info: ErrorInfo, message?: string, pendingBatchResponse?: Promise) { + // Override default message if custom message provided. + super(info, message || info.message); + this.pendingBatchResponse = pendingBatchResponse; + + /* tslint:disable:max-line-length */ + // Set the prototype explicitly. See the following link for more details: + // https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes#extending-built-ins-like-error-array-and-map-may-no-longer-work + /* tslint:enable:max-line-length */ + (this as any).__proto__ = FirebaseMessagingSessionError.prototype; + } + + /** @returns The object representation of the error. */ + public toJSON(): object { + return { + code: this.code, + message: this.message, + pendingBatchResponse: this.pendingBatchResponse, + }; + } +} + /** * Firebase project management error code structure. This extends PrefixedFirebaseError. */ @@ -377,7 +410,6 @@ export class AppErrorCodes { public static INVALID_APP_OPTIONS = 'invalid-app-options'; public static INVALID_CREDENTIAL = 'invalid-credential'; public static NETWORK_ERROR = 'network-error'; - public static HTTP2_SESSION_ERROR = 'http2-session-error'; public static NETWORK_TIMEOUT = 'network-timeout'; public static NO_APP = 'no-app'; public static UNABLE_TO_PARSE_RESPONSE = 'unable-to-parse-response'; diff --git a/test/unit/messaging/messaging.spec.ts b/test/unit/messaging/messaging.spec.ts index 41ea43b6fb..1ea0c059de 100644 --- a/test/unit/messaging/messaging.spec.ts +++ b/test/unit/messaging/messaging.spec.ts @@ -34,6 +34,7 @@ import { import { HttpClient } from '../../../src/utils/api-request'; import { getMetricsHeader, getSdkVersion } from '../../../src/utils/index'; import * as utils from '../utils'; +import { FirebaseMessagingSessionError } from '../../../src/utils/error'; chai.should(); chai.use(sinonChai); @@ -923,9 +924,10 @@ describe('Messaging', () => { return messaging.sendEach( [validMessage, validMessage], true - ).catch(async (error) => { - expect(error.errorInfo.code).to.equal('app/http2-session-error') - await error.pendingBatchResponse.then((response: BatchResponse) => { + ).catch(async (error: FirebaseMessagingSessionError) => { + expect(error.code).to.equal('messaging/app/network-error'); + expect(error.pendingBatchResponse).to.not.be.undefined; + await error.pendingBatchResponse?.then((response: BatchResponse) => { expect(http2Mocker.requests.length).to.equal(2); expect(response.failureCount).to.equal(1); const responses = response.responses; diff --git a/test/unit/utils/api-request.spec.ts b/test/unit/utils/api-request.spec.ts index db2b43006e..691f0f89e0 100644 --- a/test/unit/utils/api-request.spec.ts +++ b/test/unit/utils/api-request.spec.ts @@ -2538,6 +2538,7 @@ describe('Http2Client', () => { }); await http2SessionHandler.invoke().should.eventually.be.rejectedWith(sessionError) + .and.have.property('code', 'app/network-error') }); });