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

Commit 9ecff8b

Browse files
gaofanmichaelgaofanmichael
authored
Change v8debugAPI clear to async. (#327)
* Change v8debugAPI clear to async. This PR aims to change the exisiting v8debug API clear function to async in order to prepare for the migration debug api from v8 to inspector, as inspector api is async. * resolve comments
1 parent fa36973 commit 9ecff8b

12 files changed

+246
-138
lines changed

‎package-lock.json‎

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
"@types/lodash": "^4.14.69",
3030
"@types/mocha": "^2.2.41",
3131
"@types/nock": "^8.2.1",
32-
"@types/node": "^7.0.18",
32+
"@types/node": "^8.0.27",
3333
"@types/request": "^2.0.0",
3434
"@types/semver": "^5.3.32",
3535
"@types/source-map": "^0.5.0",

‎src/agent/debuglet.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -664,7 +664,9 @@ export class Debuglet extends EventEmitter {
664664
// TODO: Address the case when `breakpoint.id` is `undefined`.
665665
delete this.activeBreakpointMap_[breakpoint.id as string];
666666
if (this.v8debug_) {
667-
this.v8debug_.clear(breakpoint);
667+
this.v8debug_.clear(breakpoint, (err) => {
668+
if (err) this.logger_.error(err);
669+
});
668670
}
669671
}
670672

‎src/agent/state.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import {DebugAgentConfig} from './config';
3232
// TODO: Determine if `ScopeType` should be named `scopeType`.
3333
// tslint:disable-next-line:variable-name
3434
const ScopeType = vm.runInDebugContext('ScopeType');
35-
const assert = debugAssert(process.env.CLOUD_DEBUG_ASSERTIONS);
35+
const assert = debugAssert(!!process.env.CLOUD_DEBUG_ASSERTIONS);
3636

3737
// Error message indices into the resolved variable table.
3838
const BUFFER_FULL_MESSAGE_INDEX = 0;

‎src/agent/v8debugapi.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ const messages = {
3737
'A script matching the source file was not found loaded on the debuggee',
3838
SOURCE_FILE_AMBIGUOUS: 'Multiple files match the path specified',
3939
V8_BREAKPOINT_ERROR: 'Unable to set breakpoint in v8',
40+
V8_BREAKPOINT_CLEAR_ERROR: 'Unable to clear breakpoint in v8',
4041
SYNTAX_ERROR_IN_CONDITION: 'Syntax error in condition: ',
4142
ERROR_EVALUATING_CONDITION: 'Error evaluating condition: ',
4243
ERROR_COMPILING_CONDITION: 'Error compiling condition.',
@@ -54,7 +55,8 @@ const MODULE_WRAP_PREFIX_LENGTH = require('module').wrap('☃').indexOf('☃');
5455

5556
export interface V8DebugApi {
5657
set: (breakpoint: apiTypes.Breakpoint, cb: (err: Error|null) => void) => void;
57-
clear: (breakpoint: apiTypes.Breakpoint) => boolean;
58+
clear:
59+
(breakpoint: apiTypes.Breakpoint, cb: (err: Error|null) => void) => void;
5860
wait:
5961
(breakpoint: apiTypes.Breakpoint,
6062
callback: (err?: Error) => void) => void;
@@ -189,13 +191,18 @@ export function create(
189191
}
190192
},
191193

192-
clear: function(breakpoint: apiTypes.Breakpoint): boolean {
194+
clear: function(
195+
breakpoint: apiTypes.Breakpoint, cb: (err: Error|null) => void): void {
193196
if (typeof breakpoint.id === 'undefined') {
194-
return false;
197+
return setErrorStatusAndCallback(
198+
cb, breakpoint, StatusMessage.BREAKPOINT_CONDITION,
199+
messages.V8_BREAKPOINT_CLEAR_ERROR);
195200
}
196201
const breakpointData = breakpoints[breakpoint.id];
197202
if (!breakpointData) {
198-
return false;
203+
return setErrorStatusAndCallback(
204+
cb, breakpoint, StatusMessage.BREAKPOINT_CONDITION,
205+
messages.V8_BREAKPOINT_CLEAR_ERROR);
199206
}
200207
const v8bp = breakpointData.v8Breakpoint;
201208

@@ -208,7 +215,9 @@ export function create(
208215
logger.info('deactivating v8 breakpoint listener');
209216
v8.setListener(null);
210217
}
211-
return true;
218+
return setImmediate(function() {
219+
cb(null);
220+
});
212221
},
213222

214223
/**

‎test/test-debuglet.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ nock.disableNetConnect();
4545

4646
const defaultConfig = extend(true, {}, DEFAULT_CONFIG, {logLevel: 0});
4747

48-
let oldGP: string;
48+
let oldGP: string|undefined;
4949

5050
declare type MetadataCallback = (err: Error|null, ob?: any, result?: string) => void;
5151

@@ -436,7 +436,7 @@ describe('Debuglet', function() {
436436

437437
it('should respect GCLOUD_DEBUG_LOGLEVEL', function(done) {
438438
process.env.GCLOUD_PROJECT = '11020304f2934';
439-
process.env.GCLOUD_DEBUG_LOGLEVEL = 3;
439+
process.env.GCLOUD_DEBUG_LOGLEVEL = '3';
440440
const debug = new Debug({credentials: fakeCredentials});
441441
const debuglet = new Debuglet(debug, defaultConfig);
442442

‎test/test-duplicate-expressions.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,10 @@ describe(__filename, function() {
8989
assert.equal(varTableIndicesSeen.indexOf(expr.varTableIndex as number), -1);
9090
varTableIndicesSeen.push(expr.varTableIndex as number);
9191
});
92-
api.clear(breakpointInFoo);
93-
done();
92+
api.clear(breakpointInFoo, function(err) {
93+
assert.ifError(err);
94+
done();
95+
});
9496
});
9597
process.nextTick(foo);
9698
});

‎test/test-duplicate-nested-expressions.ts‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,10 @@ describe(__filename, function() {
8888
locals[0],
8989
{name: 'a', value: 'test'}
9090
);
91-
api.clear(brk);
92-
done();
91+
api.clear(brk,function(err) {
92+
assert.ifError(err);
93+
done();
94+
})
9395
});
9496
process.nextTick(foo.bind(null, 'test'));
9597
});
@@ -115,8 +117,10 @@ describe(__filename, function() {
115117
locals[0],
116118
{name: 'a', value: '10'}
117119
);
118-
api.clear(brk);
119-
done();
120+
api.clear(brk, function(err) {
121+
assert.ifError(err);
122+
done();
123+
});
120124
});
121125
process.nextTick(foo.bind(null, 'test'));
122126
});
@@ -142,8 +146,10 @@ describe(__filename, function() {
142146
locals[0],
143147
{name: 'a', value: '11'}
144148
);
145-
api.clear(brk);
146-
done();
149+
api.clear(brk, function(err) {
150+
assert.ifError(err);
151+
done();
152+
});
147153
});
148154
process.nextTick(foo.bind(null, 'test'));
149155
});
@@ -173,8 +179,10 @@ describe(__filename, function() {
173179
locals[1],
174180
{name: 'a', value: 'true'}
175181
);
176-
api.clear(brk);
177-
done();
182+
api.clear(brk, function(err) {
183+
assert.ifError(err);
184+
done();
185+
});
178186
});
179187
process.nextTick(foo.bind(null, 'test'));
180188
});

‎test/test-fat-arrow.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import defaultConfig from '../src/agent/config';
2626
import * as SourceMapper from '../src/agent/sourcemapper';
2727
import * as scanner from '../src/agent/scanner';
2828

29-
process.env.GCLOUD_PROJECT = 0;
29+
process.env.GCLOUD_PROJECT = '0';
3030

3131
function stateIsClean(api: V8DebugApi): boolean {
3232
assert.equal(api.numBreakpoints_(), 0,
@@ -89,8 +89,10 @@ describe(__filename, function() {
8989
locals[0],
9090
{name: 'b', value: '1'}
9191
);
92-
api.clear(brk);
93-
done();
92+
api.clear(brk, function(err) {
93+
assert.ifError(err);
94+
done();
95+
});
9496
});
9597
process.nextTick(foo.bind(null, 'test'));
9698
});
@@ -115,8 +117,10 @@ describe(__filename, function() {
115117
locals[0],
116118
{name: 'b', value: '2'}
117119
);
118-
api.clear(brk);
119-
done();
120+
api.clear(brk, function(err) {
121+
assert.ifError(err);
122+
done();
123+
});
120124
});
121125
process.nextTick(foo.bind(null, 'test'));
122126
});

‎test/test-this-context.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,15 +85,17 @@ describe(__filename, function() {
8585
// TODO: Determine how to remove these casts to any.
8686
ctxMembers = (brk as any).variableTable.slice((brk as any).variableTable.length-1)[0]
8787
.members;
88-
assert.deepEqual(ctxMembers.length, 1,
88+
assert.deepEqual(ctxMembers.length, 1,
8989
'There should be one member in the context variable value');
9090
assert.deepEqual(ctxMembers[0], {name: 'a', value: '10'});
9191
assert.equal(args.length, 0, 'There should be zero arguments');
9292
assert.equal(locals.length, 2, 'There should be two locals');
9393
assert.deepEqual(locals[0], {name: 'b', value: '1'});
9494
assert.deepEqual(locals[1].name, 'context');
95-
api.clear(brk);
96-
done();
95+
api.clear(brk, function(err) {
96+
assert.ifError(err);
97+
done();
98+
});
9799
});
98100
process.nextTick(code.foo.bind({}, 1));
99101
});
@@ -115,8 +117,10 @@ describe(__filename, function() {
115117
assert.equal(args.length, 0, 'There should be zero arguments');
116118
assert.equal(locals.length, 1, 'There should be one local');
117119
assert.deepEqual(locals[0], {name: 'j', value: '1'});
118-
api.clear(brk);
119-
done();
120+
api.clear(brk, function(err) {
121+
assert.ifError(err);
122+
done();
123+
});
120124
});
121125
process.nextTick(code.bar.bind(null, 1));
122126
});

0 commit comments

Comments
 (0)