diff --git a/devguide.md b/devguide.md index 4ba7bae8..01a820d6 100644 --- a/devguide.md +++ b/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` | diff --git a/src/cli.ts b/src/cli.ts index 9c186726..2b206c38 100644 --- a/src/cli.ts +++ b/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); } diff --git a/src/engine.ts b/src/engine.ts index 2b876878..b98526b9 100644 --- a/src/engine.ts +++ b/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}); } } diff --git a/src/output.ts b/src/output.ts index 1b0c053d..c67ad789 100644 --- a/src/output.ts +++ b/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) { diff --git a/test/engine/encoding.test.js b/test/engine/encoding.test.js index a04f1234..b13d2e9f 100644 --- a/test/engine/encoding.test.js +++ b/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, }; diff --git a/test/engine/replacement-file.test.js b/test/engine/replacement-file.test.js index 19abca35..5614d18e 100644 --- a/test/engine/replacement-file.test.js +++ b/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, }); }); diff --git a/test/fix/691.test.js b/test/fix/691.test.js index a97a9882..f98848fc 100644 --- a/test/fix/691.test.js +++ b/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( diff --git a/test/fix/700.test.js b/test/fix/700.test.js index 79cd47d8..686be3cf 100644 --- a/test/fix/700.test.js +++ b/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, }); }); diff --git a/test/fix/788.test.js b/test/fix/788.test.js index f98d03c2..7402fcf8 100644 --- a/test/fix/788.test.js +++ b/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'); diff --git a/test/fix/816.test.js b/test/fix/816.test.js index 60fe0ff9..40bae980 100644 --- a/test/fix/816.test.js +++ b/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('(?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); }); diff --git a/test/fix/818.test.js b/test/fix/818.test.js index c4029dc5..5d5ab881 100644 --- a/test/fix/818.test.js +++ b/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)', ]); }); }); diff --git a/test/fix/819.test.js b/test/fix/819.test.js index 5c0350aa..e97086a5 100644 --- a/test/fix/819.test.js +++ b/test/fix/819.test.js @@ -42,7 +42,7 @@ 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); } ); @@ -50,7 +50,7 @@ describe('#819 a file deleted before --sort looks at it', () => { 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, }); }); @@ -58,7 +58,7 @@ describe('#819 a file deleted before --sort looks at it', () => { 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, }); }); @@ -66,7 +66,7 @@ describe('#819 a file deleted before --sort looks at it', () => { 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'); diff --git a/test/fix/825.test.js b/test/fix/825.test.js index 067420b2..d9e9e4f8 100644 --- a/test/fix/825.test.js +++ b/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'); diff --git a/test/fix/839.test.js b/test/fix/839.test.js new file mode 100644 index 00000000..23af8e68 --- /dev/null +++ b/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(''); + }); +});