Address api review changes and typos.

etc/firebase-admin.messaging.api.md

@@ -183,11 +183,13 @@ export type Message = TokenMessage | TopicMessage | ConditionMessage;
 // @public
 export class Messaging {
     get app(): App;
+    // @deprecated
+    enableLegacyHttpTransport(): void;
     send(message: Message, dryRun?: boolean): Promise<string>;
     // @deprecated
     sendAll(messages: Message[], dryRun?: boolean): Promise<BatchResponse>;
-    sendEach(messages: Message[], dryRun?: boolean, useHttp2?: boolean): Promise<BatchResponse>;
-    sendEachForMulticast(message: MulticastMessage, dryRun?: boolean, useHttp2?: boolean): Promise<BatchResponse>;
+    sendEach(messages: Message[], dryRun?: boolean): Promise<BatchResponse>;
+    sendEachForMulticast(message: MulticastMessage, dryRun?: boolean): Promise<BatchResponse>;
     // @deprecated
     sendMulticast(message: MulticastMessage, dryRun?: boolean): Promise<BatchResponse>;
     sendToCondition(condition: string, payload: MessagingPayload, options?: MessagingOptions): Promise<MessagingConditionResponse>;

src/messaging/messaging.ts

@@ -193,6 +193,7 @@ export class Messaging {
   private urlPath: string;
   private readonly appInternal: App;
   private readonly messagingRequestHandler: FirebaseMessagingRequestHandler;
+  private useLegacyTransport = false;
 
   /**
    * @internal
@@ -222,6 +223,22 @@ export class Messaging {
     return this.appInternal;
   }
 
+  /**
+   * Enables the use of the legacy HTTP/1.1 transport for sendEach() and sendEachForMulticast().
+   * 
+   * @example
+   * ```javascript
+   * const messaging = getMessaging(app);
+   * messaging.enableLegacyTransport();
+   * messaging.sendEach(messages);
+   * ```
+   * 
+   * @deprecated This is to be removed once the HTTP/2 transport is universally safe.
+   */
+  public enableLegacyHttpTransport(): void {
+    this.useLegacyTransport = true;
+  }
+
   /**
    * Sends the given message via FCM.
    *
@@ -270,7 +287,7 @@ export class Messaging {
   * @returns A Promise fulfilled with an object representing the result of the
   *   send operation.
   */
-  public sendEach(messages: Message[], dryRun?: boolean, useHttp2?: boolean): Promise<BatchResponse> {
+  public sendEach(messages: Message[], dryRun?: boolean): Promise<BatchResponse> {
     if (validator.isArray(messages) && messages.constructor !== Array) {
       // In more recent JS specs, an array-like object might have a constructor that is not of
       // Array type. Our deepCopy() method doesn't handle them properly. Convert such objects to
@@ -292,12 +309,8 @@ export class Messaging {
       throw new FirebaseMessagingError(
         MessagingClientErrorCode.INVALID_ARGUMENT, 'dryRun must be a boolean');
     }
-    if (typeof useHttp2 !== 'undefined' && !validator.isBoolean(useHttp2)) {
-      throw new FirebaseMessagingError(
-        MessagingClientErrorCode.INVALID_ARGUMENT, 'enableHttp2 must be a boolean');
-    }
 
-    const http2SessionHandler = useHttp2 ? new Http2SessionHandler(`https://${FCM_SEND_HOST}`) : undefined
+    const http2SessionHandler = this.useLegacyTransport ? undefined : new Http2SessionHandler(`https://${FCM_SEND_HOST}`)
 
     return this.getUrlPath()
       .then((urlPath) => {
@@ -357,7 +370,7 @@ export class Messaging {
    * @returns A Promise fulfilled with an object representing the result of the
    *   send operation.
    */
-  public sendEachForMulticast(message: MulticastMessage, dryRun?: boolean, useHttp2?: boolean): Promise<BatchResponse> {
+  public sendEachForMulticast(message: MulticastMessage, dryRun?: boolean): Promise<BatchResponse> {
     const copy: MulticastMessage = deepCopy(message);
     if (!validator.isNonNullObject(copy)) {
       throw new FirebaseMessagingError(
@@ -372,10 +385,6 @@ export class Messaging {
         MessagingClientErrorCode.INVALID_ARGUMENT,
         `tokens list must not contain more than ${FCM_MAX_BATCH_SIZE} items`);
     }
-    if (typeof useHttp2 !== 'undefined' && !validator.isBoolean(useHttp2)) {
-      throw new FirebaseMessagingError(
-        MessagingClientErrorCode.INVALID_ARGUMENT, 'enableHttp2 must be a boolean');
-    }
 
     const messages: Message[] = copy.tokens.map((token) => {
       return {
@@ -388,7 +397,7 @@ export class Messaging {
         fcmOptions: copy.fcmOptions,
       };
     });
-    return this.sendEach(messages, dryRun, useHttp2);
+    return this.sendEach(messages, dryRun);
   }
 
   /**

src/utils/api-request.ts

@@ -1307,14 +1307,14 @@ export class Http2SessionHandler {
       http2Session.on('goaway', (errorCode, _, opaqueData) => {
         throw new FirebaseAppError(
           AppErrorCodes.NETWORK_ERROR,
-          `Error while making request: GOAWAY - ${opaqueData.toString()}, Error code: ${errorCode}`
+          `Error while making requests: GOAWAY - ${opaqueData.toString()}, Error code: ${errorCode}`
         );
       })
 
       http2Session.on('error', (error) => {
         throw new FirebaseAppError(
           AppErrorCodes.NETWORK_ERROR,
-          `Error while making request: ${error}`
+          `Error while making requests: ${error}`
         );
       })
       return http2Session

test/integration/messaging.spec.ts

@@ -17,6 +17,7 @@
 import * as chai from 'chai';
 import * as chaiAsPromised from 'chai-as-promised';
 import { Message, MulticastMessage, getMessaging } from '../../lib/messaging/index';
+import { legacyTransportApp } from './setup';
 
 chai.should();
 chai.use(chaiAsPromised);
@@ -101,6 +102,11 @@ const options = {
 };
 
 describe('admin.messaging', () => {
+
+  before(() => {
+    getMessaging(legacyTransportApp).enableLegacyHttpTransport()
+  })
+
   it('send(message, dryRun) returns a message ID', () => {
     return getMessaging().send(message, true)
       .then((name) => {
@@ -110,7 +116,7 @@ describe('admin.messaging', () => {
 
   it('sendEach()', () => {
     const messages: Message[] = [message, message, message];
-    return getMessaging().sendEach(messages, true)
+    return getMessaging(legacyTransportApp).sendEach(messages, true)
       .then((response) => {
         expect(response.responses.length).to.equal(messages.length);
         expect(response.successCount).to.equal(messages.length);
@@ -127,7 +133,7 @@ describe('admin.messaging', () => {
     for (let i = 0; i < 500; i++) {
       messages.push({ topic: `foo-bar-${i % 10}` });
     }
-    return getMessaging().sendEach(messages, true)
+    return getMessaging(legacyTransportApp).sendEach(messages, true)
       .then((response) => {
         expect(response.responses.length).to.equal(messages.length);
         expect(response.successCount).to.equal(messages.length);
@@ -141,7 +147,7 @@ describe('admin.messaging', () => {
 
   it('sendEach() using HTTP2', () => {
     const messages: Message[] = [message, message, message];
-    return getMessaging().sendEach(messages, true, true)
+    return getMessaging().sendEach(messages, true)
       .then((response) => {
         expect(response.responses.length).to.equal(messages.length);
         expect(response.successCount).to.equal(messages.length);
@@ -158,7 +164,7 @@ describe('admin.messaging', () => {
     for (let i = 0; i < 500; i++) {
       messages.push({ topic: `foo-bar-${i % 10}` });
     }
-    return getMessaging().sendEach(messages, true, true)
+    return getMessaging().sendEach(messages, true)
       .then((response) => {
         expect(response.responses.length).to.equal(messages.length);
         expect(response.successCount).to.equal(messages.length);
@@ -207,7 +213,7 @@ describe('admin.messaging', () => {
       android: message.android,
       tokens: ['not-a-token', 'also-not-a-token'],
     };
-    return getMessaging().sendEachForMulticast(multicastMessage, true)
+    return getMessaging(legacyTransportApp).sendEachForMulticast(multicastMessage, true)
       .then((response) => {
         expect(response.responses.length).to.equal(2);
         expect(response.successCount).to.equal(0);
@@ -226,7 +232,7 @@ describe('admin.messaging', () => {
       android: message.android,
       tokens: ['not-a-token', 'also-not-a-token'],
     };
-    return getMessaging().sendEachForMulticast(multicastMessage, true, true)
+    return getMessaging().sendEachForMulticast(multicastMessage, true)
       .then((response) => {
         expect(response.responses.length).to.equal(2);
         expect(response.successCount).to.equal(0);

test/integration/setup.ts

@@ -31,6 +31,7 @@ export let projectId: string;
 export let apiKey: string;
 
 export let defaultApp: App;
+export let legacyTransportApp: App;
 export let nullApp: App;
 export let nonNullApp: App;
 export let noServiceAccountApp: App;
@@ -92,6 +93,13 @@ before(() => {
     storageBucket,
   });
 
+  legacyTransportApp = initializeApp({
+    ...getCredential(),
+    projectId,
+    databaseURL: databaseUrl,
+    storageBucket,
+  }, 'legacyTransport');
+
   nullApp = initializeApp({
     ...getCredential(),
     projectId,
@@ -127,6 +135,7 @@ before(() => {
 after(() => {
   return Promise.all([
     deleteApp(defaultApp),
+    deleteApp(legacyTransportApp),
     deleteApp(nullApp),
     deleteApp(nonNullApp),
     deleteApp(noServiceAccountApp),