Puppeteer: page.waitForNavigation() timeout=0 does not disable timeout, but actually sets it to zero

[vonGameTheory]

v1.0.0-rc page.waitForNavigation()

setting timeout:0 should disable timeout according to the docs (meaning it never times out), but it seems to just actually set it to 0 seconds, and then promise reverts to null immediately everytime.

[vonGameTheory]

Or on second thought, maybe it isn't "setting it to zero seconds" rather than just breaking it somehow as it doesn't throw a timeout error, but the promise returned just seems to disappear (starts out as pending but silently goes to null immediately). In any case, it doesn't work.

[vonGameTheory]

So here is the relevant bit of code from NavigatorWatcher.js, line 49 and following:

    const lifecycleCompletePromise = new Promise(fulfill => {
      this._lifecycleCompleteCallback = fulfill;
    });
    this._navigationPromise = Promise.race([
      this._createTimeoutPromise(),
      lifecycleCompletePromise
    ]).then(error => {
      this._cleanup();
      return error;
    });
  }

  /**
   * @return {?Promise<?Error>}
   */
  _createTimeoutPromise() {
    if (!this._timeout)
      return null;
    const errorMessage = 'Navigation Timeout Exceeded: ' + this._timeout + 'ms exceeded';
    return new Promise(fulfill => this._maximumTimer = setTimeout(fulfill, this._timeout))
        .then(() => new Error(errorMessage));
  }

If timeout is set to zero, then _createTimeoutPromise returns a non-Promise plain null, which ends up in the Promise.race function above it, apparently with the idea that it would just ignore it, but any non-Promise given to Promise.race will "win the race" immediately and be returned as the resolved value, thus resolving _navigationPromise with null, which should never happen.

[aslushnikov]

@vonGameTheory thank you for the investigation. I've sent the fix.