From 47fbf4bf7ccfd0588e4d9048dd956fcc575f2bd1 Mon Sep 17 00:00:00 2001 From: Mathias Wulff Date: Fri, 23 Oct 2026 21:03:17 +1100 Subject: [PATCH] Show only the message when --js code fails --- README.md | 2 +- src/cli.ts | 2 +- src/engine.ts | 14 ++++++-- test/engine/replacement-js.test.js | 19 +++++++---- test/engine/void-euro.test.js | 9 ++---- test/fix/705.test.js | 6 ++-- test/fix/808.test.js | 52 ++++++++++++++++++++++++++++++ 7 files changed, 84 insertions(+), 20 deletions(-) create mode 100644 test/fix/808.test.js diff --git a/README.md b/README.md index 1dbf3150..efb9389c 100644 --- a/README.md +++ b/README.md @@ -348,7 +348,7 @@ If planned renames clash, the process is halted and no change is applied to any | `-A` | **`--void-async`** Handle files in a synchronous flow. Good to limit memory usage when handling large files. [boolean] | | `-B` | **`--void-backup`** Avoid temporary backing up file. Works async (independent of -A flag) and will speed up things but at one point data lives only in memory, and you will lose the content if the process is interrupted. [boolean] | | `-b` | **`--keep-backup`** Keep a backup file of the original content. With --rename the old file is kept. [boolean] | -| `-S` | **`--simulate`** Do everything but write the files. The names of the files that would be changed are printed as usual, and no backup is made. [boolean] | +| `-S` | **`--simulate`** Execute everything during a run except changes to the file system. Names of affected files and content are still printed as in a usual run. [boolean] | | | **`--sort`** Process the files one at a time, in a sorted order. The order is chosen from 6 values: `natural` (numbers compared as numbers, and case ignored), `ascii` (alphabetic), `modified`, `created`, `size`, or `none` (the order of the input: the files given first, and then the files found by globs). Files that sort the same (like the same size) are ordered by ascii sort. Without --sort the files are processed asynchronously, so the order of the output can not be guaranteed, but the processing is started in natural order. [string] [choices: "natural", "ascii", "modified", "created", "size", "none"] | | | **`--sort-reverse`** Turn the order of the files around. [boolean] | | `-F` | **`--expect-files`** Check that at least one file is found. The exit code is 1 if not. [boolean] | diff --git a/src/cli.ts b/src/cli.ts index 3c9f6390..9d170219 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -261,7 +261,7 @@ const yargs = require('yargs') .boolean('S') .describe( 'S', - 'Do everything but write the files. The names of the files that would be changed are printed as usual, and no backup is made.' + 'Execute everything during a run except changes to the file system. Names of affected files and content are still printed as in a usual run.' ) .alias('S', 'simulate') diff --git a/src/engine.ts b/src/engine.ts index 67f9cddd..49c83ad9 100644 --- a/src/engine.ts +++ b/src/engine.ts @@ -905,6 +905,16 @@ function dynamicReplacement( 'return eval(__code_rr);' ); + // When the code fails, only the message is told, and not the stack trace with all the code put in front of the code of the user + const runCode = (...args) => { + try { + return dynamicContent(...args); + } catch (e) { + const where = _config_rr.dataIsPiped ? 'the piped data' : _file_rr; + return panic('The code of --js failed for ' + where + ':', String(e)); + } + }; + const needsByteOrSize = re.byteOrSize.test(_config_rr.replacement); const betterToReadfromFile = needsByteOrSize && 50000000 < _text.length; // around 50 Mb will lead to reading filezise from file instead of copying into buffer @@ -947,7 +957,7 @@ function dynamicReplacement( !re.matchNumbers.test(_config_rr.replacement) && !usesNamedGroups(_config_rr) ) { - const result = dynamicContent( + const result = runCode( require, fs, globs, @@ -1042,7 +1052,7 @@ function dynamicReplacement( } } - return dynamicContent( + return runCode( require, fs, globs, diff --git a/test/engine/replacement-js.test.js b/test/engine/replacement-js.test.js index a068d180..a98338bc 100644 --- a/test/engine/replacement-js.test.js +++ b/test/engine/replacement-js.test.js @@ -46,12 +46,18 @@ describe('--replacement-js', () => { expect(fs.readFileSync('my.file', 'utf8')).toBe('xxxbar'); }); - test('throws the errors of the code', async () => { + test('stops the run with the error of the code (#808)', async () => { fs.writeFileSync('my.file', 'foobar'); const config = {includeGlob: ['my.file'], replacementJs: true}; - await expect(rr('foo', 'this is not javascript', config)).rejects.toThrow(SyntaxError); - await expect(rr('foo', 'nope.nope', config)).rejects.toThrow(ReferenceError); - await expect(rr('foo', 'throw new Error("my own")', config)).rejects.toThrow('my own'); + for (const [code, message] of [ + ['this is not javascript', 'SyntaxError'], + ['nope.nope', 'ReferenceError'], + ['throw new Error("my own")', 'Error: my own'], + ]) { + const result = await rr('foo', code, config); + expect(result.err).toContain(message); + expect(result.exit).toBe(3); + } expect(fs.readFileSync('my.file', 'utf8')).toBe('foobar'); }); @@ -302,9 +308,8 @@ describe('--replacement-js', () => { describe('combined', () => { test('with --void-euro there is only the $ to get to the groups', async () => { expect(await js('(o)', '$1.toUpperCase()', 'fo', {voidEuro: true})).toBe('fO'); - await expect(js('(o)', '€1.toUpperCase()', 'fo', {voidEuro: true})).rejects.toThrow( - SyntaxError - ); + const config = {pipedData: 'fo', replacementJs: true, voidEuro: true}; + expect((await rr('(o)', '€1.toUpperCase()', config)).err).toContain('SyntaxError'); }); test('with --void-section a § can be used in the code', async () => { diff --git a/test/engine/void-euro.test.js b/test/engine/void-euro.test.js index 3477118e..a02d64e9 100644 --- a/test/engine/void-euro.test.js +++ b/test/engine/void-euro.test.js @@ -54,13 +54,8 @@ describe('--void-euro', () => { expect( (await rr('(o)', '€1.toUpperCase()', {pipedData: 'fo', replacementJs: true})).out ).toBe('fO'); - await expect( - rr('(o)', '€1.toUpperCase()', { - pipedData: 'fo', - replacementJs: true, - voidEuro: true, - }) - ).rejects.toThrow(SyntaxError); + const voided = {pipedData: 'fo', replacementJs: true, voidEuro: true}; + expect((await rr('(o)', '€1.toUpperCase()', voided)).err).toContain('SyntaxError'); }); test('with --replacement-pipe the piped replacement is never converted', async () => { diff --git a/test/fix/705.test.js b/test/fix/705.test.js index fe3a43a3..7baeea14 100644 --- a/test/fix/705.test.js +++ b/test/fix/705.test.js @@ -94,11 +94,13 @@ describe('#705 the engine returns a promise that resolves when all files are don }); }); - describe('the promise is rejected when the work can not be done', () => { + describe('the run stops when the work can not be done', () => { test.each(flows)('like with code that fails with %s', async (name, flags) => { fs.writeFileSync('my.file', 'foo'); const config = {includeGlob: ['my.file'], replacementJs: true, ...flags}; - await expect(rr('o', 'nope.nope', config)).rejects.toThrow(ReferenceError); + const result = await rr('o', 'nope.nope', config); + expect(result.err).toContain('ReferenceError'); + expect(result.exit).toBe(3); expect(fs.readdirSync('.')).toEqual(['my.file']); }); }); diff --git a/test/fix/808.test.js b/test/fix/808.test.js new file mode 100644 index 00000000..6836c92b --- /dev/null +++ b/test/fix/808.test.js @@ -0,0 +1,52 @@ +// https://github.com/mathiasrw/rexreplace/issues/808 +// When the code of --js failed, the stack trace showed all the code put in front of the code of the user. +import {describe, test, expect} from 'bun:test'; +import fs from 'fs'; +import {rr, rrStatus} from '../helper.js'; + +const failed = (where, message) => + ` ❌ The code of --js failed for ${where}: ${message}\n No further processing.\n\n`; + +describe('#808 code of --js that fails', () => { + test('stops the run with only the message', async () => { + expect(await rr('o', 'nope()', {pipedData: 'foo', replacementJs: true})).toEqual({ + out: '', + err: failed('the piped data', 'ReferenceError: nope is not defined'), + exit: 3, + }); + }); + + test('tells a syntax error the same way', async () => { + const result = await rr('o', '1 +', {pipedData: 'foo', replacementJs: true}); + expect(result.err).toStartWith(' ❌ The code of --js failed for the piped data: SyntaxError'); + expect(result.exit).toBe(3); + }); + + test('tells the file, also when the code runs once per match, and changes nothing', async () => { + fs.writeFileSync('my.file', 'foo'); + const config = {includeGlob: ['my.file'], replacementJs: true}; + expect((await rr('(o)', 'nope($1)', config)).err).toBe( + failed('my.file', 'ReferenceError: nope is not defined') + ); + expect(fs.readFileSync('my.file', 'utf8')).toBe('foo'); + }); + + test('stops the run in the async flow too', async () => { + const names = Array.from({length: 20}, (_, i) => `file${i}.txt`); + names.forEach(name => fs.writeFileSync(name, 'foo')); + const result = await rr('o', 'nope()', { + includeGlob: ['*.txt'], + replacementJs: true, + voidAsync: false, + }); + expect(result.err).toContain('ReferenceError: nope is not defined'); + expect(result.exit).toBe(3); + }); + + test('is in the errors of the status report', async () => { + const status = await rrStatus('o', 'nope()', {pipedData: 'foo', replacementJs: true}); + expect(status.errors).toEqual([ + 'The code of --js failed for the piped data: ReferenceError: nope is not defined', + ]); + }); +});