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

Commit b58b2f7

Browse files
DominicKramerDominicKramer
authored
Address Some TODOs (#324)
As part of the change to Typescript, many TODOs were added to the code for items to address. This PR addresses some of these TODOs.
1 parent 1a253ce commit b58b2f7

File tree

11 files changed

+119
-211
lines changed

11 files changed

+119
-211
lines changed

‎src/agent/config.ts‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@
1717
import {AuthenticationConfig} from '../types/common-types';
1818

1919
export interface DebugAgentConfig extends AuthenticationConfig {
20-
workingDirectory: string|null;
20+
workingDirectory?: string;
2121

2222
/**
2323
* A user specified way of identifying the service
2424
*/
25-
description: string|null;
25+
description?: string;
2626

2727
/**
2828
* Whether or not it is permitted to evaluate expressions.
@@ -44,17 +44,17 @@ export interface DebugAgentConfig extends AuthenticationConfig {
4444
/**
4545
* The service name.
4646
*/
47-
service: string | null;
47+
service?: string;
4848

4949
/**
5050
* The service version.
5151
*/
52-
version: string | null;
52+
version?: string;
5353

5454
/**
5555
* A unique deployment identifier. This is used internally only.
5656
*/
57-
minorVersion_: string | null;
57+
minorVersion_?: string;
5858
};
5959

6060
/**
@@ -65,7 +65,7 @@ export interface DebugAgentConfig extends AuthenticationConfig {
6565
* cases where the debug agent is unable to resolve breakpoint locations
6666
* unambiguously.
6767
*/
68-
appPathRelativeToRepository: string|null;
68+
appPathRelativeToRepository?: string;
6969

7070
/**
7171
* agent log level 0-disabled, 1-error, 2-warn, 3-info, 4-debug
@@ -170,14 +170,15 @@ const defaultConfig: DebugAgentConfig = {
170170
// FIXME(ofrobots): presently this is dependent what cwd() is at the time this
171171
// file is first required. We should make the default config static.
172172
workingDirectory: process.cwd(),
173-
description: null,
173+
description: undefined,
174174
allowExpressions: false,
175175

176176
// FIXME(ofrobots): today we prioritize GAE_MODULE_NAME/GAE_MODULE_VERSION
177177
// over the user specified config. We should reverse that.
178-
serviceContext: {service: null, version: null, minorVersion_: null},
178+
serviceContext:
179+
{service: undefined, version: undefined, minorVersion_: undefined},
179180

180-
appPathRelativeToRepository: null,
181+
appPathRelativeToRepository: undefined,
181182
logLevel: 1,
182183
breakpointUpdateIntervalSec: 10,
183184
breakpointExpirationSec: 60 * 60 * 24, // 24 hours

‎src/agent/debuglet.ts‎

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -108,22 +108,19 @@ const formatBreakpoints = function(
108108
};
109109

110110
export class Debuglet extends EventEmitter {
111-
// TODO: Determine how to update the tests so that this can be private.
112-
config_: DebugAgentConfig;
113111
private debug_: Debug;
114112
private v8debug_: V8DebugApi|null;
115113
private running_: boolean;
116114
private project_: string|null;
117-
// TODO: Determine how to update the tests so that this can be private.
115+
private debugletApi_: Controller;
116+
private completedBreakpointMap_: {[key: string]: boolean};
117+
118+
// Exposed for testing
119+
config_: DebugAgentConfig;
118120
fetcherActive_: boolean;
119-
// TODO: Determine how to update the tests so that this can be private.
120121
logger_: Logger;
121-
private debugletApi_: Controller;
122-
// TODO: Determine how to update the tests so that this can be private.
123122
debuggee_: Debuggee|null;
124-
// TODO: Determine how to update the tests so that this can be private.
125123
activeBreakpointMap_: {[key: string]: Breakpoint};
126-
private completedBreakpointMap_: {[key: string]: boolean};
127124

128125
/**
129126
* @param {Debug} debug - A Debug instance.
@@ -241,17 +238,18 @@ export class Debuglet extends EventEmitter {
241238

242239
const jsStats = fileStats.selectStats(/.js$/);
243240
const mapFiles = fileStats.selectFiles(/.map$/, process.cwd());
244-
SourceMapper.create(mapFiles, async function(err3, mapper) {
241+
SourceMapper.create(mapFiles, async function(err3, sourcemapper) {
245242
if (err3) {
246243
that.logger_.error('Error processing the sourcemaps.', err3);
247244
that.emit('initError', err3);
248245
return;
249246
}
250247

251-
that.v8debug_ = v8debugapi.create(
252-
// TODO: Handle the case where `mapper` is `undefined`.
253-
that.logger_, that.config_, jsStats,
254-
mapper as SourceMapper.SourceMapper);
248+
// At this point err3 being falsy implies sourcemapper is defined
249+
const mapper = sourcemapper as SourceMapper.SourceMapper;
250+
251+
that.v8debug_ =
252+
v8debugapi.create(that.logger_, that.config_, jsStats, mapper);
255253

256254
id = id || fileStats.hash;
257255

@@ -278,7 +276,7 @@ export class Debuglet extends EventEmitter {
278276
that.config_.serviceContext = {
279277
service: clusterName,
280278
version: 'unversioned',
281-
minorVersion_: null
279+
minorVersion_: undefined
282280
};
283281
} catch (err) {
284282
/* we are not running on GKE - Ignore error. */
@@ -308,7 +306,7 @@ export class Debuglet extends EventEmitter {
308306
that.debuggee_ = Debuglet.createDebuggee(
309307
// TODO: Address the case when `id` is `undefined`.
310308
project, id as string, that.config_.serviceContext, sourceContext,
311-
that.config_.description, null, onGCP);
309+
onGCP, that.config_.description, undefined);
312310
that.scheduleRegistration_(0 /* immediately */);
313311
that.emit('started');
314312
});
@@ -321,13 +319,11 @@ export class Debuglet extends EventEmitter {
321319
*/
322320
// TODO: Determine the type of sourceContext
323321
static createDebuggee(
324-
projectId: string, uid: string, serviceContext: {
325-
service: string | null,
326-
version: string|null,
327-
minorVersion_: string|null
328-
},
329-
sourceContext: {[key: string]: string}, description: string|null,
330-
errorMessage: string|null, onGCP: boolean): Debuggee {
322+
projectId: string, uid: string,
323+
serviceContext:
324+
{service?: string, version?: string, minorVersion_?: string},
325+
sourceContext: {[key: string]: string}, onGCP: boolean,
326+
description?: string, errorMessage?: string): Debuggee {
331327
const cwd = process.cwd();
332328
const mainScript = path.relative(cwd, process.argv[1]);
333329

@@ -378,7 +374,7 @@ export class Debuglet extends EventEmitter {
378374

379375
const statusMessage = errorMessage ?
380376
new StatusMessage(StatusMessage.UNSPECIFIED, errorMessage, true) :
381-
null;
377+
undefined;
382378

383379
const properties = {
384380
project: projectId,

‎src/agent/state.ts‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -354,20 +354,6 @@ class StateResolver {
354354
};
355355
}
356356

357-
// TODO: This method doesn't appear to be used. Determine if it can be
358-
// removed.
359-
extractArgumentsList_(frame: v8Types.FrameDetails): apiTypes.Variable[] {
360-
const args: apiTypes.Variable[] = [];
361-
for (let i = 0; i < frame.argumentCount(); i++) {
362-
// Don't resolve unnamed arguments.
363-
if (!frame.argumentName(i)) {
364-
continue;
365-
}
366-
args.push({name: frame.argumentName(i), value: frame.argumentValue(i)});
367-
}
368-
return args;
369-
}
370-
371357
/**
372358
* Iterates and returns variable information for all scopes (excluding global)
373359
* in a given frame. FrameMirrors should return their scope object list with

0 commit comments

Comments
 (0)