Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vivianludrick wants to merge 4 commits into
base: master
Choose a base branch
from
+475 −49
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
909dc67
LOC-7325: stop uncatchable TypeError on empty binary output in Local.…
vivianludrick Aug 21, 2026
1db2f0b
LOC-7325: close remaining escape paths in binary output handling
vivianludrick Aug 31, 2026
7cbe054
LOC-7325: harden adjacent callback paths from second review round
vivianludrick Aug 31, 2026
b9fe94e
LOC-7325: third review round — download-flow hardening + payload sema…
vivianludrick Aug 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,15 @@ declare module "browserstack-local" {
[key: string]: string | boolean;
}

interface LocalError extends Error {
/** Raw binary output (truncated to 1KB) attached when the output could not be parsed. */
extra?: string;
}

class Local {
start(options: Partial<Options>, callback: (error?: Error) => void): void;
start(options: Partial<Options>, callback: (error?: LocalError) => void): void;
startSync(options: Partial<Options>): LocalError | undefined;
isRunning(): boolean;
stop(callback: () => void): void;
stop(callback: (error?: LocalError) => void): void;
}
}
217 changes: 173 additions & 44 deletions lib/Local.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ function Local(){
this.windows = os.platform().match(/mswin|msys|mingw|cygwin|bccwin|wince|emc|win32/i);
this.pid = undefined;
this.isProcessRunning = false;
this.userProvidedBinaryPath = false;
this.retriesLeft = 9;
this.key = process.env.BROWSERSTACK_ACCESS_KEY;
this.logfile = this.sanitizePath(path.join(process.cwd(), 'local.log'));
Expand Down Expand Up @@ -48,16 +49,21 @@ function Local(){
}
try{
const obj = childProcess.spawnSync(that.binaryPath, that.getBinaryArgs());
if(obj.error)
throw obj.error;
this.tunnel = {pid: obj.pid};
var data = {};
if(obj.stdout.length > 0)
data = JSON.parse(obj.stdout);
else if(obj.stderr.length > 0)
data = JSON.parse(obj.stderr);
else
return new LocalError('No output received');
var result = that.parseBinaryOutput(obj.stdout, obj.stderr);
if(result.error) {
if(result.invalidOutput) {
// A cached binary that runs but prints garbage may be corrupt on
// disk; evict it so the next start re-downloads a fresh copy.
that.evictDownloadedBinary();
}
return result.error;
}
var data = result.data;
if(data['state'] != 'connected'){
return new LocalError(data['message']['message']);
return new LocalError(that.getErrorMessage(data));
} else {
that.pid = data['pid'];
that.isProcessRunning = true;
Expand All @@ -67,12 +73,7 @@ function Local(){
const binaryDownloadErrorMessage = `Error while trying to execute binary: ${util.format(error)}`;
console.error(binaryDownloadErrorMessage);
if(that.retriesLeft > 0) {
console.log('Retrying Binary Download. Retries Left', that.retriesLeft);
that.retriesLeft -= 1;
fs.unlinkSync(that.binaryPath);
delete(that.binaryPath);
process.env.BINARY_DOWNLOAD_ERROR_MESSAGE = binaryDownloadErrorMessage;
process.env.BINARY_DOWNLOAD_FALLBACK_ENABLED = true;
that.prepareBinaryRetry(binaryDownloadErrorMessage);
return that.startSync(options);
} else {
throw new LocalError(error.toString());
Expand All @@ -89,6 +90,12 @@ function Local(){
return callback();

this.getBinaryPath(function(binaryPath){
if(!binaryPath){
// Terminal download failure signalled by LocalBinary (falsy path);
// retrying here would only re-run the whole download cascade.
var downloadErrorMessage = (that.binary && that.binary.downloadErrorMessage) || 'Unable to download BrowserStack Local binary';
return callback(new LocalError(downloadErrorMessage));
}
that.binaryPath = binaryPath;
try {
fs.writeFileSync(that.logfile, '');
Expand All @@ -98,50 +105,170 @@ function Local(){

that.opcode = 'start';
that.tunnel = childProcess.execFile(that.binaryPath, that.getBinaryArgs(), function(error, stdout, stderr){
if(error) {
const binaryDownloadErrorMessage = `Error while trying to execute binary: ${util.format(error)}`;
console.error(binaryDownloadErrorMessage);
if(that.retriesLeft > 0) {
console.log('Retrying Binary Download. Retries Left', that.retriesLeft);
that.retriesLeft -= 1;
fs.unlinkSync(that.binaryPath);
delete(that.binaryPath);
process.env.BINARY_DOWNLOAD_ERROR_MESSAGE = binaryDownloadErrorMessage;
process.env.BINARY_DOWNLOAD_FALLBACK_ENABLED = true;
that.start(options, callback);
// Everything below runs inside node's exithandler: a throw here is an
// uncaughtException the caller cannot catch, and a fall-through can
// invoke the callback twice. Guard both structurally. See LOC-7325.
var callbackCalled = false;
var safeCallback = function(err){
if(callbackCalled) return;
callbackCalled = true;
callback(err);
};
try {
var result = that.parseBinaryOutput(stdout, stderr);
if(error) {
const binaryDownloadErrorMessage = `Error while trying to execute binary: ${util.format(error)}`;
console.error(binaryDownloadErrorMessage);
if(result.data) {
// The binary executed and reported a structured result, so the
// failure is not a corrupt download — retrying (delete +
// re-download) cannot help; fail fast with the richer diagnostic
// instead of burning the retry budget first.
if(result.data['state'] == 'connected' && result.data['pid']) {
// The daemon came up even though the foreground process exited
// non-zero; treat it as success so isRunning()/stop() agree
// with reality (startSync likewise ignores the exit status
// when the payload says connected).
that.pid = result.data['pid'];
that.isProcessRunning = true;
safeCallback();
return;
}
var payloadMessage = that.extractErrorMessage(result.data);
if(payloadMessage) {
safeCallback(new LocalError(payloadMessage));
return;
}
// Payload parsed but carries no usable message: surface the
// exec error and keep the raw payload as extra.
var rawPayload = (stdout && stdout.length > 0) ? stdout : stderr;
safeCallback(new LocalError(error.toString(), that.truncateForExtra(rawPayload)));
return;
}
if(that.retriesLeft > 0) {
that.prepareBinaryRetry(binaryDownloadErrorMessage);
that.start(options, callback);
return;
}
// Keep any raw (non-JSON) output as extra — it usually holds
// the crash text.
safeCallback(new LocalError(error.toString(), result.error && result.error.extra));
return;
} else {
callback(new LocalError(error.toString()));
}
}

var data = {};
if(stdout)
data = JSON.parse(stdout);
else if(stderr)
data = JSON.parse(stderr);
else
callback(new LocalError('No output received'));

if(data['state'] != 'connected'){
callback(new LocalError(data['message']['message']));
} else {
that.pid = data['pid'];
that.isProcessRunning = true;
callback();
if(result.error) {
if(result.invalidOutput) {
// A cached binary that runs but prints garbage may be corrupt
// on disk; evict it so the next start re-downloads a fresh copy.
that.evictDownloadedBinary();
}
safeCallback(result.error);
return;
}
if(result.data['state'] != 'connected'){
safeCallback(new LocalError(that.getErrorMessage(result.data)));
} else {
that.pid = result.data['pid'];
that.isProcessRunning = true;
safeCallback();
}
} catch(err) {
if(callbackCalled) throw err; // the caller's own callback threw — theirs to handle
safeCallback(new LocalError(err.toString()));
}
});
}, options['bs-host']);
};

// The binary reports failures as {"state": "...", "message": {"message": "..."}},
// but not every non-connected payload carries a message key, and the value is
// not guaranteed to be a string. Dereferencing it blindly throws, and inside
// the execFile callback that throw is an uncaughtException the caller cannot
// catch; a non-string message crashes consumers doing error.message.match().
// See LOC-7325.
this.extractErrorMessage = function(data){
var message = data && data['message'];
if(message && typeof message === 'object')
message = message['message'];
if(typeof message === 'string' && message.length > 0)
return message;
return null;
};

this.getErrorMessage = function(data){
return this.extractErrorMessage(data) || 'Failed to start BrowserStack Local';
};

// Shared retry bookkeeping for start and startSync: drop the (possibly
// corrupt) binary so the next attempt re-downloads it, and record the
// failure for the fallback download source.
this.prepareBinaryRetry = function(binaryDownloadErrorMessage){
console.log('Retrying Binary Download. Retries Left', this.retriesLeft);
this.retriesLeft -= 1;
try {
fs.unlinkSync(this.binaryPath);
} catch(unlinkError) {
// The binary may already be gone (a prior retry or a concurrent
// instance); the retry only needs the path cleared for re-download.
console.error('Could not delete binary: ', unlinkError.message);
}
delete(this.binaryPath);
process.env.BINARY_DOWNLOAD_ERROR_MESSAGE = binaryDownloadErrorMessage;
process.env.BINARY_DOWNLOAD_FALLBACK_ENABLED = true;
};

// Raw binary output can be up to execFile's 1MB maxBuffer; truncate before
// attaching it to an error so serializers don't dump the whole buffer.
this.truncateForExtra = function(output){
output = String(output);
if(output.length <= 1024)
return output;
return output.slice(0, 1024) + ' [truncated ' + (output.length - 1024) + ' bytes]';
};

// Shared by start and startSync so both classify binary output the same way.
// Returns {data} for a parsed JSON object, {error} otherwise — including
// output that parses to null or a non-object ('null' is valid JSON, so a
// parse guard alone does not cover it).
this.parseBinaryOutput = function(stdout, stderr){
var output = (stdout && stdout.length > 0) ? stdout : stderr;
if(!output || output.length === 0)
return { error: new LocalError('No output received') };
var data;
try {
data = JSON.parse(output);
} catch(parseError) {
return { error: new LocalError('Invalid output received: ' + parseError.message, this.truncateForExtra(output)), invalidOutput: true };
}
if(!data || typeof data !== 'object')
return { error: new LocalError('Invalid output received: expected a JSON object', this.truncateForExtra(output)), invalidOutput: true };
return { data: data };
};

// A binary that executes but prints unparseable output may be corrupt on
// disk; evicting it lets the next start() download a fresh copy instead of
// failing identically forever. Binaries supplied by the user via the
// `binarypath` option are never evicted.
this.evictDownloadedBinary = function(){
if(this.userProvidedBinaryPath || !this.binaryPath) return;
try {
fs.unlinkSync(this.binaryPath);
} catch(unlinkError) {
console.error('Could not delete binary: ', unlinkError.message);
}
delete(this.binaryPath);
};

this.isRunning = function(){
return this.pid && running(this.pid) && this.isProcessRunning;
};

this.stop = function (callback) {
if(!this.pid) return callback();
this.killAllProcesses(function(error){
if(error) callback(new LocalError(error.toString()));
// Without the return, a treeKill error fired the callback twice:
// once with the error, then once with undefined.
if(error) return callback(new LocalError(error.toString()));
callback();
});
};
Expand Down Expand Up @@ -241,8 +368,10 @@ function Local(){
break;

case 'binarypath':
if(value)
if(value){
this.binaryPath = value;
this.userProvidedBinaryPath = true;
}
break;

default:
Expand Down
Loading
Loading