From d21121dddbc8a4a1be9eb9bb24969a225b87b493 Mon Sep 17 00:00:00 2001 From: Matthew McEachen Date: Fri, 8 Dec 2023 22:07:52 -0800 Subject: [PATCH] Expand isWarning() detection to avoid spurious errors --- CHANGELOG.md | 6 +++++- src/IsWarning.spec.ts | 2 +- src/IsWarning.ts | 8 ++------ src/WriteTask.spec.ts | 41 +++++++++++++++++++++++------------------ 4 files changed, 31 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97cd5849..b5c141ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,11 +26,15 @@ vendored versions of ExifTool match the version that they vendor. ## Version history +### v24.1.0 + +- 📦 Relaxed `isWarning()` detection to be simply `/warning:/i`. v24.0.0 would throw errors when extracting binary thumbnails due to issues like "Warning: Ignored non-standard EXIF at TIFF-IFD0-JPEG-APP1-IFD0", which is decidedly a warning. `ExifTool.write` now leans (hard) on returning `.warnings` rather than throwing errors: **It is up to you to inspect `.warnings` and decide for your own usecase if the issue is exceptional**. See [issue #162](https://github.com/photostructure/exiftool-vendored.js/issues/162) for details. + ### v24.0.0 - 💔 In the interests of reducing complexity, the `ExifToolOptions.isIgnorableError` predicate field was removed -- if this was used by anyone, please open an issue and we can talk about it. -- 💔 `ExifTool.write` now returns metadata describing how many files were unchanged, updated, or created, and no longer throws an error if the operation is a no-op. See [issue #162](https://github.com/photostructure/exiftool-vendored.js/issues/162) for details. +- 💔 `ExifTool.write` now returns metadata describing how many files were unchanged, updated, or created, and **no longer throws errors** if the operation is a no-op or inputs are invalid. See [issue #162](https://github.com/photostructure/exiftool-vendored.js/issues/162) for details. - ✨ `.warnings` are returned by `ExifTool.read` and `ExifTool.write` tasks if there are non-critical warnings emitted to `stderr` by ExifTool. diff --git a/src/IsWarning.spec.ts b/src/IsWarning.spec.ts index 2a106060..87dee5e1 100644 --- a/src/IsWarning.spec.ts +++ b/src/IsWarning.spec.ts @@ -5,6 +5,7 @@ describe("IsWarning", () => { for (const msg of [ "Warning: Duplicate MakerNoteUnknown tag in ExifIFD", "Warning: ICC_Profile deleted. Image colors may be affected", + "Warning: Tag 'INVALID_TAG_NAME' is not defined", "Error: Nothing to write - /tmp/test.xmp", "Nothing to do.", ]) { @@ -13,7 +14,6 @@ describe("IsWarning", () => { }) } for (const msg of [ - "Warning: Tag 'INVALID_TAG_NAME' is not defined", "Error: File not found - nonexistant.jpg", ]) { it(`rejects ${msg}`, () => { diff --git a/src/IsWarning.ts b/src/IsWarning.ts index 8385726d..56143a14 100644 --- a/src/IsWarning.ts +++ b/src/IsWarning.ts @@ -5,14 +5,10 @@ export interface IgnorableError { (err: Maybe): boolean } -// These are ignorable: -// Warning: Duplicate MakerNoteUnknown tag in ExifIFD +// This is quite lax by design: it's up to the user to check .warnings! -// These are not: -// Warning: Tag 'INVALID_TAG_NAME' is not defined +const WarningRE = /\bwarning: |\bnothing to (?:write|do)\b/i -const WarningRE = - /^warning: duplicate \w+ tag|^error: nothing to write|^nothing to do|^warning: icc_profile deleted/i /** * This is the default implementation of IgnorableError, and ignores null, * undefined, errors without a message, warnings about duplicate tags, and diff --git a/src/WriteTask.spec.ts b/src/WriteTask.spec.ts index c1600154..222c305f 100644 --- a/src/WriteTask.spec.ts +++ b/src/WriteTask.spec.ts @@ -352,23 +352,22 @@ describe("WriteTask", function () { await exiftool.write(f, { RegistryID: struct }) const tags = await exiftool.read(f) expect(tags.RegistryID).to.eql(struct) - return }) it("rejects setting to a non-time value", async () => { const src = await dest() - return expect( - exiftool.write(src, { + expect( + (await exiftool.write(src, { DateTimeOriginal: "this is not a time" as any, - }) - ).to.be.rejectedWith(/Invalid date\/time/) + })).warnings?.join("\n") + ).to.match(/Invalid date\/time/) }) it("rejects an invalid numeric Orientation", async () => { const src = await dest() - return expect( - exiftool.write(src, { "Orientation#": -1 }) - ).to.be.rejectedWith(/Value below int16u minimum/i) + expect( + (await exiftool.write(src, { "Orientation#": -1 })).warnings?.join("\n") + ).to.match(/Value below int16u minimum/i) }) it("tags case-insensitively", async () => { @@ -385,18 +384,24 @@ describe("WriteTask", function () { it("rejects un-writable tags", async () => { const src = await dest() - await expect( - exiftool.write(src, { ImageOffset: 12345 } as any) - ).to.be.rejectedWith(/ImageOffset is not writable/) + expect( + ( + await exiftool.write(src, { + ImageOffset: 12345, + } as any) + ).warnings?.join("\n") + ).to.match(/ImageOffset is not writable/i) }) it("rejects an invalid string Orientation", async () => { const src = await dest() - await expect( - exiftool.write(src, { - Orientation: "this isn't a valid orientation" as any, - }) - ).to.be.rejectedWith(/Can't convert IFD0:Orientation/i) + expect( + ( + await exiftool.write(src, { + Orientation: "this isn't a valid orientation", + }) + ).warnings?.join("\n") + ).to.be.match(/Can't convert IFD0:Orientation/i) }) it("handles deleting tags from empty files", async () => { @@ -490,8 +495,8 @@ describe("WriteTask", function () { it("rejects unknown tags", async () => { const src = await dest() return expect( - exiftool.write(src, { RandomTag: 123 } as any) - ).to.be.rejectedWith(/Tag 'RandomTag' is not defined/) + (await exiftool.write(src, { RandomTag: 123 } as any)).warnings?.join("\n") + ).to.match(/Tag 'RandomTag' is not defined/) }) it("round-trips a struct tag with a ResourceEvent with primitive values", async () => {