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.

src/agent/config.ts

@@ -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.
@@ -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;
   };
 
   /**
@@ -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
@@ -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

src/agent/debuglet.ts

@@ -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.
@@ -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;
 
@@ -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. */
@@ -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');
       });
@@ -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]);
 
@@ -378,7 +374,7 @@ export class Debuglet extends EventEmitter {
 
     const statusMessage = errorMessage ?
         new StatusMessage(StatusMessage.UNSPECIFIED, errorMessage, true) :
-        null;
+        undefined;
 
     const properties = {
       project: projectId,

src/agent/state.ts

@@ -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