Cleaned up implementation and updated session error types

Author: jonathanedey

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<PromiseSettledResult<SendResponse>[]>
+        if (http2SessionHandler) {
+          let batchResponsePromise: Promise<PromiseSettledResult<SendResponse>[]>;
           return new Promise((resolve: (result: PromiseSettledResult<SendResponse>[]) => 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<SendResponse>[] = 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<SendResponse>[] = 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<SendResponse>[] = 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<SendResponse>[]): 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,
+    };
   }
 
   /**

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<void> {
     return this.promise
   }

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';

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<PromiseSettledResult<SendResponse>[]>
-        // }
-        // 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++) {