fix: allow snapshots in shorter transpiled files (#513)

If a source file had `n` lines and was transpiled to a JavaScript
file with `m` lines with `m < n`, and one tried to set a snapshot
on a line between `m+1` and `n` in the original source file,
setting the snapshot would incorrectly fail with an error stating
that the requested line was beyond the size of the file.

Fixes: #464

src/agent/v8/inspector-debugapi.ts

@@ -308,21 +308,6 @@ export class InspectorDebugApi implements debugapi.DebugApi {
           utils.messages.SOURCE_FILE_AMBIGUOUS);
     }
 
-    // TODO: Address the case where `breakpoint.location` is `null`.
-    // TODO: Address the case where `fileStats[matchingScript]` is `null`.
-    if ((breakpoint.location as stackdriver.SourceLocation).line >=
-        (this.fileStats[matchingScript] as FileStats).lines) {
-      return utils.setErrorStatusAndCallback(
-          cb, breakpoint, StatusMessage.BREAKPOINT_SOURCE_LOCATION,
-          utils.messages.INVALID_LINE_NUMBER + matchingScript + ':' +
-              (breakpoint.location as stackdriver.SourceLocation).line +
-              '. Loaded script contained ' +
-              (this.fileStats[matchingScript] as FileStats).lines +
-              ' lines. Please ensure' +
-              ' that the snapshot was set in the same code version as the' +
-              ' deployed source.');
-    }
-
     // The breakpoint protobuf message presently doesn't have a column
     // property but it may have one in the future.
     // TODO: Address the case where `breakpoint.location` is `null`.
@@ -339,6 +324,19 @@ export class InspectorDebugApi implements debugapi.DebugApi {
       column += debugapi.MODULE_WRAP_PREFIX_LENGTH - 1;
     }
 
+    // TODO: Address the case where `breakpoint.location` is `null`.
+    // TODO: Address the case where `fileStats[matchingScript]` is `null`.
+    if (line >= (this.fileStats[matchingScript] as FileStats).lines) {
+      return utils.setErrorStatusAndCallback(
+          cb, breakpoint, StatusMessage.BREAKPOINT_SOURCE_LOCATION,
+          utils.messages.INVALID_LINE_NUMBER + matchingScript + ':' + line +
+              '. Loaded script contained ' +
+              (this.fileStats[matchingScript] as FileStats).lines +
+              ' lines. Please ensure' +
+              ' that the snapshot was set in the same code version as the' +
+              ' deployed source.');
+    }
+
     const result =
         this.setAndStoreBreakpoint(breakpoint, line, column, matchingScript);
     if (!result) {

src/agent/v8/legacy-debugapi.ts

@@ -290,19 +290,6 @@ export class V8DebugApi implements debugapi.DebugApi {
           utils.messages.SOURCE_FILE_AMBIGUOUS);
     }
 
-    // TODO: Address the case where `fileStats[matchingScript]` is `null`.
-    if ((breakpoint.location as stackdriver.SourceLocation).line >=
-        (this.fileStats[matchingScript] as FileStats).lines) {
-      return utils.setErrorStatusAndCallback(
-          cb, breakpoint, StatusMessage.BREAKPOINT_SOURCE_LOCATION,
-          utils.messages.INVALID_LINE_NUMBER + matchingScript + ':' +
-              (breakpoint.location as stackdriver.SourceLocation).line +
-              '. Loaded script contained ' +
-              (this.fileStats[matchingScript] as FileStats).lines +
-              ' lines. Please ensure' +
-              ' that the snapshot was set in the same code version as the' +
-              ' deployed source.');
-    }
     // The breakpoint protobuf message presently doesn't have a column property
     // but it may have one in the future.
     // TODO: Address the case where `breakpoint.location` is `null`.
@@ -320,6 +307,18 @@ export class V8DebugApi implements debugapi.DebugApi {
       column += debugapi.MODULE_WRAP_PREFIX_LENGTH - 1;
     }
 
+    // TODO: Address the case where `fileStats[matchingScript]` is `null`.
+    if (line >= (this.fileStats[matchingScript] as FileStats).lines) {
+      return utils.setErrorStatusAndCallback(
+          cb, breakpoint, StatusMessage.BREAKPOINT_SOURCE_LOCATION,
+          utils.messages.INVALID_LINE_NUMBER + matchingScript + ':' + line +
+              '. Loaded script contained ' +
+              (this.fileStats[matchingScript] as FileStats).lines +
+              ' lines. Please ensure' +
+              ' that the snapshot was set in the same code version as the' +
+              ' deployed source.');
+    }
+
     const v8bp = this.setByRegExp(matchingScript, line, column);
     if (!v8bp) {
       return utils.setErrorStatusAndCallback(

test/fixtures/transpiled-shorter/build.sh

@@ -0,0 +1,4 @@
+#!/usr/bin/env bash
+
+# Install 'coffee' using 'npm install -g coffeescript'
+coffee --map --output . in.coffee

test/fixtures/transpiled-shorter/in.coffee

@@ -0,0 +1,80 @@
+
+# This file has extra blank lines so that the
+# transpiled file has fewer lines than this file.
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+someFunction = () ->
+  nums = [1, 2, 3, 4, 5]
+  square = (x) -> x*x;
+  allSquares = (square x for x in nums)
+
+module.exports = someFunction
+
+#
+# Copyright 2018 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the 'License');
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an 'AS IS' BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#

test/test-v8debugapi.ts

@@ -708,6 +708,39 @@ describe('v8debugapi', () => {
          });
        });
 
+    it('should hit breakpoints in shorter transpiled files', (done) => {
+      const someFunction = require('./fixtures/transpiled-shorter/in.js');
+      const bp: stackdriver.Breakpoint = {
+        id: 'fake-id-shorter-transpiled',
+        location: {
+          path: path.join(
+              'build', 'test', 'fixtures', 'transpiled-shorter', 'in.coffee'),
+          // Note: The file `./fixtures/transpiled-shorter/in.js` was generated
+          // from
+          //       transpiling `./fixtures/transpiled-shorter/in.coffee`, and
+          //       `in.js` only has 44 lines.  The purpose of this test is to
+          //       ensure that if the line number specified below is larger than
+          //       the number of lines in `in.js` but less than or equal to the
+          //       number of lines in `in.coffee`, the breakpoint will still hit
+          //       correctly.
+          line: 60
+        }
+      } as stackdriver.Breakpoint;
+      api.set(bp, (err1) => {
+        assert.ifError(err1);
+        api.wait(bp, (err2) => {
+          assert.ifError(err2);
+          assert(bp.location);
+          assert.strictEqual(bp.location!.line, 60);
+          api.clear(bp, (err3) => {
+            assert.ifError(err3);
+            done();
+          });
+        });
+        process.nextTick(someFunction);
+      });
+    });
+
     it('should work with multiply hit breakpoints', (done) => {
       const oldWarn = logger.warn;
       let logCount = 0;