Skip to content

Commit

Permalink
Expand isWarning() detection to avoid spurious errors
Browse files Browse the repository at this point in the history
  • Loading branch information
mceachen committed Dec 9, 2023
1 parent e0ff9a9 commit d21121d
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 26 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
2 changes: 1 addition & 1 deletion src/IsWarning.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
]) {
Expand All @@ -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}`, () => {
Expand Down
8 changes: 2 additions & 6 deletions src/IsWarning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,10 @@ export interface IgnorableError {
(err: Maybe<Error | string>): 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
Expand Down
41 changes: 23 additions & 18 deletions src/WriteTask.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down

0 comments on commit d21121d

Please sign in to comment.