[Bug] Exit code always 0

[dawsbot]

jscodeshift executions always exits with code 0, regardless of whether there were any errors. Here is my output, although this is generic to any situation where you have a non-zero errors count.

Processing 1 files...
Spawning 1 workers...
Sending 1 files to free worker...
 ERR /var/folders/tv/fdtq5cgn6zq4bj1_lzvrhl7h0000gn/T/07741760665/__testfixtures__/replace-require.input.js Transformation error
Error: arg "toReplace" required. Received undefined
    at req (/Users/dawsonbotsford/code/uber-codemods/src/replace-require.js:69:11)
    at replace (/Users/dawsonbotsford/code/uber-codemods/src/replace-require.js:74:3)
All done.
Results:
1 errors
0 unmodified
0 skipped
0 ok
Time elapsed: 0.688seconds

 ~/code/uber-codemods   regex-binding-bug ●  echo $?
0

Would the best practice instead be to set an exit code that's non-zero if there are any errors? In the current implementation, it's not intuitive to create an exec wrapper or the like in order to execute jscodeshift programmatically.

[fkling]

Yes, we should definitely return a proper exit code.

[macklinu]

Curious about how to go about making this change. These are the guts of the Runner.run() function:

jscodeshift/src/Runner.js

Lines 148 to 182 in 855f565

	if (/^http/.test(transformFile)) {
	usedRemoteScript = true;
	return new Promise((resolve, reject) => {
	// call the correct `http` or `https` implementation
	(transformFile.indexOf('https') !== 0 ? http : https).get(transformFile, (res) => {
	let contents = '';
	res
	.on('data', (d) => {
	contents += d.toString();
	})
	.on('end', () => {
	temp.open('jscodeshift', (err, info) => {
	if (err) return reject(err);
	fs.write(info.fd, contents, function (err) {
	if (err) return reject(err);
	fs.close(info.fd, function(err) {
	if (err) return reject(err);
	transform(info.path).then(resolve, reject);
	});
	});
	});
	})
	})
	.on('error', (e) => {
	reject(e);
	});
	});
	} else if (!fs.existsSync(transformFile)) {
	process.stderr.write(
	colors.white.bgRed('ERROR') + ' Transform file ' + transformFile + ' does not exist \n'
	);
	return;
	} else {
	return transform(transformFile);
	}

If this is updated to ensure all branches return a Promise that resolves or rejects as expected, would it be as simple as adding a .catch() statement calling process.exit(1) to the following block in the jscodeshift bin file?

jscodeshift/bin/jscodeshift.sh

Lines 110 to 114 in 855f565

	Runner.run(
	/^https?/.test(opts.transform) ? opts.transform : path.resolve(opts.transform),
	opts.path,
	opts
	);

[ycjcl868]

+1

[marcodejongh]

I have a PR up to fix this, but haven't had much luck getting reviews. Anyone know the best way to approach getting this change merged?