feat(FrameManage): improve errors from frame.waitFor* methods (#2292)

This patch adds title for WaitTask, using it later in generating error messages and
making exceptions much more traceable.

Fixes #2037

lib/FrameManager.js

@@ -676,7 +676,7 @@ class Frame {
   waitForFunction(pageFunction, options = {}, ...args) {
     const timeout = helper.isNumber(options.timeout) ? options.timeout : 30000;
     const polling = options.polling || 'raf';
-    return new WaitTask(this, pageFunction, polling, timeout, ...args).promise;
+    return new WaitTask(this, pageFunction, 'function', polling, timeout, ...args).promise;
   }
 
   /**
@@ -696,7 +696,8 @@ class Frame {
     const waitForVisible = !!options.visible;
     const waitForHidden = !!options.hidden;
     const polling = waitForVisible || waitForHidden ? 'raf' : 'mutation';
-    return this.waitForFunction(predicate, {timeout: options.timeout, polling}, selectorOrXPath, isXPath, waitForVisible, waitForHidden);
+    const timeout = helper.isNumber(options.timeout) ? options.timeout : 30000;
+    return new WaitTask(this, predicate, `${isXPath ? 'XPath' : 'selector'} "${selectorOrXPath}"`, polling, timeout, selectorOrXPath, isXPath, waitForVisible, waitForHidden).promise;
 
     /**
      * @param {string} selectorOrXPath
@@ -769,7 +770,7 @@ class WaitTask {
    * @param {number} timeout
    * @param {!Array<*>} args
    */
-  constructor(frame, predicateBody, polling, timeout, ...args) {
+  constructor(frame, predicateBody, title, polling, timeout, ...args) {
     if (helper.isString(polling))
       console.assert(polling === 'raf' || polling === 'mutation', 'Unknown polling option: ' + polling);
     else if (helper.isNumber(polling))
@@ -791,7 +792,7 @@ class WaitTask {
     // Since page navigation requires us to re-install the pageScript, we should track
     // timeout on our end.
     if (timeout)
-      this._timeoutTimer = setTimeout(() => this.terminate(new Error(`waiting failed: timeout ${timeout}ms exceeded`)), timeout);
+      this._timeoutTimer = setTimeout(() => this.terminate(new Error(`waiting for ${title} failed: timeout ${timeout}ms exceeded`)), timeout);
     this.rerun();
   }
 

test/frame.spec.js

@@ -145,6 +145,12 @@ module.exports.addTests = function({testRunner, expect}) {
       await page.evaluate(element => element.remove(), div);
       await waitForFunction;
     });
+    it('should respect timeout', async({page}) => {
+      let error = null;
+      await page.waitForFunction('false', {timeout: 10}).catch(e => error = e);
+      expect(error).toBeTruthy();
+      expect(error.message).toContain('waiting for function failed: timeout');
+    });
     it('should disable timeout when its set to 0', async({page}) => {
       let error = null;
       const res = await page.waitForFunction(() => new Promise(res => setTimeout(() => res(42), 100)), {timeout: 0}).catch(e => error = e);
@@ -292,7 +298,7 @@ module.exports.addTests = function({testRunner, expect}) {
       let error = null;
       await page.waitForSelector('div', {timeout: 10}).catch(e => error = e);
       expect(error).toBeTruthy();
-      expect(error.message).toContain('waiting failed: timeout');
+      expect(error.message).toContain('waiting for selector "div" failed: timeout');
     });
 
     it('should respond to node attribute mutation', async({page, server}) => {
@@ -318,6 +324,12 @@ module.exports.addTests = function({testRunner, expect}) {
       const waitForXPath = page.waitForXPath('//p[normalize-space(.)="hello world"]');
       expect(await page.evaluate(x => x.textContent, await waitForXPath)).toBe('hello  world  ');
     });
+    it('should respect timeout', async({page}) => {
+      let error = null;
+      await page.waitForXPath('//div', {timeout: 10}).catch(e => error = e);
+      expect(error).toBeTruthy();
+      expect(error.message).toContain('waiting for XPath "//div" failed: timeout');
+    });
     it('should run in specified frame', async({page, server}) => {
       await utils.attachFrame(page, 'frame1', server.EMPTY_PAGE);
       await utils.attachFrame(page, 'frame2', server.EMPTY_PAGE);