Sharp: Integrate with forthcoming libvips v8.5.0 logging improvements

[puzrin]

When i process a lot of images, sometime something like this is printed to console:

vips warning: VipsJpeg: bad exif meta "exif-ifd3-GPSVersionID"
vips warning: icc_transform: embedded profile incompatible with image

Unexpected console texts are not acceptable - that can destroy terminal output (progress bars etc). Is it possible to suppress such things (i think that should be default behaviour)?

[lovell]

Hello, these are from libvips - set the VIPS_WARNING environment variable to suppress them, e.g. https://github.com/lovell/sharp/blob/master/package.json#L35

[puzrin]

I'm looking for long-term solution. It looks strange that additional actions should be applied to make module work right (and uncontrolled console writes is a bug for server side IMHO). May be we should report to libvips that warnings should be off by default (then anyone can enable those via env)?

[puzrin]

Reported to libvips. I hope, all will agree that writing to console from library is not a good practice.

[lovell]

Thanks Vitaly, I'll leave this open to track integration with any new libvips API when available.

[lovell]

I expect the sharp API to access libvips' log entries will probably look something like:

sharp.log.on('warning', (err) => {
  console.log(`libvips warning ${err.message}`);
});

Note that this event handler is registered against the sharp.log singleton rather than a sharp instance as there's no obvious way to tie a log entry back to a given input/pipeline.

[puzrin]

@lovell IMHO sharp.on('warning', ...) would be more simple, since sharp is already EventEmitter instance.

Anyway, those warnings looks useless. Probably it worth to disable those at all or dump to console via debug package. You can add interface later, if anyone request it, but i doubt this will ever happens.

I think, real problem is that now warnings poison console output, not "how to read those".

[thiagomorato]

By the way I'm one affected by the warning messing up the logs.

[lovell]

An alternative (better?) "API" would be to use util.debuglog.

[puzrin]

I'm for util.debuglog or debug. See no difference, because i have no plans to check those warnings.

[lovell]

Commit af4f0e8 on the ridge branch exposes libvips warning messages via Node's standard util.debuglog() function and the NODE_DEBUG=sharp environment variable. More importantly, this change means these warnings won't end up on stdout/stderr by default.

The implementation takes advantage of the rather nice C++11 lock_guard feature for thread-safety.

[mceachen]

Thanks for letting us squelch those emissions to stderr!

Unfortunately, for corrupt images, seeing warning messages was the only clue that a given image isn't valid. How do you handle these sorts of errors--do your use cases allow for just ignoring them?

(I haven't seen a libvips analog to imagemagick/graphicsmagick's identify -verbose that validates a given image, or perhaps I missed it?)

[puzrin]

@mceachen see https://github.com/lovell/sharp/commit/af4f0e81b09aae850e8cf3491fa4501ac1af27df#diff-343723a0cb380f664aedf0dfed5c1c2fR9

That's warnings, not errors. If image is completely corrupted, you should get error after sharp call. If problem is in image header, those are dropped with warning.

If you still wish to see those warnings set NODE_DEBUG=sharp env variable prior to start your script.

[mceachen]

Yeah, unfortunately for me I find that sharp is more than happy to read, resize, and write images from corrupted input.

Try truncating any valid JPEG (truncate -s 10k sample.jpg) or overlay zeroes into the middle of the file. identify -v (from ImageMagick/GraphicsMagick) and Python's PIL both see the image is corrupt, but with both the current released version of sharp, as well as the ridge branch, I see just warnings (with no description):

SHARP 13348: read gave 2 warnings

This is somewhat orthogonal to logging in general, I think, so should this be a new issue? Happy to move this comment wherever.

In any event, huge thanks to @lovell for so much work on sharp, and @puzrin , thanks for the assist here!

[puzrin]

As far as i understand, warnings are a strange thing for debug reasons. Like when you have some useless info, no idea what to do with it, and wish to show is somehow :)

I think, if you have warning instead of error on really broken image, that's another issue. Probably, you may wish more strict (or tune-able) data validation (and that's another issue too).

/cc @jcupitt

[jcupitt]

Hello, yes, the libvips default is to process corrupt images, but to issue a warning. I've often wanted to get what data I can from a damaged image.

Other warnings mean the same thing: something bad has happened, but libvips has carried on, on the assumption that some output (or wrong output) is better than none.

There's a fail option to all loaders that will make them issue an error and terminate instead.

[lovell]

@mceachen Feel free to open a new issue to track adding support for the fail option.

[lovell]

v0.18.0 now available, thanks for all the feedback.

[puzrin]

@lovell i still see those annoying warnings in console. Is it possible to hide those?

[lovell]

@puzrin Are you referring to corrupt JPEG image warnings? If so, we'll need to get #925 landed, which is itself dependent on a new feature of libvips v8.5.9, so will be in sharp v0.19.x.