Firestore: add experimentalLongPollingTimeout setting (b/266868871, #6987)

Author: dconeybe

common/api-review/firestore.api.md

@@ -227,6 +227,7 @@ export interface FirestoreSettings {
     cacheSizeBytes?: number;
     experimentalAutoDetectLongPolling?: boolean;
     experimentalForceLongPolling?: boolean;
+    experimentalLongPollingTimeout?: number;
     host?: string;
     ignoreUndefinedProperties?: boolean;
     localCache?: FirestoreLocalCache;

packages/firestore/src/api/settings.ts

@@ -92,4 +92,23 @@ export interface FirestoreSettings extends LiteSettings {
    * cannot be combined with `experimentalForceLongPolling`.
    */
   experimentalAutoDetectLongPolling?: boolean;
+
+  /**
+   * The desired maximum timeout interval (in milliseconds) to complete a
+   * long-polling GET response.
+   *
+   * By default, when long-polling is used the "hanging GET" request sent by the
+   * client times out after 30 seconds (30,000 milliseconds). To request a
+   * different timeout from the server, set this setting with the desired
+   * timeout. This may be useful, for example, if the buffering proxy that
+   * necessitated enabling long-polling in the first place has a shorter timeout
+   * for hanging GET requests, in which case setting the long-polling timeout to
+   * a shorter value, such as 25 seconds, may fix prematurely-closed hanging GET
+   * requests.
+   *
+   * This setting is only used if `experimentalForceLongPolling` is true or if
+   * `experimentalAutoDetectLongPolling` is true and the auto-detection
+   * determined that long-polling was needed. Otherwise, it is ignored.
+   */
+  experimentalLongPollingTimeout?: number;
 }

packages/firestore/src/core/database_info.ts

@@ -34,6 +34,9 @@ export class DatabaseInfo {
    * when using WebChannel as the network transport.
    * @param autoDetectLongPolling - Whether to use the detectBufferingProxy
    * option when using WebChannel as the network transport.
+   * @param longPollingTimeout The desired maximum timeout interval (in
+   * milliseconds) to complete a long-polling GET response; the server _may_
+   * ignore this value; may be undefined to use the default (30 seconds).
    * @param useFetchStreams Whether to use the Fetch API instead of
    * XMLHTTPRequest
    */
@@ -45,6 +48,7 @@ export class DatabaseInfo {
     readonly ssl: boolean,
     readonly forceLongPolling: boolean,
     readonly autoDetectLongPolling: boolean,
+    readonly longPollingTimeout: number | undefined,
     readonly useFetchStreams: boolean
   ) {}
 }

packages/firestore/src/lite-api/components.ts

@@ -117,6 +117,7 @@ export function makeDatabaseInfo(
     settings.ssl,
     settings.experimentalForceLongPolling,
     settings.experimentalAutoDetectLongPolling,
+    settings.experimentalLongPollingTimeout,
     settings.useFetchStreams
   );
 }

packages/firestore/src/lite-api/settings.ts

@@ -29,6 +29,17 @@ import { validateIsNotUsedTogether } from '../util/input_validation';
 export const DEFAULT_HOST = 'firestore.googleapis.com';
 export const DEFAULT_SSL = true;
 
+// The minimum long-polling timeout is hardcoded on the server. The value here
+// should be kept in sync with the value used by the server, as the server will
+// silently ignore a value below the minimum and fall back to the default.
+// Googlers see http://google3/net/webchannel/internal/webchannel_config.cc;l=118;rcl=510899643
+const MIN_LONG_POLLING_TIMEOUT = 5000;
+
+// No maximum long-polling timeout is not enforced by the server; however, a
+// "reasonable" maximum value is set by the client to provide some guard rails.
+// Googlers see b/266868871 for relevant discussion.
+const MAX_LONG_POLLING_TIMEOUT = 600000;
+
 /**
  * Specifies custom configurations for your Cloud Firestore instance.
  * You must set these before invoking any other methods.
@@ -60,6 +71,8 @@ export interface PrivateSettings extends FirestoreSettings {
   // Used in firestore@exp
   experimentalAutoDetectLongPolling?: boolean;
   // Used in firestore@exp
+  experimentalLongPollingTimeout?: number;
+  // Used in firestore@exp
   useFetchStreams?: boolean;
 
   localCache?: FirestoreLocalCache;
@@ -83,6 +96,8 @@ export class FirestoreSettingsImpl {
 
   readonly experimentalAutoDetectLongPolling: boolean;
 
+  readonly experimentalLongPollingTimeout: number | undefined;
+
   readonly ignoreUndefinedProperties: boolean;
 
   readonly useFetchStreams: boolean;
@@ -130,6 +145,7 @@ export class FirestoreSettingsImpl {
     this.experimentalForceLongPolling = !!settings.experimentalForceLongPolling;
     this.experimentalAutoDetectLongPolling =
       !!settings.experimentalAutoDetectLongPolling;
+    this.experimentalLongPollingTimeout = settings?.experimentalLongPollingTimeout;
     this.useFetchStreams = !!settings.useFetchStreams;
 
     validateIsNotUsedTogether(
@@ -138,6 +154,32 @@ export class FirestoreSettingsImpl {
       'experimentalAutoDetectLongPolling',
       settings.experimentalAutoDetectLongPolling
     );
+
+    if (typeof this.experimentalLongPollingTimeout === 'number') {
+      if (! Number.isInteger(this.experimentalLongPollingTimeout)) {
+        throw new FirestoreError(
+          Code.INVALID_ARGUMENT,
+          `invalid long polling timeout: ` +
+          `${this.experimentalLongPollingTimeout} (must be an integer)`
+        );
+      }
+      if (this.experimentalLongPollingTimeout < MIN_LONG_POLLING_TIMEOUT) {
+        throw new FirestoreError(
+          Code.INVALID_ARGUMENT,
+          `invalid long polling timeout: ` +
+          `${this.experimentalLongPollingTimeout} ` +
+          `(minimum allowed value is ${MIN_LONG_POLLING_TIMEOUT})`
+        );
+      }
+      if (this.experimentalLongPollingTimeout > MAX_LONG_POLLING_TIMEOUT) {
+        throw new FirestoreError(
+          Code.INVALID_ARGUMENT,
+          `invalid long polling timeout: ` +
+          `${this.experimentalLongPollingTimeout} ` +
+          `(maximum allowed value is ${MAX_LONG_POLLING_TIMEOUT})`
+        );
+      }
+    }
   }
 
   isEqual(other: FirestoreSettingsImpl): boolean {
@@ -150,6 +192,8 @@ export class FirestoreSettingsImpl {
         other.experimentalForceLongPolling &&
       this.experimentalAutoDetectLongPolling ===
         other.experimentalAutoDetectLongPolling &&
+      this.experimentalLongPollingTimeout ===
+        other.experimentalLongPollingTimeout &&
       this.ignoreUndefinedProperties === other.ignoreUndefinedProperties &&
       this.useFetchStreams === other.useFetchStreams
     );

packages/firestore/src/platform/browser/webchannel_connection.ts

@@ -57,12 +57,14 @@ export class WebChannelConnection extends RestConnection {
   private readonly forceLongPolling: boolean;
   private readonly autoDetectLongPolling: boolean;
   private readonly useFetchStreams: boolean;
+  private readonly longPollingTimeout: number | undefined;
 
   constructor(info: DatabaseInfo) {
     super(info);
     this.forceLongPolling = info.forceLongPolling;
     this.autoDetectLongPolling = info.autoDetectLongPolling;
     this.useFetchStreams = info.useFetchStreams;
+    this.longPollingTimeout = info.longPollingTimeout;
   }
 
   protected performRPCRequest<Req, Resp>(
@@ -200,6 +202,10 @@ export class WebChannelConnection extends RestConnection {
       detectBufferingProxy: this.autoDetectLongPolling
     };
 
+    if (this.longPollingTimeout) {
+      request.longPollingTimeout = this.longPollingTimeout;
+    }
+
     if (this.useFetchStreams) {
       request.xmlHttpFactory = new FetchXmlHttpFactory({});
     }

packages/firestore/test/integration/util/internal_helpers.ts

@@ -57,6 +57,7 @@ export function getDefaultDatabaseInfo(): DatabaseInfo {
     !!DEFAULT_SETTINGS.ssl,
     !!DEFAULT_SETTINGS.experimentalForceLongPolling,
     !!DEFAULT_SETTINGS.experimentalAutoDetectLongPolling,
+    DEFAULT_SETTINGS.experimentalLongPollingTimeout,
     /*use FetchStreams= */ false
   );
 }

packages/firestore/test/unit/api/database.test.ts

@@ -265,7 +265,7 @@ describe('SnapshotMetadata', () => {
   });
 });
 
-describe('Settings', () => {
+describe.only('Settings', () => {
   it('can not use mutually exclusive settings together', () => {
     // Use a new instance of Firestore in order to configure settings.
     const db = newTestFirestore();
@@ -279,6 +279,68 @@ describe('Settings', () => {
     );
   });
 
+  it('experimentalLongPollingTimeout is undefined by default', () => {
+    // Use a new instance of Firestore in order to configure settings.
+    const db = newTestFirestore();
+    expect(db._getSettings().experimentalLongPollingTimeout).to.be.undefined;
+  });
+
+  it('experimentalLongPollingTimeout minimum value is allowed', () => {
+    // Use a new instance of Firestore in order to configure settings.
+    const db = newTestFirestore();
+    db._setSettings({experimentalLongPollingTimeout: 5000});
+    expect(db._getSettings().experimentalLongPollingTimeout).to.equal(5000);
+  });
+
+  it('experimentalLongPollingTimeout maximum value is allowed', () => {
+    // Use a new instance of Firestore in order to configure settings.
+    const db = newTestFirestore();
+    db._setSettings({experimentalLongPollingTimeout: 600000});
+    expect(db._getSettings().experimentalLongPollingTimeout).to.equal(600000);
+  });
+
+  it('experimentalLongPollingTimeout typical value is allowed', () => {
+    // Use a new instance of Firestore in order to configure settings.
+    const db = newTestFirestore();
+    db._setSettings({experimentalLongPollingTimeout: 25000});
+    expect(db._getSettings().experimentalLongPollingTimeout).to.equal(25000);
+  });
+
+  it('experimentalLongPollingTimeout value of 4999 throws', () => {
+    // Use a new instance of Firestore in order to configure settings.
+    const db = newTestFirestore();
+    expect(() => db._setSettings({experimentalLongPollingTimeout: 4999}))
+      .to.throw(/invalid.*timeout.*4999.*\(.*5000.*\)/i);
+  });
+
+  it('experimentalLongPollingTimeout value of 0 throws', () => {
+    // Use a new instance of Firestore in order to configure settings.
+    const db = newTestFirestore();
+    expect(() => db._setSettings({experimentalLongPollingTimeout: 0}))
+      .to.throw(/invalid.*timeout.*0.*\(.*5000.*\)/i);
+  });
+
+  it('experimentalLongPollingTimeout value of -1 throws', () => {
+    // Use a new instance of Firestore in order to configure settings.
+    const db = newTestFirestore();
+    expect(() => db._setSettings({experimentalLongPollingTimeout: -1}))
+      .to.throw(/invalid.*timeout.*-1.*\(.*5000.*\)/i);
+  });
+
+  it('experimentalLongPollingTimeout value of 600001 throws', () => {
+    // Use a new instance of Firestore in order to configure settings.
+    const db = newTestFirestore();
+    expect(() => db._setSettings({experimentalLongPollingTimeout: 600001}))
+      .to.throw(/invalid.*timeout.*600001.*\(.*600000.*\)/i);
+  });
+
+  it('experimentalLongPollingTimeout non-integral value throws', () => {
+    // Use a new instance of Firestore in order to configure settings.
+    const db = newTestFirestore();
+    expect(() => db._setSettings({experimentalLongPollingTimeout: 123.456}))
+      .to.throw(/invalid.*timeout.*123.456.*\(.*integer.*\)/i);
+  });
+
   it('gets settings from useEmulator', () => {
     // Use a new instance of Firestore in order to configure settings.
     const db = newTestFirestore();

packages/firestore/test/unit/remote/rest_connection.test.ts

@@ -65,6 +65,7 @@ describe('RestConnection', () => {
     /*ssl=*/ false,
     /*forceLongPolling=*/ false,
     /*autoDetectLongPolling=*/ false,
+    /*longPollingTimeout=*/ undefined,
     /*useFetchStreams=*/ false
   );
   const connection = new TestRestConnection(testDatabaseInfo);

packages/firestore/test/unit/specs/spec_test_runner.ts

@@ -273,6 +273,7 @@ abstract class TestRunner {
       /*ssl=*/ false,
       /*forceLongPolling=*/ false,
       /*autoDetectLongPolling=*/ false,
+      /*longPollingTimeout=*/ undefined,
       /*useFetchStreams=*/ false
     );
 

packages/webchannel-wrapper/externs/overrides.js

@@ -69,6 +69,9 @@ goog.net.WebChannel.Options.detectBufferingProxy;
 /** @type {unknown} */
 goog.net.WebChannel.Options.xmlHttpFactory;
 
+/** @type {number|undefined} */
+goog.net.WebChannel.Options.longPollingTimeout;
+
 goog.labs.net.webChannel.requestStats.Event = {};
 goog.labs.net.webChannel.requestStats.Event.STAT_EVENT;
 

packages/webchannel-wrapper/package.json

@@ -27,8 +27,8 @@
   "license": "Apache-2.0",
   "devDependencies": {
     "@rollup/plugin-commonjs": "21.1.0",
-    "google-closure-compiler": "20220301.0.0",
-    "google-closure-library": "20220301.0.0",
+    "google-closure-compiler": "20230228.0.0",
+    "google-closure-library": "20230228.0.0",
     "gulp": "4.0.2",
     "gulp-sourcemaps": "3.0.0",
     "rollup": "2.79.1",

packages/webchannel-wrapper/src/index.d.ts

@@ -101,6 +101,7 @@ export interface WebChannelOptions {
   encodeInitMessageHeaders?: boolean;
   forceLongPolling?: boolean;
   detectBufferingProxy?: boolean;
+  longPollingTimeout?: number;
   fastHandshake?: boolean;
   disableRedac?: boolean;
   clientProfile?: string;