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

Commit

Permalink
fix: use well-formatted URLs when setting breakpoints (#535)
Browse files Browse the repository at this point in the history
Newer versions of Node will require that URLs be well-formatted when setting breakpoints (and will also return well-formatted URLs), as a result of nodejs/node#22223 (this will also affect versions of Node to which that PR will be backported). Enforcing well-formatted URLs will break the Debug Agent, so this PR addresses that.

If tests pass for current versions of Node, I think this should be merged now (rather than wait for Node 11).
  • Loading branch information
kjin authored and DominicKramer committed Nov 20, 2018
1 parent c60f61e commit cefd4ae
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 5 deletions.
6 changes: 4 additions & 2 deletions src/agent/state/inspector-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,10 @@ class StateResolver {
if (this.scriptmapper[scriptId].url === undefined) {
return '';
}
return this.scriptmapper[scriptId].url;
const scriptUrl = this.scriptmapper[scriptId].url;
// In Node 11+, non-internal files are formatted as URLs, so get just the
// path.
return StateResolver.stripFileProtocol_(scriptUrl);
}

resolveRelativePath_(frame: inspector.Debugger.CallFrame): string {
Expand All @@ -298,7 +301,6 @@ class StateResolver {
}

isPathInCurrentWorkingDirectory_(path: string): boolean {
// return true;
return StateResolver.stripFileProtocol_(path).indexOf(
this.config.workingDirectory) === 0;
}
Expand Down
22 changes: 19 additions & 3 deletions src/agent/v8/inspector-debugapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,17 @@ import * as utils from '../util/utils';
import * as debugapi from './debugapi';
import {V8Inspector} from './v8inspector';

/**
* An interface that describes options that set behavior when interacting with
* the V8 Inspector API.
*/
interface InspectorOptions {
/**
* Whether to add a 'file://' prefix to a URL when setting breakpoints.
*/
useWellFormattedUrl: boolean;
}

export class BreakpointData {
constructor(
public id: inspector.Debugger.BreakpointId,
Expand Down Expand Up @@ -58,6 +69,8 @@ export class InspectorDebugApi implements debugapi.DebugApi {
// stackdriver breakpoint id.
breakpointMapper: {[id: string]: stackdriver.BreakpointId[]} = {};
numBreakpoints = 0;
// Options for behavior when interfacing with the Inspector API.
private inspectorOptions: InspectorOptions;
v8Inspector: V8Inspector;
constructor(
logger: consoleLogLevel.Logger, config: ResolvedDebugAgentConfig,
Expand All @@ -80,6 +93,10 @@ export class InspectorDebugApi implements debugapi.DebugApi {
this.logger.error(error);
}
});
this.inspectorOptions = {
// Well-Formatted URL is required in Node 10.11.1+.
useWellFormattedUrl: utils.satisfies(process.version, '>10.11.0')
};
this.v8Inspector = new V8Inspector(this.session);
}

Expand Down Expand Up @@ -367,9 +384,8 @@ export class InspectorDebugApi implements debugapi.DebugApi {
let v8BreakpointId; // v8/inspector breakpoint id
if (!this.locationMapper[locationStr]) {
// The first time when a breakpoint was set to this location.

const url = utils.satisfies(process.version, '>=10.12.0') ?
'file://' + matchingScript :
const url = this.inspectorOptions.useWellFormattedUrl ?
`file://${matchingScript}` :
matchingScript;
const res = this.v8Inspector.setBreakpointByUrl({
lineNumber: line - 1,
Expand Down

0 comments on commit cefd4ae

Please sign in to comment.