End the run as usual when the reader of a pipe stops early to close #839

devguide.md

@@ -23,7 +23,7 @@ All output goes through `src/output.ts`. Pick the function by who the message is
 | `printDone()` | At the end of a run, also when it is stopped | Ends stdout and stderr with a line break when `--nl` is set | |
 | `info(name)` | Unless `--quiet` | The name of an affected file. stdout, or stderr when stdout has content. | `a.md` |
 | `warn(msg)` | Unless `--quiet` | Something is likely a mistake, or there is nothing to do, but the run goes on. Starts with 🟡. Also added to `status.warnings`. | `0 files found` |
-| `error(msg, data)` | Unless `--quiet-total` | Something went wrong. Starts with ❌. The run goes on, unless `--halt` is set: then a panic is thrown. Also added to `status.errors`, so the CLI gives the exit code 2. | `File not found: a.md` |
+| `error(msg, data)` | Unless `--quiet-total` | Something went wrong. Starts with ❌, and the data, like the file, is on its own line under the message. The run goes on, unless `--halt` is set: then a panic is thrown. Also added to `status.errors`, so the CLI gives the exit code 2. | `File not found`, and `a.md` on the next line |
 | `checkFailed(text)` | Unless `--quiet-total` | A check from `--expect-*` that is not met. An error, also in `status.checks`, so the CLI gives the exit code 1 when the failed checks are the only errors. | `Expected a diff, but nothing was changed` |
 | `panic(msg, data, help)` | Unless `--quiet-total` | Something went wrong and the run can not go on. Starts with ❌, ends with `No further processing.`, and throws a panic, so the CLI exits with 3. Also added to `status.errors`. | `No data piped into replacement` |
 | `chat(msg)` | `--verbose`, also with `--quiet`, but not with `--quiet-total` | The user: what happens to each file. Only ever to stderr. Also kept in `engine.steps` of the status report. | `Open sync: a.md`, `Nothing changed in: a.md` |

src/cli.ts

@@ -1,7 +1,7 @@
 // CLI interface for rexreplace
 
 import * as rexreplace from './engine.ts';
-import {outputConfig, panic} from './output.ts';
+import {outputConfig, panic, writeTo} from './output.ts';
 import {alias} from './alias.ts';
 
 // Make it visible when Bun is the runtime (like with rexreplace-bun) as the regex engine is then not the one from Node.
@@ -525,9 +525,9 @@ Z	z
 // The CLI is the only part that acts on the process.
 function printStatus(config, status) {
 	if (config.status) {
-		process.stdout.write('\n' + JSON.stringify(status, null, 2) + '\n');
+		writeTo(process.stdout, '\n' + JSON.stringify(status, null, 2) + '\n');
 	} else if (config.statusCompact) {
-		process.stdout.write(JSON.stringify(status) + '\n');
+		writeTo(process.stdout, JSON.stringify(status) + '\n');
 	}
 }
 
@@ -619,7 +619,7 @@ const unescape = (key, name) => {
 	} catch (e) {
 		// The engine is not running yet, so the panic is caught here
 		try {
-			panic('Invalid escape sequence in ' + name + ':', typed[key]);
+			panic('Invalid escape sequence in ' + name, typed[key]);
 		} catch (e) {
 			exitOnPanic(config, e);
 		}

src/engine.ts

@@ -52,7 +52,7 @@ const runtimeEngine = process.versions.bun ? 'JSC' : 'V8';
 
 function validateEncoding(encoding = 'utf8') {
 	if (!Buffer.isEncoding(encoding)) {
-		panic('Unknown encoding:', encoding);
+		panic('Unknown encoding', encoding);
 	}
 }
 
@@ -159,7 +159,7 @@ async function run(config) {
 			? namedGroups(config.regex).find(name => name.endsWith('_'))
 			: null;
 	if (endsWith_) {
-		panic('A named group can not end with _:', endsWith_);
+		panic('A named group can not end with _', endsWith_);
 	}
 
 	// What the regex engine is asked to do. With --replacement-js and --output-match it gets a function.
@@ -235,7 +235,7 @@ async function run(config) {
 		(config.replacementJs && re.runNumbers.test(config.replacementOri));
 
 	const files = config.files.filter(filepath =>
-		fs.existsSync(filepath) ? true : error('File not found:', filepath)
+		fs.existsSync(filepath) ? true : error('File not found', filepath)
 	);
 
 	if (config.rename || config.renamePath) {
@@ -703,7 +703,7 @@ function getFinalReplacement(replacement, conf: any) {
 		try {
 			return readReplacementFile(replacement, conf);
 		} catch (e) {
-			return panic('The replacement file can not be read:', pathAndReason(replacement, e));
+			return panic('The replacement file can not be read', pathAndReason(replacement, e));
 		}
 	}
 
@@ -750,7 +750,7 @@ function codeReplacement(file, config, text, numbers) {
 				return readReplacementFile(filepath, config);
 			} catch (e) {
 				config.unreadableFiles.add(filepath);
-				error('The replacement file can not be read:', pathAndReason(filepath, e));
+				error('The replacement file can not be read', pathAndReason(filepath, e));
 			}
 		}
 		return args[0];
@@ -1080,11 +1080,8 @@ function dynamicReplacement(
 			// With --verbose it also tells where the code failed, when that is in a file of the user, like a module loaded with r()
 			const at = _config_rr.verbose ? userCodeLocation(e) : '';
 			return panic(
-				'The code of --js failed for ' +
-					where +
-					'\n    ' +
-					thrownText(e) +
-					(at ? '\n    at ' + at : '')
+				'The code of --js failed for ' + where,
+				thrownText(e) + (at ? '\n    at ' + at : '')
 			);
 		}
 	};
@@ -1351,7 +1348,7 @@ function sortName(value) {
 		return 'natural';
 	}
 	if (!sorts.includes(name)) {
-		panic('--sort must be one of ' + sorts.join(', ') + ', not:', String(value));
+		panic('--sort must be one of ' + sorts.join(', '), String(value));
 	}
 	return name;
 }
@@ -1431,9 +1428,9 @@ function glob(patterns, globOptions, exclude = false) {
 		return globs.sync(patterns, globOptions);
 	} catch (e) {
 		if (exclude) {
-			return panic('The files to exclude can not be found:', e.message);
+			return panic('The files to exclude can not be found', e.message);
 		}
-		error('Not all paths could be accessed:', e.message);
+		error('Not all paths could be accessed', e.message);
 		return globs.sync(patterns, {...globOptions, suppressErrors: true});
 	}
 }

src/output.ts

@@ -23,9 +23,32 @@ export class Panic extends Error {
 	status = null;
 }
 
+// A pipe the reader has closed, like `| head`, can not take more. That is fine: nobody reads the rest.
+// Bun throws when it is written to, and Node gives an error event, so both are caught.
+const closed = new Set();
+const whenClosed = stream => e => {
+	if ('EPIPE' !== e?.code) {
+		throw e;
+	}
+	closed.add(stream);
+};
+process.stdout.on?.('error', whenClosed(process.stdout));
+process.stderr.on?.('error', whenClosed(process.stderr));
+
+export const writeTo = function (stream, text) {
+	if (closed.has(stream)) {
+		return;
+	}
+	try {
+		stream.write(text);
+	} catch (e) {
+		whenClosed(stream)(e);
+	}
+};
+
 // Everything is printed from here, so the last character of each stream is known
 const write = function (text, stream) {
-	stream.write(text);
+	writeTo(stream, text);
 	if (text) {
 		lastPrintedChar.set(stream, text.charCodeAt(text.length - 1));
 	}
@@ -104,10 +127,13 @@ const printError = function (text, stop) {
 	}
 };
 
+// The detail, like the file, goes on a line of its own under the message
+const errorText = (msg, data) => msg + (data ? '\n    ' + data : '');
+
 // The run can not go on, so it stops here.
 // The files in work are finished, and they can fail too. Only the first panic is told, the rest are only in the status.
 export const panic = function (msg = '', data = '', displayHelp = false) {
-	const text = msg + (data ? ' ' + data : '');
+	const text = errorText(msg, data);
 	config.statusInfo?.errors.push(text);
 	if (!config.stopped) {
 		config.stopped = true;
@@ -128,7 +154,7 @@ export const checkFailed = function (text) {
 
 // Something failed, and the run goes on. With --halt it is a panic.
 export const error = function (msg, data = '') {
-	const text = msg + (data ? ' ' + data : '');
+	const text = errorText(msg, data);
 	config.statusInfo?.errors.push(text);
 	// After a panic only the status is told about the files that are finished
 	if (!config.stopped) {

test/engine/encoding.test.js

@@ -50,7 +50,7 @@ describe('--encoding', () => {
 	// The error message, and not the stack trace from deep inside node, when an encoding is unknown (#690)
 	const unknownEncoding = {
 		out: '',
-		err: ' ❌ Unknown encoding: nope\n    No further processing.\n\n',
+		err: ' ❌ Unknown encoding\n    nope\n    No further processing.\n\n',
 		exit: 3,
 	};
 

test/engine/replacement-file.test.js

@@ -60,7 +60,7 @@ describe('--replacement-file', () => {
 		fs.writeFileSync('my.file', 'foo');
 		const result = await rr('foo', 'nope.txt', {includeGlob: ['my.file'], replacementFile: true});
 		expect(result.err).toStartWith(
-			' ❌ The replacement file can not be read: nope.txt (ENOENT: no such file or directory)'
+			' ❌ The replacement file can not be read\n    nope.txt (ENOENT: no such file or directory)'
 		);
 		expect(result.exit).toBe(3);
 		expect(fs.readFileSync('my.file', 'utf8')).toBe('foo');
@@ -70,7 +70,7 @@ describe('--replacement-file', () => {
 		const config = {pipedData: 'foo', replacementJs: true, replacementFile: true};
 		expect(await rr('o', '"nope.txt"', config)).toEqual({
 			out: 'foo',
-			err: ' ❌ The replacement file can not be read: nope.txt (ENOENT: no such file or directory)\n\n',
+			err: ' ❌ The replacement file can not be read\n    nope.txt (ENOENT: no such file or directory)\n\n',
 			exit: 2,
 		});
 	});

test/fix/691.test.js

@@ -77,7 +77,7 @@ describe('#691 replacement that used to crash the CLI', () => {
 		const result = await sh($`bun ${cli} o ${'\\x4'} my.file --output --no-content-delimiter-tail`);
 		expect(result.out).toBe('');
 		expect(result.err).toBe(
-			' ❌ Invalid escape sequence in replacement: \\x4\n    No further processing.\n\n'
+			' ❌ Invalid escape sequence in replacement\n    \\x4\n    No further processing.\n\n'
 		);
 		expect(result.exit).toBe(3);
 		expect(

test/fix/700.test.js

@@ -39,7 +39,7 @@ describe('#700 § is a full substitute for \\ in the replacement', () => {
 	test('an invalid escape sequence gives the same error as with a backslash', async () => {
 		expect(await replaceB('§x4')).toEqual({
 			out: '',
-			err: ' ❌ Invalid escape sequence in replacement: §x4\n    No further processing.\n\n',
+			err: ' ❌ Invalid escape sequence in replacement\n    §x4\n    No further processing.\n\n',
 			exit: 3,
 		});
 	});

test/fix/788.test.js

@@ -43,7 +43,7 @@ describe('#788 a path that can not be looked at', () => {
 			fs.writeFileSync('a.md', 'foo');
 			fs.writeFileSync('b.md', 'foo');
 			const result = await rr('foo', 'bar', config);
-			expect(result.err).toStartWith(' ❌ Not all paths could be accessed: ENOTDIR');
+			expect(result.err).toStartWith(' ❌ Not all paths could be accessed\n    ENOTDIR');
 			expect(result.exit).toBe(2);
 			expect(fs.readFileSync('b.md', 'utf8')).toBe('bar');
 		}
@@ -56,7 +56,7 @@ describe('#788 a path that can not be looked at', () => {
 		fs.chmodSync('locked', 0);
 		try {
 			const result = await rr('foo', 'bar', {includeGlob: ['**/*.md']});
-			expect(result.err).toStartWith(' ❌ Not all paths could be accessed: EACCES');
+			expect(result.err).toStartWith(' ❌ Not all paths could be accessed\n    EACCES');
 			expect(result.out).toBe('a.md\n');
 			expect(result.exit).toBe(2);
 			expect(fs.readFileSync('a.md', 'utf8')).toBe('bar');
@@ -75,7 +75,7 @@ describe('#788 a path that can not be looked at', () => {
 	test('is a panic in --exclude-glob, as the files to keep can not be known', async () => {
 		fs.writeFileSync('a.md', 'foo');
 		const result = await rr('foo', 'bar', {includeGlob: ['a.md'], excludeGlob: ['a.md/x']});
-		expect(result.err).toStartWith(' ❌ The files to exclude can not be found: ENOTDIR');
+		expect(result.err).toStartWith(' ❌ The files to exclude can not be found\n    ENOTDIR');
 		expect(result.err).toEndWith('\n    No further processing.\n\n');
 		expect(result.exit).toBe(3);
 		expect(fs.readFileSync('a.md', 'utf8')).toBe('foo');

test/fix/816.test.js

@@ -18,7 +18,7 @@ describe('#816 named groups with letters that are not ASCII', () => {
 
 	test('can not end with _ either', async () => {
 		const result = await rr('(?<año_>x)', 'rr_año', {pipedData: 'x', replacementJs: true});
-		expect(result.err).toStartWith(' ❌ A named group can not end with _: año_');
+		expect(result.err).toStartWith(' ❌ A named group can not end with _\n    año_');
 		expect(result.exit).toBe(3);
 	});
 

test/fix/818.test.js

@@ -28,7 +28,7 @@ describe('#818 a replacement file that can not be read', () => {
 		fs.mkdirSync('adir');
 		fs.writeFileSync('a.txt', 'X');
 		const result = await rr('X', filepath, {includeGlob: ['a.txt'], replacementFile: true});
-		expect(result.err).toStartWith(' ❌ The replacement file can not be read: ' + text);
+		expect(result.err).toStartWith(' ❌ The replacement file can not be read\n    ' + text);
 	});
 
 	test('with --js is told once in the run, not once for each file', async () => {
@@ -37,7 +37,7 @@ describe('#818 a replacement file that can not be read', () => {
 		const config = {includeGlob: ['*.txt'], replacementJs: true, replacementFile: true};
 		const status = await rrStatus('X', '"nope.txt"', config);
 		expect(status.errors).toEqual([
-			'The replacement file can not be read: nope.txt (ENOENT: no such file or directory)',
+			'The replacement file can not be read\n    nope.txt (ENOENT: no such file or directory)',
 		]);
 	});
 });

test/fix/819.test.js

@@ -42,31 +42,31 @@ describe('#819 a file deleted before --sort looks at it', () => {
 		async sort => {
 			const result = await sortWithGone(['gone.md'], {sort});
 			expect(result.out).toBe('z.md\na.md\n');
-			expect(result.err).toEndWith(' ❌ File not found: gone.md\n\n');
+			expect(result.err).toEndWith(' ❌ File not found\n    gone.md\n\n');
 			expect(result.exit).toBe(2);
 		}
 	);
 
 	test('is told about for each file, when more than one is deleted', async () => {
 		expect(await sortWithGone(['gone.md', 'gone2.md'], {sort: 'size'})).toEqual({
 			out: 'z.md\na.md\n',
-			err: ' ❌ File not found: gone.md\n\n ❌ File not found: gone2.md\n\n',
+			err: ' ❌ File not found\n    gone.md\n\n ❌ File not found\n    gone2.md\n\n',
 			exit: 2,
 		});
 	});
 
 	test('does not change the order of the others with --sort-reverse', async () => {
 		expect(await sortWithGone(['gone.md'], {sort: 'size', sortReverse: true})).toEqual({
 			out: 'a.md\nz.md\n',
-			err: ' ❌ File not found: gone.md\n\n',
+			err: ' ❌ File not found\n    gone.md\n\n',
 			exit: 2,
 		});
 	});
 
 	test('stops the run with --halt, before any file is changed', async () => {
 		expect(await sortWithGone(['gone.md'], {sort: 'size', halt: true})).toEqual({
 			out: '',
-			err: ' ❌ File not found: gone.md\n    No further processing.\n\n',
+			err: ' ❌ File not found\n    gone.md\n    No further processing.\n\n',
 			exit: 3,
 		});
 		expect(fs.readFileSync('a.md', 'utf8')).toBe('foo foo');

test/fix/825.test.js

@@ -48,7 +48,7 @@ describe('#825 a named group that ends with _', () => {
 			});
 			expect(result).toEqual({
 				out: '',
-				err: ` ❌ A named group can not end with _: ${name}\n    No further processing.\n\n`,
+				err: ` ❌ A named group can not end with _\n    ${name}\n    No further processing.\n\n`,
 				exit: 3,
 			});
 			expect(fs.readFileSync('a.md', 'utf8')).toBe('in 1999');

test/fix/839.test.js

@@ -0,0 +1,24 @@
+// https://github.com/mathiasrw/rexreplace/issues/839
+// When the reader of a pipe stopped early, like head, the run crashed with EPIPE and a stack trace.
+import {describe, test, expect} from 'bun:test';
+import fs from 'fs';
+import {$} from 'bun';
+import {cli, sh} from '../helper.js';
+
+describe('#839 a pipe the reader closes early', () => {
+	test('ends the run as usual, with no error', async () => {
+		fs.writeFileSync('big.txt', Array.from({length: 200000}, (_, i) => `${i} foo`).join('\n'));
+		expect(await sh($`bun ${cli} foo bar big.txt -o | head -1`)).toEqual({
+			out: '0 bar\n',
+			err: 'big.txt\n',
+			exit: 0,
+		});
+	});
+
+	test('also when the status report comes after the reader is gone', async () => {
+		fs.writeFileSync('big.txt', Array.from({length: 200000}, (_, i) => `${i} foo`).join('\n'));
+		const result = await sh($`bun ${cli} foo bar big.txt -o -d | head -1`);
+		expect(result.out).toBe('0 bar\n');
+		expect(result.err).toBe('');
+	});
+});