From 6d3f8dc2b85e82cb3ba0ffb74e7bbb8e8d926785 Mon Sep 17 00:00:00 2001 From: Noj Vek Date: Tue, 21 Mar 2017 23:57:41 -0700 Subject: [PATCH 1/5] Support source mapping of stack traces in the Debug Console #6 --- src/chrome/chromeDebugAdapter.ts | 35 +++++++++++++++++++++++--- src/chrome/consoleHelper.ts | 2 +- test/chrome/chromeDebugAdapter.test.ts | 28 +++++++++++++++++++++ test/chrome/consoleHelper.test.ts | 4 +-- 4 files changed, 62 insertions(+), 7 deletions(-) diff --git a/src/chrome/chromeDebugAdapter.ts b/src/chrome/chromeDebugAdapter.ts index 3b4af79a6..4593b8cfa 100644 --- a/src/chrome/chromeDebugAdapter.ts +++ b/src/chrome/chromeDebugAdapter.ts @@ -740,10 +740,37 @@ export abstract class ChromeDebugAdapter implements IDebugAdapter { protected onExceptionThrown(params: Crdp.Runtime.ExceptionThrownEvent): void { const formattedException = formatExceptionDetails(params.exceptionDetails); - this._session.sendEvent(new OutputEvent( - formattedException, - 'stderr' - )); + this.mapFormattedException(formattedException).then(formattedException => { + this._session.sendEvent(new OutputEvent( + formattedException, + 'stderr' + )); + }); + } + + // We parse stack trace from `formattedException`, source map it and return a new string + protected async mapFormattedException(formattedException: string): Promise { + const exceptionLines = formattedException.split(/\r?\n/); + + for (let i = 0, len = exceptionLines.length; i < len; ++i) { + const line = exceptionLines[i]; + const matches = line.match(/^\s+at (.*?)\s*\(?([^ ]+\.js):(\d+):(\d+)\)?$/); + + if (matches) { + const fnName = matches[1]; + const path = matches[2]; + const lineNum = parseInt(matches[3]); + const columnNum = parseInt(matches[4]); + const clientPath = this._pathTransformer.getClientPathFromTargetPath(path); + const mapped = await this._sourceMapTransformer.mapToAuthored(clientPath, lineNum, columnNum); + + if (mapped) { + exceptionLines[i] = ` at ${fnName ? fnName + ' ': ''}(${mapped.source}:${mapped.line}:${mapped.column})`; + } + } + } + + return exceptionLines.join('\n'); } /** diff --git a/src/chrome/consoleHelper.ts b/src/chrome/consoleHelper.ts index e5fbf0c47..90a3cffb4 100644 --- a/src/chrome/consoleHelper.ts +++ b/src/chrome/consoleHelper.ts @@ -140,7 +140,7 @@ function stackTraceToString(stackTrace: Crdp.Runtime.StackTrace): string { .map(frame => { const fnName = frame.functionName || (frame.url ? '(anonymous)' : '(eval)'); const fileName = frame.url ? url.parse(frame.url).pathname : 'eval'; - return ` at ${fnName} (${fileName}:${frame.lineNumber})`; + return ` at ${fnName} (${fileName}:${frame.lineNumber}:${frame.columnNumber})`; }) .join('\n'); } diff --git a/test/chrome/chromeDebugAdapter.test.ts b/test/chrome/chromeDebugAdapter.test.ts index 5c3dcbfdd..25b62883a 100644 --- a/test/chrome/chromeDebugAdapter.test.ts +++ b/test/chrome/chromeDebugAdapter.test.ts @@ -468,6 +468,34 @@ suite('ChromeDebugAdapter', () => { }); }); + suite('onExceptionThrown', () => { + test('exceptions are source mapped when shown in console', async () => { + await chromeDebugAdapter.attach(ATTACH_ARGS); + mockEventEmitter.emit('Runtime.exceptionThrown', { + "timestamp": 1490164925297.7559, + "exceptionDetails": { + "exceptionId": 21, + "text": "Uncaught", + "lineNumber": 5, + "columnNumber": 10, + "url": "http://localhost:9999/error.js", + "stackTrace": {}, + "exception": { + "type": "object", + "subtype": "error", + "className": "Error", + "description": "Error: kaboom!\n at error (http://localhost:9999/error.js:6:11)\n at some (http://localhost:9999/error.js:4:33)\n at up (http://localhost:9999/error.js:3:31)\n at blow (http://localhost:9999/error.js:2:33)\n at boom (http://localhost:9999/error.js:1:33)\n at http://localhost:9999/error.js:8:1", + "objectId": "{\"injectedScriptId\":148,\"id\":1}" + }, + "executionContextId": 148 + } + }); + + //TODO - Test mapped exception + + }); + }); + suite('setExceptionBreakpoints()', () => { }); suite('stepping', () => { }); suite('stackTrace()', () => { }); diff --git a/test/chrome/consoleHelper.test.ts b/test/chrome/consoleHelper.test.ts index dcab2c142..58054f07c 100644 --- a/test/chrome/consoleHelper.test.ts +++ b/test/chrome/consoleHelper.test.ts @@ -99,7 +99,7 @@ suite('ConsoleHelper', () => { suite('console.assert()', () => { test(`Prints params and doesn't resolve format specifiers`, () => { - doAssertForString(Runtime.makeAssert('Fail %s 123', 456), 'Assertion failed: Fail %s 123 456\n at myFn (/script/a.js:4)', true); + doAssertForString(Runtime.makeAssert('Fail %s 123', 456), 'Assertion failed: Fail %s 123 456\n at myFn (/script/a.js:4:1)', true); }); }); }); @@ -162,7 +162,7 @@ namespace Runtime { export function makeAssert(...args: any[]): Crdp.Runtime.ConsoleAPICalledEvent { const fakeStackTrace: Crdp.Runtime.StackTrace = { - callFrames: [{ url: '/script/a.js', lineNumber: 4, columnNumber: 0, scriptId: '1', functionName: 'myFn' }] + callFrames: [{ url: '/script/a.js', lineNumber: 4, columnNumber: 1, scriptId: '1', functionName: 'myFn' }] }; return makeMockMessage('assert', args, { level: 'error', stackTrace: fakeStackTrace }); } From b4a76bb32181990c92c46b7d7277e414710bcd90 Mon Sep 17 00:00:00 2001 From: Noj Vek Date: Wed, 22 Mar 2017 11:02:11 -0700 Subject: [PATCH 2/5] tslint plugin is legendary --- src/chrome/chromeDebugAdapter.ts | 12 ++++++------ test/chrome/chromeDebugAdapter.test.ts | 13 ++++++++++++- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/chrome/chromeDebugAdapter.ts b/src/chrome/chromeDebugAdapter.ts index 4593b8cfa..5d8d804c1 100644 --- a/src/chrome/chromeDebugAdapter.ts +++ b/src/chrome/chromeDebugAdapter.ts @@ -740,16 +740,16 @@ export abstract class ChromeDebugAdapter implements IDebugAdapter { protected onExceptionThrown(params: Crdp.Runtime.ExceptionThrownEvent): void { const formattedException = formatExceptionDetails(params.exceptionDetails); - this.mapFormattedException(formattedException).then(formattedException => { + this.sourceMapFormattedException(formattedException).then(exceptionStr => { this._session.sendEvent(new OutputEvent( - formattedException, + exceptionStr, 'stderr' )); }); } // We parse stack trace from `formattedException`, source map it and return a new string - protected async mapFormattedException(formattedException: string): Promise { + protected async sourceMapFormattedException(formattedException: string): Promise { const exceptionLines = formattedException.split(/\r?\n/); for (let i = 0, len = exceptionLines.length; i < len; ++i) { @@ -759,13 +759,13 @@ export abstract class ChromeDebugAdapter implements IDebugAdapter { if (matches) { const fnName = matches[1]; const path = matches[2]; - const lineNum = parseInt(matches[3]); - const columnNum = parseInt(matches[4]); + const lineNum = parseInt(matches[3], 10); + const columnNum = parseInt(matches[4], 10); const clientPath = this._pathTransformer.getClientPathFromTargetPath(path); const mapped = await this._sourceMapTransformer.mapToAuthored(clientPath, lineNum, columnNum); if (mapped) { - exceptionLines[i] = ` at ${fnName ? fnName + ' ': ''}(${mapped.source}:${mapped.line}:${mapped.column})`; + exceptionLines[i] = ` at ${fnName ? fnName + ' ' : ''}(${mapped.source}:${mapped.line}:${mapped.column})`; } } } diff --git a/test/chrome/chromeDebugAdapter.test.ts b/test/chrome/chromeDebugAdapter.test.ts index 25b62883a..362676b0c 100644 --- a/test/chrome/chromeDebugAdapter.test.ts +++ b/test/chrome/chromeDebugAdapter.test.ts @@ -471,6 +471,17 @@ suite('ChromeDebugAdapter', () => { suite('onExceptionThrown', () => { test('exceptions are source mapped when shown in console', async () => { await chromeDebugAdapter.attach(ATTACH_ARGS); + let outputEventFired = false; + sendEventHandler = (event: DebugProtocol.Event) => { + if (event.event === 'output') { + outputEventFired = true; + console.error(event); + assert.equal(true, true); + } else { + testUtils.assertFail('An unexpected event was fired'); + } + }; + mockEventEmitter.emit('Runtime.exceptionThrown', { "timestamp": 1490164925297.7559, "exceptionDetails": { @@ -491,7 +502,7 @@ suite('ChromeDebugAdapter', () => { } }); - //TODO - Test mapped exception + // TODO - Test mapped exception }); }); From 26249b78358ba699d252a7ba1fbfbd5432a68e6f Mon Sep 17 00:00:00 2001 From: Noj Vek Date: Wed, 22 Mar 2017 23:45:04 -0700 Subject: [PATCH 3/5] onExceptionThrown tests --- src/chrome/chromeDebugAdapter.ts | 8 +- test/chrome/chromeDebugAdapter.test.ts | 126 ++++++++++++++++--------- test/mocks/debugProtocolMocks.ts | 2 +- 3 files changed, 89 insertions(+), 47 deletions(-) diff --git a/src/chrome/chromeDebugAdapter.ts b/src/chrome/chromeDebugAdapter.ts index 5d8d804c1..e3fa4944f 100644 --- a/src/chrome/chromeDebugAdapter.ts +++ b/src/chrome/chromeDebugAdapter.ts @@ -757,15 +757,17 @@ export abstract class ChromeDebugAdapter implements IDebugAdapter { const matches = line.match(/^\s+at (.*?)\s*\(?([^ ]+\.js):(\d+):(\d+)\)?$/); if (matches) { - const fnName = matches[1]; const path = matches[2]; const lineNum = parseInt(matches[3], 10); const columnNum = parseInt(matches[4], 10); const clientPath = this._pathTransformer.getClientPathFromTargetPath(path); const mapped = await this._sourceMapTransformer.mapToAuthored(clientPath, lineNum, columnNum); - if (mapped) { - exceptionLines[i] = ` at ${fnName ? fnName + ' ' : ''}(${mapped.source}:${mapped.line}:${mapped.column})`; + if (mapped && mapped.source && mapped.line && mapped.column) { + exceptionLines[i] = exceptionLines[i].replace( + `${path}:${lineNum}:${columnNum}`, + `${mapped.source}:${mapped.line}:${mapped.column}` + ); } } } diff --git a/test/chrome/chromeDebugAdapter.test.ts b/test/chrome/chromeDebugAdapter.test.ts index 362676b0c..98cdd5e9a 100644 --- a/test/chrome/chromeDebugAdapter.test.ts +++ b/test/chrome/chromeDebugAdapter.test.ts @@ -75,26 +75,30 @@ suite('ChromeDebugAdapter', () => { mockSourceMapTransformer = getMockSourceMapTransformer(); mockPathTransformer = getMockPathTransformer(); + initChromeDebugAdapter(); + }); + + function initChromeDebugAdapter(): void { // Instantiate the ChromeDebugAdapter, injecting the mock ChromeConnection /* tslint:disable */ chromeDebugAdapter = new (require(MODULE_UNDER_TEST).ChromeDebugAdapter)({ - chromeConnection: function() { return mockChromeConnection.object; }, - lineColTransformer: function() { return mockLineNumberTransformer.object; }, - sourceMapTransformer: function() { return mockSourceMapTransformer.object; }, - pathTransformer: function() { return mockPathTransformer.object; } + chromeConnection: function () { return mockChromeConnection.object; }, + lineColTransformer: function () { return mockLineNumberTransformer.object; }, + sourceMapTransformer: function () { return mockSourceMapTransformer.object; }, + pathTransformer: function () { return mockPathTransformer.object; } }, - { - sendEvent: (e: DebugProtocol.Event) => { - if (sendEventHandler) { - // Filter telemetry events - if (!(e.event === 'output' && (e).body.category === 'telemetry')) { - sendEventHandler(e); + { + sendEvent: (e: DebugProtocol.Event) => { + if (sendEventHandler) { + // Filter telemetry events + if (!(e.event === 'output' && (e).body.category === 'telemetry')) { + sendEventHandler(e); + } } } - } - }); + }); /* tslint:enable */ - }); + } teardown(() => { sendEventHandler = undefined; @@ -113,6 +117,16 @@ suite('ChromeDebugAdapter', () => { mockEventEmitter.emit('Debugger.scriptParsed', { scriptId, url }); } + // Helper to run async asserts inside promises so they can be correctly awaited + function asyncAssert(assertFn: Function, resolve: (value?: any) => void, reject: (reason?: any) => void): void { + try { + assertFn(); + resolve(); + } catch (e) { + reject(e); + } + } + suite('attach()', () => { test('if successful, an initialized event is fired', () => { let initializedFired = false; @@ -469,41 +483,67 @@ suite('ChromeDebugAdapter', () => { }); suite('onExceptionThrown', () => { - test('exceptions are source mapped when shown in console', async () => { + const authoredPath = '/Users/me/error.ts'; + const generatedPath = 'http://localhost:9999/error.js'; + + const getExceptionStr = (path, line) => 'Error: kaboom!\n' + + ` at error (${path}:${line}:1)\n` + + ` at ${path}:${line}:1`; + + const generatedExceptionStr = getExceptionStr(generatedPath, 6); + const authoredExceptionStr = getExceptionStr(authoredPath, 12); + + const exceptionEvent: Crdp.Runtime.ExceptionThrownEvent = { + "timestamp": 1490164925297, + "exceptionDetails": { + "exceptionId": 21, + "text": "Uncaught", + "lineNumber": 5, + "columnNumber": 10, + "url": "http://localhost:9999/error.js", + "stackTrace": null, + "exception": { + "type": "object", + "subtype": "error", + "className": "Error", + "description": generatedExceptionStr, + "objectId": "{\"injectedScriptId\":148,\"id\":1}" + }, + "executionContextId": 148 + } + }; + + test('passes through exception when no source mapping present', async () => { await chromeDebugAdapter.attach(ATTACH_ARGS); - let outputEventFired = false; - sendEventHandler = (event: DebugProtocol.Event) => { - if (event.event === 'output') { - outputEventFired = true; - console.error(event); - assert.equal(true, true); - } else { - testUtils.assertFail('An unexpected event was fired'); - } - }; + const sendEventP = new Promise((resolve, reject) => { + sendEventHandler = (event) => + asyncAssert(() => assert.equal(event.body.output, generatedExceptionStr), resolve, reject); + }); - mockEventEmitter.emit('Runtime.exceptionThrown', { - "timestamp": 1490164925297.7559, - "exceptionDetails": { - "exceptionId": 21, - "text": "Uncaught", - "lineNumber": 5, - "columnNumber": 10, - "url": "http://localhost:9999/error.js", - "stackTrace": {}, - "exception": { - "type": "object", - "subtype": "error", - "className": "Error", - "description": "Error: kaboom!\n at error (http://localhost:9999/error.js:6:11)\n at some (http://localhost:9999/error.js:4:33)\n at up (http://localhost:9999/error.js:3:31)\n at blow (http://localhost:9999/error.js:2:33)\n at boom (http://localhost:9999/error.js:1:33)\n at http://localhost:9999/error.js:8:1", - "objectId": "{\"injectedScriptId\":148,\"id\":1}" - }, - "executionContextId": 148 - } + mockEventEmitter.emit('Runtime.exceptionThrown', exceptionEvent); + await sendEventP; + }); + + test('translates callstack to authored files via source mapping', async () => { + // We need to reset mocks and re-initialize chromeDebugAdapter + // because reset() creates a new instance of object + mockSourceMapTransformer.reset(); + mockPathTransformer.reset(); + initChromeDebugAdapter(); + + await chromeDebugAdapter.attach(ATTACH_ARGS); + const sendEventP = new Promise((resolve, reject) => { + sendEventHandler = (event) => + asyncAssert(() => assert.equal(event.body.output, authoredExceptionStr), resolve, reject); }); - // TODO - Test mapped exception + mockPathTransformer.setup(m => m.getClientPathFromTargetPath(It.isValue(generatedPath))) + .returns(path => path); + mockSourceMapTransformer.setup(m => m.mapToAuthored(It.isValue(generatedPath), It.isAnyNumber(), It.isAnyNumber())) + .returns(() => Promise.resolve({ source: authoredPath, line: 12, column: 1 })); + mockEventEmitter.emit('Runtime.exceptionThrown', exceptionEvent); + await sendEventP; }); }); diff --git a/test/mocks/debugProtocolMocks.ts b/test/mocks/debugProtocolMocks.ts index e3e3769ad..8ee877941 100644 --- a/test/mocks/debugProtocolMocks.ts +++ b/test/mocks/debugProtocolMocks.ts @@ -48,7 +48,7 @@ function getRuntimeStubs(mockEventEmitter) { evaluate() { }, onConsoleAPICalled(handler) { mockEventEmitter.on('Runtime.consoleAPICalled', handler); }, - onExceptionThrown(handler) { mockEventEmitter.on('Runtime.onExceptionThrown', handler); }, + onExceptionThrown(handler) { mockEventEmitter.on('Runtime.exceptionThrown', handler); }, onExecutionContextsCleared(handler) { mockEventEmitter.on('Runtime.executionContextsCleared', handler); } }; } From bdc70a24374f8e573fc11117d31d114dca94e4b3 Mon Sep 17 00:00:00 2001 From: Noj Vek Date: Wed, 22 Mar 2017 23:59:12 -0700 Subject: [PATCH 4/5] continues rather than nested ifs --- src/chrome/chromeDebugAdapter.ts | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/chrome/chromeDebugAdapter.ts b/src/chrome/chromeDebugAdapter.ts index e3fa4944f..35c55717b 100644 --- a/src/chrome/chromeDebugAdapter.ts +++ b/src/chrome/chromeDebugAdapter.ts @@ -756,19 +756,19 @@ export abstract class ChromeDebugAdapter implements IDebugAdapter { const line = exceptionLines[i]; const matches = line.match(/^\s+at (.*?)\s*\(?([^ ]+\.js):(\d+):(\d+)\)?$/); - if (matches) { - const path = matches[2]; - const lineNum = parseInt(matches[3], 10); - const columnNum = parseInt(matches[4], 10); - const clientPath = this._pathTransformer.getClientPathFromTargetPath(path); - const mapped = await this._sourceMapTransformer.mapToAuthored(clientPath, lineNum, columnNum); - - if (mapped && mapped.source && mapped.line && mapped.column) { - exceptionLines[i] = exceptionLines[i].replace( - `${path}:${lineNum}:${columnNum}`, - `${mapped.source}:${mapped.line}:${mapped.column}` - ); - } + if (!matches) continue; + const path = matches[2]; + const lineNum = parseInt(matches[3], 10); + const columnNum = parseInt(matches[4], 10); + const clientPath = this._pathTransformer.getClientPathFromTargetPath(path); + if (!clientPath) continue; + const mapped = await this._sourceMapTransformer.mapToAuthored(clientPath, lineNum, columnNum); + + if (mapped && mapped.source && mapped.line && mapped.column) { + exceptionLines[i] = exceptionLines[i].replace( + `${path}:${lineNum}:${columnNum}`, + `${mapped.source}:${mapped.line}:${mapped.column}` + ); } } From 2671bdb3c48b040170fabe7c38542bf23d72d922 Mon Sep 17 00:00:00 2001 From: Noj Vek Date: Thu, 23 Mar 2017 00:13:32 -0700 Subject: [PATCH 5/5] clientPath can be undefined and that is okay --- src/chrome/chromeDebugAdapter.ts | 3 +-- test/chrome/chromeDebugAdapter.test.ts | 3 --- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/chrome/chromeDebugAdapter.ts b/src/chrome/chromeDebugAdapter.ts index 35c55717b..c1777b95e 100644 --- a/src/chrome/chromeDebugAdapter.ts +++ b/src/chrome/chromeDebugAdapter.ts @@ -761,8 +761,7 @@ export abstract class ChromeDebugAdapter implements IDebugAdapter { const lineNum = parseInt(matches[3], 10); const columnNum = parseInt(matches[4], 10); const clientPath = this._pathTransformer.getClientPathFromTargetPath(path); - if (!clientPath) continue; - const mapped = await this._sourceMapTransformer.mapToAuthored(clientPath, lineNum, columnNum); + const mapped = await this._sourceMapTransformer.mapToAuthored(clientPath || path, lineNum, columnNum); if (mapped && mapped.source && mapped.line && mapped.column) { exceptionLines[i] = exceptionLines[i].replace( diff --git a/test/chrome/chromeDebugAdapter.test.ts b/test/chrome/chromeDebugAdapter.test.ts index 98cdd5e9a..fda319825 100644 --- a/test/chrome/chromeDebugAdapter.test.ts +++ b/test/chrome/chromeDebugAdapter.test.ts @@ -528,7 +528,6 @@ suite('ChromeDebugAdapter', () => { // We need to reset mocks and re-initialize chromeDebugAdapter // because reset() creates a new instance of object mockSourceMapTransformer.reset(); - mockPathTransformer.reset(); initChromeDebugAdapter(); await chromeDebugAdapter.attach(ATTACH_ARGS); @@ -537,8 +536,6 @@ suite('ChromeDebugAdapter', () => { asyncAssert(() => assert.equal(event.body.output, authoredExceptionStr), resolve, reject); }); - mockPathTransformer.setup(m => m.getClientPathFromTargetPath(It.isValue(generatedPath))) - .returns(path => path); mockSourceMapTransformer.setup(m => m.mapToAuthored(It.isValue(generatedPath), It.isAnyNumber(), It.isAnyNumber())) .returns(() => Promise.resolve({ source: authoredPath, line: 12, column: 1 }));