Fix attach timeout, add some logging, fix tests for it

test/adapter/lineNumberTransformer.test.ts

@@ -3,7 +3,9 @@
  *--------------------------------------------------------*/
 
 import * as assert from 'assert';
+
 import { LineNumberTransformer } from '../../adapter/lineNumberTransformer';
+import * as testUtils from '../testUtils';
 
 function createTransformer(clientLinesStartAt1: boolean, targetLinesStartAt1: boolean): LineNumberTransformer {
     const transformer = new LineNumberTransformer(targetLinesStartAt1);
@@ -13,6 +15,14 @@ function createTransformer(clientLinesStartAt1: boolean, targetLinesStartAt1: bo
 }
 
 suite('LineNumberTransformer', () => {
+    setup(() => {
+        testUtils.setupUnhandledRejectionListener();
+    });
+
+    teardown(() => {
+        testUtils.removeUnhandledRejectionListener();
+    });
+
     const c0t0Transformer = createTransformer(false, false);
     const c0t1Transformer = createTransformer(false, true);
     const c1t0Transformer = createTransformer(true, false);

test/adapter/sourceMaps/sourceMapTransformer.test.ts

@@ -22,6 +22,8 @@ suite('SourceMapTransformer', () => {
     let transformerSMDisabled: _SourceMapTransformer;
 
     setup(() => {
+        testUtils.setupUnhandledRejectionListener();
+
         // Set up mockery with SourceMaps mock
         mockery.enable();
         mockery.registerMock('./sourceMaps', { SourceMaps: MockSourceMaps });
@@ -43,6 +45,7 @@ suite('SourceMapTransformer', () => {
     });
 
     teardown(() => {
+        testUtils.removeUnhandledRejectionListener();
         mockery.deregisterAll();
         mockery.disable();
         transformer = null;

test/webkit/consoleHelper.test.ts

@@ -4,9 +4,18 @@
 
 import * as assert from 'assert';
 
+import * as testUtils from '../testUtils';
 import * as ConsoleHelper from '../../webkit/consoleHelper';
 
 suite('ConsoleHelper', () => {
+    setup(() => {
+        testUtils.setupUnhandledRejectionListener();
+    });
+
+    teardown(() => {
+        testUtils.removeUnhandledRejectionListener();
+    });
+
     function doAssert(message: WebKitProtocol.Console.Message, expectedText: string, expectedIsError = false): void {
         assert.deepEqual(ConsoleHelper.formatConsoleMessage(message), { text: expectedText, isError: expectedIsError });
     }

test/webkit/utilities.test.ts

@@ -5,12 +5,16 @@
 import * as mockery from 'mockery';
 import * as assert from 'assert';
 
+import * as testUtils from '../testUtils';
+
 /** Utilities without mocks - use for type only */
 import * as _Utilities from '../../webkit/utilities';
 
 const MODULE_UNDER_TEST = '../../webkit/utilities';
 suite('Utilities', () => {
     setup(() => {
+        testUtils.setupUnhandledRejectionListener();
+
         mockery.enable({ useCleanCache: true, warnOnReplace: false });
         mockery.registerMock('fs', { statSync: () => { } });
         mockery.registerMock('os', { platform: () => 'win32' });
@@ -20,6 +24,8 @@ suite('Utilities', () => {
     });
 
     teardown(() => {
+        testUtils.removeUnhandledRejectionListener();
+
         mockery.deregisterAll();
         mockery.disable();
     });
@@ -112,26 +118,25 @@ suite('Utilities', () => {
     suite('promiseTimeout()', () => {
         const Utilities: typeof _Utilities = require(MODULE_UNDER_TEST);
 
-        test('when given a promise it fails if the promise never resolves', done => {
-            Utilities.promiseTimeout(new Promise(() => { }), 5).then(
+        test('when given a promise it fails if the promise never resolves', () => {
+            return Utilities.promiseTimeout(new Promise(() => { }), 5).then(
                 () => assert.fail('This promise should fail'),
-                e => done()
+                e => { }
             );
         });
 
-        test('when given a promise it succeeds if the promise resolves', done => {
-            Utilities.promiseTimeout(Promise.resolve('test'), 5).then(
+        test('when given a promise it succeeds if the promise resolves', () => {
+            return Utilities.promiseTimeout(Promise.resolve('test'), 5).then(
                 result => {
                     assert.equal(result, 'test');
-                    done();
                 },
                 e => assert.fail('This promise should pass')
             );
         });
 
-        test('when not given a promise it resolves', done => {
-            Utilities.promiseTimeout(null, 5).then(
-                done,
+        test('when not given a promise it resolves', () => {
+            return Utilities.promiseTimeout(null, 5).then(
+                null,
                 () => assert.fail('This promise should pass')
             );
         });
@@ -140,42 +145,23 @@ suite('Utilities', () => {
     suite('retryAsync()', () => {
         const Utilities: typeof _Utilities = require(MODULE_UNDER_TEST);
 
-        test('when the function passes, it resolves with the value', done => {
-            let callCount = 0;
-            const pass5times = () => {
-                if (callCount > 5) {
-                    assert.fail('Should not be called more than 5 times');
-                }
-
-                return (++callCount === 5) ?
-                    Promise.resolve('test') :
-                    Promise.reject('fail');
-            };
-
-            Utilities.retryAsync(pass5times, 10, /*timeoutBetweenAttempts=*/0).then(
+        test('when the function passes, it resolves with the value', () => {
+            return Utilities.retryAsync(() => Promise.resolve('pass'), /*timeoutMs=*/5).then(
                 result => {
-                    assert.equal(result, 'test');
-                    done();
+                    assert.equal(result, 'pass');
                 },
                 e => {
                     assert.fail('This should have passed');
                 });
         });
 
-        test('when the function fails, it rejects', done => {
-            let callCount = 0;
-            Utilities.retryAsync(() => {
-                if (++callCount > 10) {
-                    assert.fail('Should not be called more than 10 times');
-                }
-
-                return Promise.reject('fail');
-            }, 10, /*timeoutBetweenAttempts=*/0).then(
-                () => assert.fail('This promise should fail'),
-                e => {
-                    assert.equal(e, 'fail');
-                    done();
-                });
+        test('when the function fails, it rejects', () => {
+            return Utilities.retryAsync(() => Promise.reject('fail'), /*timeoutMs=*/5)
+                .then(
+                    () => assert.fail('This promise should fail'),
+                    e => {
+                        assert.equal(e, 'fail');
+                    });
         });
     });
 

testapp/.vscode/launch.json

@@ -18,7 +18,8 @@
             "port": 9222,
             "request": "attach",
             "sourceMaps": true,
-            "outDir": "out"
+            "outDir": "out",
+            "diagnosticLogging": true
         }
     ]
 }

testapp/client/test1.ts

@@ -61,8 +61,8 @@ function evalDebugger() {
 function consoleAPIs() {
     console.log({ a: 1, b: 'asdf', c: { d: 4 } });
     console.log({ a: 1}, {b: 2});
-    console.count('a');
-    console.count('a');
+    console.count('count label');
+    console.count('count label');
     console.dir({ z: 5 });
     console.time('timing');
     console.group('my group');

webkit/utilities.ts

@@ -116,19 +116,21 @@ export function promiseTimeout(p?: Promise<any>, timeoutMs: number = 1000, timeo
     });
 }
 
-export function retryAsync(fn: () => Promise<any>, attempts: number, timeoutBetweenAttempts = 200): Promise<any> {
-    if (attempts <= 0) return Promise.reject('Must specify > 0 attempts');
+export function retryAsync(fn: () => Promise<any>, timeoutMs: number): Promise<any> {
+    var startTime = Date.now();
 
-    return fn().catch(
-        e => {
-            if (--attempts > 0) {
-                // Wait some ms, then recurse
-                return promiseTimeout(null, timeoutBetweenAttempts)
-                    .then(() => retryAsync(fn, attempts, timeoutBetweenAttempts));
-            } else {
-                return Promise.reject(e);
-            }
-        });
+    function tryUntilTimeout(): Promise<any> {
+        return fn().catch(
+            e => {
+                if (Date.now() - startTime < timeoutMs) {
+                    return tryUntilTimeout();
+                } else {
+                    return Promise.reject(e);
+                }
+            });
+    }
+
+    return tryUntilTimeout();
 }
 
 /**

webkit/webKitConnection.ts

@@ -121,21 +121,33 @@ export class WebKitConnection {
      * Attach the websocket to the first available tab in the chrome instance with the given remote debugging port number.
      */
     public attach(port: number): Promise<void> {
-        // Retrying the download 7x * (retry download + attach 5x), and 200 ms between attempts, so like 8s total to attach to Chrome
-        return Utilities.retryAsync(() => this._attach(port), 5)
+        Logger.log('Attempting to attach on port ' + port);
+        return Utilities.retryAsync(() => this._attach(port), 6000)
             .then(() => this.sendMessage('Debugger.enable'))
             .then(() => this.sendMessage('Console.enable'))
             .then(() => { });
     }
 
     public _attach(port: number): Promise<void> {
-        return Utilities.retryAsync(() => getUrl(`http://127.0.0.1:${port}/json`), 7)
+        return getUrl(`http://127.0.0.1:${port}/json`)
             .then(jsonResponse => {
-                const pages = JSON.parse(jsonResponse).filter(target => target.type === 'page');
-                if (!pages.length) return Promise.reject('No valid pages found');
+                // Validate every step of processing the response
+                try {
+                    const responseArray = JSON.parse(jsonResponse);
+                    if (Array.isArray(responseArray)) {
+                        const pages = responseArray.filter(target => target && target.type === 'page');
+                        if (pages.length) {
+                            const wsUrl = pages[0].webSocketDebuggerUrl;
+                            if (wsUrl) {
+                                return this._socket.attach(wsUrl);
+                            }
+                        }
+                    }
+                } catch (e) {
+                    // JSON.parse can throw
+                }
 
-                const wsUrl = pages[0].webSocketDebuggerUrl;
-                return this._socket.attach(wsUrl);
+                return Promise.reject('Got response, but no valid target pages found');
             });
     }
 
@@ -224,7 +236,7 @@ function getUrl(url: string): Promise<string> {
                 resolve(jsonResponse);
             });
         }).on('error', e => {
-            reject('Cannot connect to the target');
+            reject('Cannot connect to the target: ' + e.message);
         });
     });
 }

webkit/webKitDebugAdapter.ts

@@ -152,7 +152,7 @@ export class WebKitDebugAdapter implements IDebugAdapter {
     }
 
     private setupDiagnosticLogging(): void {
-        Logger.enableDiagnosticLogging(msg => this.fireEvent(new OutputEvent(` › ${msg}\n`)));
+        Logger.enableDiagnosticLogging(msg => this.fireEvent(new OutputEvent(`  ›${msg}\n`)));
     }
 
     private fireEvent(event: DebugProtocol.Event): void {