LOC-7325: stop uncatchable TypeError on empty binary output in Local.start

`start()` handles the binary's output inside an `execFile` callback. The
empty-output branch called back with 'No output received' but did not
return, so control fell through to `data['message']['message']` on
`data = {}`. That threw a TypeError, and because the throw happens inside
a callback invoked by node's internal exithandler, no try/catch around
`local.start(...)` could intercept it — it surfaced as an
uncaughtException in the host process.

Three paths reached the same unguarded deref:

- empty stdout and stderr (the reported one) — now returns after the
  callback, so it fires exactly once
- the terminal branch of the `error` handler, which also fell through
- any non-connected payload with no `message` key

Also guards `JSON.parse`: non-JSON output threw a SyntaxError from the
same uncatchable position, and is now reported through the callback with
the raw output attached as `extra`.

`startSync` shared the unguarded deref and now uses the same helper. Its
empty-output branch already returned, so it was not exposed to the
fall-through.

Adds regression tests driving start() with stub binaries for each output
shape, asserting the callback fires exactly once and nothing escapes as
an uncaughtException. They need no credentials or network. Three of the
four fail on master with the TypeError from the ticket.

Author: vivianludrick

lib/Local.js

@@ -57,7 +57,7 @@ function Local(){
       else
         return new LocalError('No output received');
       if(data['state'] != 'connected'){
-        return new LocalError(data['message']['message']);
+        return new LocalError(that.getErrorMessage(data));
       } else {
         that.pid = data['pid'];
         that.isProcessRunning = true;
@@ -112,19 +112,25 @@ function Local(){
             return;
           } else {
             callback(new LocalError(error.toString()));
+            return;
           }
         }
 
         var data = {};
-        if(stdout)
-          data = JSON.parse(stdout);
-        else if(stderr)
-          data = JSON.parse(stderr);
-        else
+        var output = stdout || stderr;
+        if(!output) {
           callback(new LocalError('No output received'));
+          return;
+        }
+        try {
+          data = JSON.parse(output);
+        } catch(parseError) {
+          callback(new LocalError('Invalid output received: ' + parseError.message, output));
+          return;
+        }
 
         if(data['state'] != 'connected'){
-          callback(new LocalError(data['message']['message']));
+          callback(new LocalError(that.getErrorMessage(data)));
         } else {
           that.pid = data['pid'];
           that.isProcessRunning = true;
@@ -134,6 +140,17 @@ function Local(){
     }, options['bs-host']);
   };
 
+  // The binary reports failures as {"state": "...", "message": {"message": "..."}},
+  // but not every non-connected payload carries a message key. Dereferencing it
+  // blindly throws, and inside the execFile callback that throw is an
+  // uncaughtException the caller cannot catch. See LOC-7325.
+  this.getErrorMessage = function(data){
+    var message = data && data['message'];
+    if(message && typeof message === 'object')
+      message = message['message'];
+    return message || 'Failed to start BrowserStack Local';
+  };
+
   this.isRunning = function(){
     return this.pid && running(this.pid) && this.isProcessRunning;
   };

test/local_start_output_handling.js

@@ -0,0 +1,104 @@
+var expect = require('expect.js'),
+    fs = require('fs'),
+    os = require('os'),
+    path = require('path'),
+    browserstack = require('../index');
+
+// Regression tests for LOC-7325.
+//
+// `Local.start` handles the binary's output inside an `execFile` callback. A
+// throw there is raised by node's internal exithandler, so no try/catch around
+// `start()` can intercept it — it surfaces as an uncaughtException and the
+// blast radius is set by the host process's exception policy. These tests drive
+// `start()` with stub binaries that reproduce each output shape and assert the
+// callback fires exactly once with an error, and that nothing throws.
+//
+// Stubs are shell scripts, so these are skipped on Windows.
+describe('Local.start output handling', function () {
+  var stubDir, bsLocal;
+
+  function stub(name, body) {
+    var stubPath = path.join(stubDir, name);
+    fs.writeFileSync(stubPath, '#!/bin/sh\n' + body + '\n', { mode: 0o755 });
+    return stubPath;
+  }
+
+  // Drives start() with the given stub and collects every callback invocation
+  // plus any uncaughtException raised out of the execFile callback.
+  function run(stubPath, done) {
+    var calls = [], uncaught = [];
+    var existing = process.listeners('uncaughtException');
+    process.removeAllListeners('uncaughtException');
+    process.on('uncaughtException', function (err) { uncaught.push(err); });
+
+    bsLocal.binaryPath = stubPath;
+    bsLocal.start({ key: 'dummy-key', localIdentifier: 'loc-7325' }, function (error) {
+      calls.push(error);
+    });
+
+    // Settle past the execFile callback before asserting, so a second
+    // (throwing) invocation would have happened by now if it were going to.
+    setTimeout(function () {
+      process.removeAllListeners('uncaughtException');
+      existing.forEach(function (listener) { process.on('uncaughtException', listener); });
+      done(calls, uncaught);
+    }, 1000);
+  }
+
+  before(function () {
+    stubDir = fs.mkdtempSync(path.join(os.tmpdir(), 'bs-local-7325-'));
+  });
+
+  beforeEach(function () {
+    bsLocal = new browserstack.Local();
+    // Keep the stubs from clobbering ./local.log in the repo root.
+    bsLocal.logfile = path.join(stubDir, 'local.log');
+  });
+
+  if (os.platform().match(/win32/i)) {
+    it.skip('skipped on Windows (stub binaries are shell scripts)');
+    return;
+  }
+
+  it('reports an error exactly once when the binary exits with no output', function (done) {
+    this.timeout(10000);
+    run(stub('empty-output.sh', 'exit 0'), function (calls, uncaught) {
+      expect(uncaught).to.eql([]);
+      expect(calls.length).to.equal(1);
+      expect(calls[0]).to.be.an('object');
+      expect(calls[0].message).to.equal('No output received');
+      done();
+    });
+  });
+
+  it('reports an error exactly once when the binary emits non-JSON output', function (done) {
+    this.timeout(10000);
+    run(stub('garbage-output.sh', 'echo "segmentation fault"; exit 0'), function (calls, uncaught) {
+      expect(uncaught).to.eql([]);
+      expect(calls.length).to.equal(1);
+      expect(calls[0].message).to.match(/^Invalid output received: /);
+      expect(calls[0].extra).to.match(/segmentation fault/);
+      done();
+    });
+  });
+
+  it('reports a fallback message when a non-connected payload has no message key', function (done) {
+    this.timeout(10000);
+    run(stub('no-message-key.sh', 'echo \'{"state":"disconnected"}\'; exit 0'), function (calls, uncaught) {
+      expect(uncaught).to.eql([]);
+      expect(calls.length).to.equal(1);
+      expect(calls[0].message).to.equal('Failed to start BrowserStack Local');
+      done();
+    });
+  });
+
+  it('surfaces the binary message when a non-connected payload carries one', function (done) {
+    this.timeout(10000);
+    run(stub('with-message.sh', 'echo \'{"state":"disconnected","message":{"message":"Invalid key"}}\'; exit 0'), function (calls, uncaught) {
+      expect(uncaught).to.eql([]);
+      expect(calls.length).to.equal(1);
+      expect(calls[0].message).to.equal('Invalid key');
+      done();
+    });
+  });
+});