Use promise.reject with an Error object instead of string, to ensure we get a stack.

Add test for bug in adapterProxy.

adapter/adapterProxy.ts

@@ -2,7 +2,7 @@
  * Copyright (C) Microsoft Corporation. All rights reserved.
  *--------------------------------------------------------*/
 
-import * as Utilities from '../webkit/utilities';
+import * as utils from '../webkit/utilities';
 
 export type EventHandler = (event: DebugProtocol.Event) => void;
 
@@ -15,7 +15,7 @@ export class AdapterProxy {
 
     public dispatchRequest(request: DebugProtocol.Request): Promise<any> {
         if (!(request.command in this._debugAdapter)) {
-            Promise.reject('unknowncommand');
+            return utils.errP('unknowncommand');
         }
 
         return this.transformRequest(request)
@@ -50,7 +50,7 @@ export class AdapterProxy {
         }
 
         const bodyTransformMethodName = request.command + 'Response';
-        const reversedTransformers = Utilities.reversedArr(this._requestTransformers);
+        const reversedTransformers = utils.reversedArr(this._requestTransformers);
         return reversedTransformers
             // If the transformer implements this command, give it a chance to modify the args. Otherwise skip it
             .filter(transformer => bodyTransformMethodName in transformer)
@@ -63,7 +63,7 @@ export class AdapterProxy {
      * Pass the event back through the transformers in reverse. They modify the object in place.
      */
     private onAdapterEvent(event: DebugProtocol.Event): void {
-        const reversedTransformers = Utilities.reversedArr(this._requestTransformers);
+        const reversedTransformers = utils.reversedArr(this._requestTransformers);
         reversedTransformers
             .filter(transformer => event.event in transformer)
             .forEach(

test/adapter/adapterProxy.test.ts

@@ -0,0 +1,29 @@
+/*---------------------------------------------------------
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ *--------------------------------------------------------*/
+
+import * as assert from 'assert';
+
+import * as testUtils from '../testUtils';
+import {AdapterProxy} from '../../adapter/adapterProxy';
+
+suite('AdapterProxy', () => {
+    setup(() => {
+        testUtils.setupUnhandledRejectionListener();
+    });
+
+    teardown(() => {
+        testUtils.removeUnhandledRejectionListener();
+    });
+
+    suite('request', () => {
+        test('if an unknown command is issued, dispatchRequest fails', () => {
+            const ap = new AdapterProxy(null, <any>{ registerEventHandler: () => { } }, null);
+            return ap.dispatchRequest(<any>{ command: 'abc' }).then(
+                () => assert.fail('Expected to fail'),
+                e => {
+                    assert.equal(e.message, 'unknowncommand');
+                });
+        });
+    });
+});

test/testUtils.ts

@@ -11,7 +11,17 @@ export function removeUnhandledRejectionListener(): void {
 }
 
 function unhandledRejectionListener(reason, p) {
-    console.log(`ERROR!! Unhandled promise rejection: ${reason}`);
+    console.log('*');
+    console.log('**');
+    console.log('***');
+    console.log('****');
+    console.log('*****');
+    console.log(`ERROR!! Unhandled promise rejection: ${reason}. A previous test may have failed but reported success.`);
+    console.log('*****');
+    console.log('****');
+    console.log('***');
+    console.log('**');
+    console.log('*');
 }
 
 export class MockEvent implements DebugProtocol.Event {

test/webkit/utilities.test.ts

@@ -156,11 +156,11 @@ suite('Utilities', () => {
         });
 
         test('when the function fails, it rejects', () => {
-            return Utilities.retryAsync(() => Promise.reject('fail'), /*timeoutMs=*/5)
+            return Utilities.retryAsync(() => Utilities.errP('fail'), /*timeoutMs=*/5)
                 .then(
                     () => assert.fail('This promise should fail'),
                     e => {
-                        assert.equal(e, 'fail');
+                        assert.equal(e.message, 'Error: fail');
                     });
         });
     });

test/webkit/webKitDebugAdapter.test.ts

@@ -8,6 +8,7 @@ import {EventEmitter} from 'events';
 import * as assert from 'assert';
 
 import * as testUtils from '../testUtils';
+import * as utils from '../../webkit/utilities';
 
 /** Not mocked - use for type only */
 import {WebKitDebugAdapter as _WebKitDebugAdapter} from '../../webkit/webKitDebugAdapter';
@@ -69,7 +70,7 @@ suite('WebKitDebugAdapter', () => {
         });
 
         test('if unsuccessful, the promise is rejected and an initialized event is not fired', done => {
-            mockWebKitConnection.expects('attach').returns(Promise.reject('Testing attach failed'));
+            mockWebKitConnection.expects('attach').returns(utils.errP('Testing attach failed'));
 
             const wkda = instantiateWKDA();
             wkda.registerEventHandler((event: DebugProtocol.Event) => {

webkit/utilities.ts

@@ -117,7 +117,7 @@ export function promiseTimeout(p?: Promise<any>, timeoutMs: number = 1000, timeo
 }
 
 export function retryAsync(fn: () => Promise<any>, timeoutMs: number): Promise<any> {
-    var startTime = Date.now();
+    const startTime = Date.now();
 
     function tryUntilTimeout(): Promise<any> {
         return fn().catch(
@@ -125,7 +125,7 @@ export function retryAsync(fn: () => Promise<any>, timeoutMs: number): Promise<a
                 if (Date.now() - startTime < timeoutMs) {
                     return tryUntilTimeout();
                 } else {
-                    return Promise.reject(e);
+                    return errP(e);
                 }
             });
     }
@@ -291,3 +291,7 @@ export function remoteObjectToValue(object: WebKitProtocol.Runtime.RemoteObject,
 
     return { value, variableHandleRef };
 }
+
+export function errP(msg: string): Promise<any> {
+    return Promise.reject(new Error(msg));
+}

webkit/webKitConnection.ts

@@ -5,7 +5,7 @@
 import * as WebSocket from 'ws';
 import * as http from 'http';
 import {EventEmitter} from 'events';
-import * as Utilities from './utilities';
+import * as utils from './utilities';
 import {Logger} from './utilities';
 
 interface IMessageWithId {
@@ -122,7 +122,7 @@ export class WebKitConnection {
      */
     public attach(port: number): Promise<void> {
         Logger.log('Attempting to attach on port ' + port);
-        return Utilities.retryAsync(() => this._attach(port), 6000)
+        return utils.retryAsync(() => this._attach(port), 6000)
             .then(() => this.sendMessage('Debugger.enable'))
             .then(() => this.sendMessage('Console.enable'))
             .then(() => { });
@@ -147,7 +147,7 @@ export class WebKitConnection {
                     // JSON.parse can throw
                 }
 
-                return Promise.reject('Got response, but no valid target pages found');
+                return utils.errP('Got response, but no valid target pages found');
             });
     }
 

webkit/webKitDebugAdapter.ts

@@ -6,7 +6,7 @@ import {Event} from '../common/v8Protocol';
 import {StoppedEvent, InitializedEvent, TerminatedEvent, OutputEvent} from '../common/debugSession';
 import {Handles} from '../common/handles';
 import {WebKitConnection} from './webKitConnection';
-import * as Utilities from './utilities';
+import * as utils from './utilities';
 import {Logger} from './utilities';
 import {formatConsoleMessage} from './consoleHelper';
 
@@ -24,7 +24,7 @@ export class WebKitDebugAdapter implements IDebugAdapter {
     private _variableHandles: Handles<string>;
     private _currentStack: WebKitProtocol.Debugger.CallFrame[];
     private _committedBreakpointsByUrl: Map<string, WebKitProtocol.Debugger.BreakpointId[]>;
-    private _overlayHelper: Utilities.DebounceHelper;
+    private _overlayHelper: utils.DebounceHelper;
 
     private _chromeProc: ChildProcess;
     private _webKitConnection: WebKitConnection;
@@ -37,7 +37,7 @@ export class WebKitDebugAdapter implements IDebugAdapter {
 
     public constructor() {
         this._variableHandles = new Handles<string>();
-        this._overlayHelper = new Utilities.DebounceHelper(/*timeoutMs=*/200);
+        this._overlayHelper = new utils.DebounceHelper(/*timeoutMs=*/200);
 
         this.clearEverything();
     }
@@ -73,9 +73,9 @@ export class WebKitDebugAdapter implements IDebugAdapter {
         }
 
         // Check exists?
-        const chromePath = args.runtimeExecutable || Utilities.getBrowserPath();
+        const chromePath = args.runtimeExecutable || utils.getBrowserPath();
         if (!chromePath) {
-            return Promise.reject(`Can't find Chrome - install it or set the "runtimeExecutable" field in the launch config.`);
+            return utils.errP(`Can't find Chrome - install it or set the "runtimeExecutable" field in the launch config.`);
         }
 
         // Start with remote debugging enabled
@@ -93,7 +93,7 @@ export class WebKitDebugAdapter implements IDebugAdapter {
         } else if (args.url) {
             chromeArgs.push(args.url);
         } else {
-            return Promise.reject('The launch config must specify either the "file" or "url" field.');
+            return utils.errP('The launch config must specify either the "file" or "url" field.');
         }
 
         Logger.log(`spawn('${chromePath}', ${JSON.stringify(chromeArgs) })`);
@@ -108,11 +108,11 @@ export class WebKitDebugAdapter implements IDebugAdapter {
 
     public attach(args: IAttachRequestArgs): Promise<void> {
         if (args.address !== 'localhost' && args.address !== '127.0.0.1') {
-            return Promise.reject('Remote debugging is not supported');
+            return utils.errP('Remote debugging is not supported');
         }
 
         if (args.port == null) {
-            return Promise.reject('The "port" field is required in the attach config.');
+            return utils.errP('The "port" field is required in the attach config.');
         }
 
         if (args.diagnosticLogging) {
@@ -144,7 +144,7 @@ export class WebKitDebugAdapter implements IDebugAdapter {
                 () => this.fireEvent(new InitializedEvent()),
                 e => {
                     this.clearEverything();
-                    return Promise.reject(e);
+                    return utils.errP(e);
                 });
         } else {
             return Promise.resolve<void>();
@@ -276,14 +276,14 @@ export class WebKitDebugAdapter implements IDebugAdapter {
                 .then(() => this._addBreakpoints(targetScriptUrl, args.lines, args.cols))
                 .then(responses => ({ breakpoints: this._webkitBreakpointResponsesToODPBreakpoints(targetScriptUrl, responses, args.lines) }));
 
-            const setBreakpointsPTimeout = Utilities.promiseTimeout(setBreakpointsPFailOnError, /*timeoutMs*/2000, 'Set breakpoints request timed out');
+            const setBreakpointsPTimeout = utils.promiseTimeout(setBreakpointsPFailOnError, /*timeoutMs*/2000, 'Set breakpoints request timed out');
 
             // Do just one setBreakpointsRequest at a time to avoid interleaving breakpoint removed/breakpoint added requests to Chrome.
             // Swallow errors in the promise queue chain so it doesn't get blocked, but return the failing promise for error handling.
             this._setBreakpointsRequestQ = setBreakpointsPTimeout.catch(() => undefined);
             return setBreakpointsPTimeout;
         } else {
-            return Promise.reject(`Can't find script for breakpoint request`);
+            return utils.errP(`Can't find script for breakpoint request`);
         }
     }
 
@@ -481,7 +481,7 @@ export class WebKitDebugAdapter implements IDebugAdapter {
         return evalPromise.then(evalResponse => {
             if (evalResponse.result.wasThrown) {
                 const errorMessage = evalResponse.result.exceptionDetails ? evalResponse.result.exceptionDetails.text : 'Error';
-                return Promise.reject(errorMessage);
+                return utils.errP(errorMessage);
             }
 
             const { value, variablesReference } = this.remoteObjectToValue(evalResponse.result.result);
@@ -505,7 +505,7 @@ export class WebKitDebugAdapter implements IDebugAdapter {
      * use it with this instance's variableHandles to create a variable handle.
      */
     private remoteObjectToValue(object: WebKitProtocol.Runtime.RemoteObject): { value: string, variablesReference: number } {
-        const { value, variableHandleRef } = Utilities.remoteObjectToValue(object);
+        const { value, variableHandleRef } = utils.remoteObjectToValue(object);
         const result = { value, variablesReference: 0 };
         if (variableHandleRef) {
             result.variablesReference = this._variableHandles.create(variableHandleRef);

webkit/webKitDebugSession.ts

@@ -61,8 +61,8 @@ export class WebKitDebugSession extends DebugSession {
             },
             e => {
                 const eStr = e ? e.toString() : 'Unknown error';
-                if (eStr === 'unknowncommand') {
-                    this.sendErrorResponse(response, 1014, 'Unrecognized request', null, ErrorDestination.Telemetry);
+                if (eStr === 'Error: unknowncommand') {
+                    this.sendErrorResponse(response, 1014, '[webkit-debug-adapter] Unrecognized request: ' + request.command, null, ErrorDestination.Telemetry);
                     return;
                 }
 
@@ -74,7 +74,7 @@ export class WebKitDebugSession extends DebugSession {
                     // These errors show up in the message bar at the top (or nowhere), sometimes not obvious that they
                     // come from the adapter
                     response.message = '[webkit-debug-adapter] ' + eStr;
-                    Logger.log('Error: ' + eStr);
+                    Logger.log('Error: ' + e ? e.stack : eStr);
                 }
 
                 response.success = false;