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

Commit

Permalink
Address Some TODOs (#324)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
DominicKramer authored Aug 9, 2017
1 parent 1a253ce commit b58b2f7
Show file tree
Hide file tree
Showing 11 changed files with 119 additions and 211 deletions.
19 changes: 10 additions & 9 deletions src/agent/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@
import {AuthenticationConfig} from '../types/common-types';

export interface DebugAgentConfig extends AuthenticationConfig {
workingDirectory: string|null;
workingDirectory?: string;

/**
* A user specified way of identifying the service
*/
description: string|null;
description?: string;

/**
* Whether or not it is permitted to evaluate expressions.
Expand All @@ -44,17 +44,17 @@ export interface DebugAgentConfig extends AuthenticationConfig {
/**
* The service name.
*/
service: string | null;
service?: string;

/**
* The service version.
*/
version: string | null;
version?: string;

/**
* A unique deployment identifier. This is used internally only.
*/
minorVersion_: string | null;
minorVersion_?: string;
};

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

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

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

appPathRelativeToRepository: null,
appPathRelativeToRepository: undefined,
logLevel: 1,
breakpointUpdateIntervalSec: 10,
breakpointExpirationSec: 60 * 60 * 24, // 24 hours
Expand Down
42 changes: 19 additions & 23 deletions src/agent/debuglet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,22 +108,19 @@ const formatBreakpoints = function(
};

export class Debuglet extends EventEmitter {
// TODO: Determine how to update the tests so that this can be private.
config_: DebugAgentConfig;
private debug_: Debug;
private v8debug_: V8DebugApi|null;
private running_: boolean;
private project_: string|null;
// TODO: Determine how to update the tests so that this can be private.
private debugletApi_: Controller;
private completedBreakpointMap_: {[key: string]: boolean};

// Exposed for testing
config_: DebugAgentConfig;
fetcherActive_: boolean;
// TODO: Determine how to update the tests so that this can be private.
logger_: Logger;
private debugletApi_: Controller;
// TODO: Determine how to update the tests so that this can be private.
debuggee_: Debuggee|null;
// TODO: Determine how to update the tests so that this can be private.
activeBreakpointMap_: {[key: string]: Breakpoint};
private completedBreakpointMap_: {[key: string]: boolean};

/**
* @param {Debug} debug - A Debug instance.
Expand Down Expand Up @@ -241,17 +238,18 @@ export class Debuglet extends EventEmitter {

const jsStats = fileStats.selectStats(/.js$/);
const mapFiles = fileStats.selectFiles(/.map$/, process.cwd());
SourceMapper.create(mapFiles, async function(err3, mapper) {
SourceMapper.create(mapFiles, async function(err3, sourcemapper) {
if (err3) {
that.logger_.error('Error processing the sourcemaps.', err3);
that.emit('initError', err3);
return;
}

that.v8debug_ = v8debugapi.create(
// TODO: Handle the case where `mapper` is `undefined`.
that.logger_, that.config_, jsStats,
mapper as SourceMapper.SourceMapper);
// At this point err3 being falsy implies sourcemapper is defined
const mapper = sourcemapper as SourceMapper.SourceMapper;

that.v8debug_ =
v8debugapi.create(that.logger_, that.config_, jsStats, mapper);

id = id || fileStats.hash;

Expand All @@ -278,7 +276,7 @@ export class Debuglet extends EventEmitter {
that.config_.serviceContext = {
service: clusterName,
version: 'unversioned',
minorVersion_: null
minorVersion_: undefined
};
} catch (err) {
/* we are not running on GKE - Ignore error. */
Expand Down Expand Up @@ -308,7 +306,7 @@ export class Debuglet extends EventEmitter {
that.debuggee_ = Debuglet.createDebuggee(
// TODO: Address the case when `id` is `undefined`.
project, id as string, that.config_.serviceContext, sourceContext,
that.config_.description, null, onGCP);
onGCP, that.config_.description, undefined);
that.scheduleRegistration_(0 /* immediately */);
that.emit('started');
});
Expand All @@ -321,13 +319,11 @@ export class Debuglet extends EventEmitter {
*/
// TODO: Determine the type of sourceContext
static createDebuggee(
projectId: string, uid: string, serviceContext: {
service: string | null,
version: string|null,
minorVersion_: string|null
},
sourceContext: {[key: string]: string}, description: string|null,
errorMessage: string|null, onGCP: boolean): Debuggee {
projectId: string, uid: string,
serviceContext:
{service?: string, version?: string, minorVersion_?: string},
sourceContext: {[key: string]: string}, onGCP: boolean,
description?: string, errorMessage?: string): Debuggee {
const cwd = process.cwd();
const mainScript = path.relative(cwd, process.argv[1]);

Expand Down Expand Up @@ -378,7 +374,7 @@ export class Debuglet extends EventEmitter {

const statusMessage = errorMessage ?
new StatusMessage(StatusMessage.UNSPECIFIED, errorMessage, true) :
null;
undefined;

const properties = {
project: projectId,
Expand Down
14 changes: 0 additions & 14 deletions src/agent/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -354,20 +354,6 @@ class StateResolver {
};
}

// TODO: This method doesn't appear to be used. Determine if it can be
// removed.
extractArgumentsList_(frame: v8Types.FrameDetails): apiTypes.Variable[] {
const args: apiTypes.Variable[] = [];
for (let i = 0; i < frame.argumentCount(); i++) {
// Don't resolve unnamed arguments.
if (!frame.argumentName(i)) {
continue;
}
args.push({name: frame.argumentName(i), value: frame.argumentValue(i)});
}
return args;
}

/**
* Iterates and returns variable information for all scopes (excluding global)
* in a given frame. FrameMirrors should return their scope object list with
Expand Down
Loading

0 comments on commit b58b2f7

Please sign in to comment.