diff --git a/CHANGELOG.md b/CHANGELOG.md index a4f0499..a2d4176 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,8 +3,9 @@ ## 6.0.0-dev - Added `--dry` option to process images without writing output files. -- Added `--print` option to print input and output metadata as JSON. -- Removed `jpg` output format alias; use `jpeg` instead. +- Added `--print` option to print input and output metadata as JSON ([#98](https://github.com/vseventer/sharp-cli/issues/48)). +- Added graceful handling of partial failures during batch image processing ([#98](https://github.com/vseventer/sharp-cli/issues/98)). +- Fixes bug where format options could cause errors. Consequently, removed `jpg` output format alias; use `jpeg` instead ([#92](https://github.com/vseventer/sharp-cli/issues/92)). - Internal refactor from CommonJS to ESM. - Updated `sharp` dependency to 0.35.3. - Updated dependencies. diff --git a/lib/convert.js b/lib/convert.js index 5b14f6f..9a926f9 100644 --- a/lib/convert.js +++ b/lib/convert.js @@ -117,7 +117,7 @@ export default { })); }); }); - return Promise.all(promises); + return Promise.allSettled(promises); }, // Convert a stream. diff --git a/lib/index.js b/lib/index.js index 2072c18..b371d2b 100644 --- a/lib/index.js +++ b/lib/index.js @@ -43,12 +43,32 @@ export default (args, options = {}) => { if (argv.input) { return convert .files(argv.input, argv.output, context) - .then((output) => { + .then((results) => { + // On error, set the code and let the program finish naturally + const containsError = results.some( + ({ status }) => status === "rejected", + ); + if (containsError) process.exitCode = 1; + if (argv.print) { + const output = results.map(({ reason, status, value }) => { + return status === "fulfilled" + ? value + : { error: reason.message }; + }); logger.log(JSON.stringify(output)); } else { - const arr = Array.isArray(output) ? output : [output]; - arr.forEach((file) => logger.log(file.output.path)); + results.forEach((result) => { + if (result.status === "fulfilled") { + logger.log(result.value.output.path); + } else { + logger.error(`FAILED: ${result.reason.message}`); + } + }); + if (containsError) { + logger.error(); + logger.error("Specify --help for available options"); + } } }); } diff --git a/test/convert.js b/test/convert.js index ecfdc14..aa5dbd9 100644 --- a/test/convert.js +++ b/test/convert.js @@ -39,6 +39,10 @@ import tile from "../cmd/output.js"; describe("convert", () => { const options = { sequentialRead: false }; const createContext = () => ({ options, queue: [] }); + const getValue = (result) => { + expect(result.status).to.equal("fulfilled"); + return result.value; + }; // Default input. const input = fileURLToPath(new URL("./fixtures/input.jpg", import.meta.url)); @@ -61,12 +65,15 @@ describe("convert", () => { it("must convert a file", () => { return convert .files([input], dest, createContext()) - .then(([info]) => expect(fs.existsSync(info.output.path)).to.be.true()); + .then(([result]) => + expect(fs.existsSync(getValue(result).output.path)).to.be.true(), + ); }); it("must convert a file formatted based on extension", () => { return convert .files([input], path.join(dest, "{name}.avif"), createContext()) - .then(([info]) => { + .then(([result]) => { + const info = getValue(result); expect(info.output).to.have.property("format", "heif"); expect(info.output).to.have.property("path"); expect(info.output.path).to.contain(".avif"); @@ -75,7 +82,8 @@ describe("convert", () => { it("must not write a file during a dry run", () => { const dryDest = path.join(dest, "dry.jpg"); const context = { ...createContext(), dry: true }; - return convert.files([input], dryDest, context).then(([info]) => { + return convert.files([input], dryDest, context).then(([result]) => { + const info = getValue(result); expect(info.output).to.have.property("format", "jpeg"); expect(info.output).to.have.property("size"); expect(info.output.path).to.equal(dryDest); @@ -85,7 +93,8 @@ describe("convert", () => { it("must return input and output metadata", () => { const output = path.join(dest, "output.jpg"); const context = { ...createContext(), dry: true, print: true }; - return convert.files([input], output, context).then(([info]) => { + return convert.files([input], output, context).then(([result]) => { + const info = getValue(result); expect(info.input).to.have.property("format", "jpeg"); expect(info.input.path).to.equal(input); expect(info.output).to.have.property("format", "jpeg"); @@ -96,8 +105,8 @@ describe("convert", () => { const context = { ...createContext(), dry: true, print: true }; return convert.files([input, input], dest, context).then((info) => { expect(info).to.have.length(2); - expect(info[0].input).to.have.property("format", "jpeg"); - expect(info[0].output).to.have.property("format", "jpeg"); + expect(getValue(info[0]).input).to.have.property("format", "jpeg"); + expect(getValue(info[0]).output).to.have.property("format", "jpeg"); }); }); it("must pass the output extension format to queued handlers", () => { @@ -114,19 +123,16 @@ describe("convert", () => { .files([input], path.join(dest, "{name}.avif"), context) .then(() => expect(format).to.equal("avif")); }); - it("must convert a file and output to an existing directory", () => { + it("must report a file conversion error", () => { // Negative test for directory that does not exist. const rand = "" + Math.random(); return convert .files([input, input], rand, createContext()) - .then(() => { - throw new Error("STOP"); - }) - .catch((err) => { - expect(err).to.exist(); - expect(err).to.have.property("message"); - expect(err.message).to.contain(`${rand}/input.jpg`); - expect(err.message).to.contain("No such file or directory"); + .then(([result]) => { + expect(result.status).to.equal("rejected"); + expect(result.reason).to.have.property("message"); + expect(result.reason.message).to.contain(`${rand}/input.jpg`); + expect(result.reason.message).to.contain("No such file or directory"); }); }); it("must convert multiple files", () => { @@ -138,8 +144,8 @@ describe("convert", () => { const rand = Math.random(); return convert .files([input], path.join(dest, `{name}-${rand}{ext}`), createContext()) - .then(([info]) => - expect(info.output.path).to.contain(`input-${rand}.jpg`), + .then(([result]) => + expect(getValue(result).output.path).to.contain(`input-${rand}.jpg`), ); }); it("must allow the same file as input and output", () => { diff --git a/test/index.js b/test/index.js index c5cfd50..2280d17 100644 --- a/test/index.js +++ b/test/index.js @@ -22,6 +22,7 @@ */ // Standard lib. +import path from "node:path"; import { fileURLToPath } from "node:url"; // Package modules. @@ -97,6 +98,34 @@ describe("CLI", () => { sinon.assert.notCalled(logger.error); }); }); + it("must print partial batch failures as JSON", () => { + const invalid = path.join(dest, "invalid.jpg"); + return fs + .outputFile(invalid, "not an image") + .then(() => + cli(["--dry", "--print", "-i", input, invalid, "-o", dest], { logger }), + ) + .then(() => { + const output = JSON.parse(logger.log.firstCall.args[0]); + expect(output).to.have.length(2); + expect(output[0]).to.have.property("input"); + expect(output[0]).to.have.property("output"); + expect(output[1]).to.have.property("error"); + sinon.assert.notCalled(logger.error); + expect(process.exitCode).to.equal(1); + }); + }); + it("must report partial batch failures", () => { + const invalid = path.join(dest, "invalid.jpg"); + return fs + .outputFile(invalid, "not an image") + .then(() => cli(["-i", input, invalid, "-o", dest], { logger })) + .then(() => { + sinon.assert.calledWithMatch(logger.log, path.join(dest, "input.jpg")); + sinon.assert.calledWithMatch(logger.error, "FAILED:"); + expect(process.exitCode).to.equal(1); + }); + }); it("must display errors", () => { return cli(["-i", missing, "-o", dest], { logger }).then(() => { sinon.assert.notCalled(logger.log);