Expected behavior of failOn

[LekoArts]

Question about an existing feature

Hi! I'd like some clarification about the failOn feature and how it's expected to behave. Right now I'm not sure if I'm doing something wrong or if it just will work like that.

What are you trying to achieve?

While working on gatsbyjs/gatsby#37177 I wanted to add a test case to verify that the expected behavior works. We often wrap sharp like this:

try {
  const pipeline = sharp({ failOn: pluginOptions.failOn })
  fs.createReadStream(file.absolutePath).pipe(pipeline)

  metadata = await pipeline.metadata()
} catch (err) {
  reportError(
    `Failed to retrieve metadata from image ${file.absolutePath}`,
    err,
    reporter
  )
  return null
}

The goal of the above PR is that when failOn: 'none' is set the reportError function shouldn't be called. For this test case I then created a .txt file, added some text to it and then renamed the file to have a .png extension. So a completely broken .png image really.

Now, even with failOn set to none, it still throws an error and in the catch the reportError is called. Is this expected behavior? Is there some threshold of what sharp will accept? I understood the option with set to none as "please don't throw any errors".

Please provide a minimal, standalone code sample, without other dependencies, that demonstrates this question

See explanation above.

[lovell]

The failOn settings take effect once the format of the image has been determined, so for PNG you'll need to add these 8 magic bytes at the start of the "broken" file.

https://github.com/libvips/libvips/blob/e3289ad2c19a436221a3cce769483fbce09f7b8b/libvips/foreign/spngload.c#L716

Otherwise you'll probably get a "...contains unsupported image format" error.

[LekoArts]

Thanks for the answer!

When I tried that it (following https://superuser.com/a/233112 and converting the bytes to "hex") errors with:

Failed to retrieve metadata from image /Users/lejoe/code/work/gatsby/packages/gatsby-plugin-sharp/src/__tests__/images/invalid.png [Error: Input buffer has corrupt header: pngload_buffer: end of stream]

When I tried any of the images in here https://github.com/YahooArchive/pngjs-image/tree/master/test/png/PngSuite/corrupt I get:

Failed to retrieve metadata from image /Users/lejoe/code/work/gatsby/packages/gatsby-plugin-sharp/src/__tests__/images/invalid.png [Error: Input buffer has corrupt header: pngload_buffer: invalid color type]

So I suppose (because I don't know how to check) that those corrupt png images have these 8 magic bytes at the beginning but it still fails?

[lovell]

Apologies, there needs to be enough of a valid header beyond the magic bytes so that libvips can read out basic settings and therefore the various decoder libraries can attempt to start to decode pixel values.

Maybe it's best to think of failOn in terms of pixel values (and the file structure relating to these) rather than header/metadata. We should probably update the docs to better reflect this.

[LekoArts]

Okay, thanks for the clarification! I'm fine with it working this way, but yeah, while reading the docs for this I understood this as "just never fail". Because right now we also passed along this notion into our docs: https://www.gatsbyjs.com/plugins/gatsby-plugin-sharp/?=plugin-sharp#allow-build-to-continue-on-image-processing-error

I'm happy to put up a PR to the sharp docs, too (while updating ours), to set the correct expectations. If you want, you can give me a "docs-ready" version of what you just said ("Maybe it's best to think of failOn in terms of pixel values (and the file structure relating to these) rather than header/metadata") or I'll do my best to paraphrase it :D

[LekoArts]

Put up the PR: #3481