Skip to content
This repository was archived by the owner on Apr 3, 2024. It is now read-only.

Commit 8b50f38

Browse files
mctavishJustinBeckwith
authored andcommitted
feat: improve experience when multiple files match a breakpoint location (#784)
The log message contains the requested path and the matching paths. It can be used to help users diagnose their ambiguous file errors.
1 parent 56fe969 commit 8b50f38

File tree

5 files changed

+80
-14
lines changed

5 files changed

+80
-14
lines changed

src/agent/util/utils.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,15 @@ function resolveScripts(
126126
// There should be no ambiguity resolution if project root is provided.
127127
return fileStats[candidate] ? [candidate] : [];
128128
}
129-
const regexp = pathToRegExp(scriptPath);
129+
130+
// Try for an exact match using the working directory.
131+
const candidate = path.join(config.workingDirectory || '', scriptPath);
132+
if (fileStats[candidate]) {
133+
return [candidate];
134+
}
135+
130136
// Next try to match path.
137+
const regexp = pathToRegExp(scriptPath);
131138
const matches = Object.keys(fileStats).filter(regexp.test.bind(regexp));
132139
if (matches.length === 1) {
133140
return matches;

src/agent/v8/inspector-debugapi.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -378,12 +378,13 @@ export class InspectorDebugApi implements debugapi.DebugApi {
378378
// working directory.
379379
let matchingScript;
380380
// TODO: Address the case where `breakpoint.location` is `null`.
381+
const scriptPath = mapInfo
382+
? mapInfo.file
383+
: path.normalize(
384+
(breakpoint.location as stackdriver.SourceLocation).path
385+
);
381386
const scripts = utils.findScripts(
382-
mapInfo
383-
? mapInfo.file
384-
: path.normalize(
385-
(breakpoint.location as stackdriver.SourceLocation).path
386-
),
387+
scriptPath,
387388
this.config,
388389
this.fileStats,
389390
this.logger
@@ -399,6 +400,9 @@ export class InspectorDebugApi implements debugapi.DebugApi {
399400
// Found the script
400401
matchingScript = scripts[0];
401402
} else {
403+
this.logger.warn(
404+
`Unable to unambiguously find ${scriptPath}. Potential matches: ${scripts}`
405+
);
402406
return utils.setErrorStatusAndCallback(
403407
cb,
404408
breakpoint,

src/agent/v8/legacy-debugapi.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -335,12 +335,13 @@ export class V8DebugApi implements debugapi.DebugApi {
335335
// debuglet, we are going to assume that repository root === the starting
336336
// working directory.
337337
let matchingScript;
338+
const scriptPath = mapInfo
339+
? mapInfo.file
340+
: path.normalize(
341+
(breakpoint.location as stackdriver.SourceLocation).path
342+
);
338343
const scripts = utils.findScripts(
339-
mapInfo
340-
? mapInfo.file
341-
: path.normalize(
342-
(breakpoint.location as stackdriver.SourceLocation).path
343-
),
344+
scriptPath,
344345
this.config,
345346
this.fileStats,
346347
this.logger
@@ -356,6 +357,9 @@ export class V8DebugApi implements debugapi.DebugApi {
356357
// Found the script
357358
matchingScript = scripts[0];
358359
} else {
360+
this.logger.warn(
361+
`Unable to unambiguously find ${scriptPath}. Potential matches: ${scripts}`
362+
);
359363
return utils.setErrorStatusAndCallback(
360364
cb,
361365
breakpoint,

test/mock-logger.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,15 @@ export class MockLogger implements consoleLogLevel.Logger {
4040
);
4141
}
4242

43+
clear() {
44+
this.traces = [];
45+
this.debugs = [];
46+
this.infos = [];
47+
this.warns = [];
48+
this.errors = [];
49+
this.fatals = [];
50+
}
51+
4352
trace(...args: Arguments) {
4453
this.traces.push({type: 'trace', args});
4554
}

test/test-v8debugapi.ts

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -246,9 +246,7 @@ describe('v8debugapi', () => {
246246
forceNewAgent_: true,
247247
javascriptFileExtensions: ['.js', '.jsz'],
248248
});
249-
const logger = consoleLogLevel({
250-
level: Debuglet.logLevelToName(config.logLevel),
251-
});
249+
const logger = new MockLogger();
252250
let api: DebugApi;
253251

254252
beforeEach(done => {
@@ -288,6 +286,7 @@ describe('v8debugapi', () => {
288286
}
289287
});
290288
afterEach(() => {
289+
logger.clear();
291290
assert(stateIsClean(api));
292291
});
293292

@@ -436,6 +435,32 @@ describe('v8debugapi', () => {
436435
assert(
437436
bp.status!.description.format === utils.messages.SOURCE_FILE_AMBIGUOUS
438437
);
438+
439+
// Verify that a log message is emitted.
440+
assert.strictEqual(
441+
logger.warns.length,
442+
1,
443+
`Expected 1 warning log message, got ${logger.allCalls.length}`
444+
);
445+
const message = logger.warns[0].args[0];
446+
let expectedSubstring = path.join('fixtures', 'a', 'hello.js');
447+
assert.notStrictEqual(
448+
message.indexOf(expectedSubstring),
449+
-1,
450+
`Missing text '${expectedSubstring}' in '${message}'`
451+
);
452+
expectedSubstring = path.join('fixtures', 'b', 'hello.js');
453+
assert.notStrictEqual(
454+
message.indexOf(expectedSubstring),
455+
-1,
456+
`Missing text '${expectedSubstring}' in '${message}'`
457+
);
458+
expectedSubstring = 'Unable to unambiguously find';
459+
assert.notStrictEqual(
460+
message.indexOf(expectedSubstring),
461+
-1,
462+
`Missing text '${expectedSubstring}' in '${message}'`
463+
);
439464
done();
440465
});
441466
});
@@ -1980,6 +2005,23 @@ describe('v8debugapi.findScripts', () => {
19802005
assert.strictEqual(logger.allCalls().length, 0);
19812006
});
19822007

2008+
it('should prefer exact path matches to ones involving subdirectories', () => {
2009+
const config = extend(true, {}, undefined!, {
2010+
workingDirectory: path.join('root'),
2011+
});
2012+
2013+
const logger = new MockLogger();
2014+
2015+
const fakeFileStats = {
2016+
[path.join('root', 'hello.js')]: {hash: 'fake', lines: 5},
2017+
[path.join('root', 'subdir', 'hello.js')]: {hash: 'fake', lines: 50},
2018+
};
2019+
const scriptPath = 'hello.js';
2020+
const result = utils.findScripts(scriptPath, config, fakeFileStats, logger);
2021+
assert.deepStrictEqual(result, [path.join('root', 'hello.js')]);
2022+
assert.strictEqual(logger.allCalls().length, 0);
2023+
});
2024+
19832025
it('should invoke pathResolver if provided', () => {
19842026
const config = extend(true, {}, undefined!, {
19852027
pathResolver(

0 commit comments

Comments
 (0)