Improve thread-safety of error and warning message handling.

Ensures all message reading occurs before thread shutdown.

docs/src/content/docs/api-input.md

@@ -18,7 +18,6 @@ Dimensions in the response will respect the `page` and `pages` properties of the
 A `Promise` is returned when `callback` is not provided.
 
 - `format`: Name of decoder used to parse image e.g. `jpeg`, `png`, `webp`, `gif`, `svg`, `heif`, `tiff`
-- `mediaType`: Media Type (MIME Type) e.g. `image/jpeg`, `image/png`, `image/svg+xml`, `image/avif`
 - `size`: Total size of image in bytes, for Stream and Buffer input only
 - `width`: Number of pixels wide (EXIF orientation is not taken into consideration, see example below)
 - `height`: Number of pixels high (EXIF orientation is not taken into consideration, see example below)

docs/src/content/docs/changelog/v0.35.0.md

@@ -20,7 +20,7 @@ slug: changelog/v0.35.0
 
 * Upgrade to libvips v8.18.0 for upstream bug fixes.
 
-* Ensure TIFF output `bitdepth` option is limited to 1, 2 or 4.
+* Improve thread-safety of error (and warning) messages.
 
 * Deprecate Windows 32-bit (win32-ia32) prebuilt binaries.
 
@@ -43,7 +43,4 @@ slug: changelog/v0.35.0
 * Ensure HEIF primary item is used as default page/frame.
   [#4487](https://github.com/lovell/sharp/issues/4487)
 
-* Add image Media Type (MIME Type) to metadata response.
-  [#4492](https://github.com/lovell/sharp/issues/4492)
-
 * Add WebP `exact` option for control over transparent pixel colour values.

lib/index.d.ts

@@ -1479,8 +1479,8 @@ declare namespace sharp {
         xres?: number | undefined;
         /** Vertical resolution in pixels/mm (optional, default 1.0) */
         yres?: number | undefined;
-        /** Reduce bitdepth to 1, 2 or 4 bit (optional) */
+        /** Reduce bitdepth to 1, 2 or 4 bit (optional, default 8) */
-        bitdepth?: 1 | 2 | 4 | undefined;
+        bitdepth?: 1 | 2 | 4 | 8 | undefined;
         /** Write 1-bit images as miniswhite (optional, default false) */
         miniswhite?: boolean | undefined;
         /** Resolution unit options: inch, cm (optional, default 'inch') */

lib/input.js

@@ -567,7 +567,6 @@ function _isStreamInput () {
  * A `Promise` is returned when `callback` is not provided.
  *
  * - `format`: Name of decoder used to parse image e.g. `jpeg`, `png`, `webp`, `gif`, `svg`, `heif`, `tiff`
- * - `mediaType`: Media Type (MIME Type) e.g. `image/jpeg`, `image/png`, `image/svg+xml`, `image/avif`
  * - `size`: Total size of image in bytes, for Stream and Buffer input only
  * - `width`: Number of pixels wide (EXIF orientation is not taken into consideration, see example below)
  * - `height`: Number of pixels high (EXIF orientation is not taken into consideration, see example below)

src/binding.gyp

@@ -21,7 +21,6 @@
         'defines': [
           '_VIPS_PUBLIC=__declspec(dllexport)',
           '_ALLOW_KEYWORD_MACROS',
-          '_HAS_EXCEPTIONS=1',
           'G_DISABLE_ASSERT',
           'G_DISABLE_CAST_CHECKS',
           'G_DISABLE_CHECKS'
@@ -149,8 +148,7 @@
           ['OS == "win"', {
             'defines': [
               '_ALLOW_KEYWORD_MACROS',
-              '_FILE_OFFSET_BITS=64',
+              '_FILE_OFFSET_BITS=64'
-              '_HAS_EXCEPTIONS=1'
             ],
             'link_settings': {
               'libraries': [

src/metadata.cc

@@ -151,52 +151,13 @@ class MetadataWorker : public Napi::AsyncWorker {
       }
       // PNG comments
       vips_image_map(image.get_image(), readPNGComment, &baton->comments);
-      // Media type
-      std::string mediaType;
-      switch (imageType) {
-        case sharp::ImageType::JPEG:
-        case sharp::ImageType::PNG:
-        case sharp::ImageType::WEBP:
-        case sharp::ImageType::JP2:
-        case sharp::ImageType::TIFF:
-        case sharp::ImageType::GIF:
-        case sharp::ImageType::FITS:
-        case sharp::ImageType::JXL:
-          baton->mediaType = "image/" + baton->format;
-          break;
-        case sharp::ImageType::SVG:
-          baton->mediaType = "image/svg+xml";
-          break;
-        case sharp::ImageType::HEIF:
-          if (baton->compression == "av1") {
-            baton->mediaType = "image/avif";
-          } else if (baton->compression == "hevc") {
-            baton->mediaType = "image/heic";
     }
-          break;
+    // Handle warnings
-        case sharp::ImageType::PDF:
+    std::string warning = sharp::VipsWarningPop();
-          baton->mediaType = "application/pdf";
+    while (!warning.empty()) {
-          break;
+      baton->warnings.push_back(warning);
-        case sharp::ImageType::OPENSLIDE:
+      warning = sharp::VipsWarningPop();
-          baton->mediaType = "image/tiff";
-          break;
-        case sharp::ImageType::PPM:
-          baton->mediaType = "image/x-portable-pixmap";
-          break;
-        case sharp::ImageType::EXR:
-          baton->mediaType = "image/x-exr";
-          break;
-        case sharp::ImageType::RAD:
-          baton->mediaType = "image/vnd.radiance";
-          break;
-        case sharp::ImageType::UHDR:
-          baton->mediaType = "image/jpeg";
-          break;
-        default:
-          break;
     }
-    }
-
     // Clean up
     vips_error_clear();
     vips_thread_shutdown();
@@ -206,19 +167,12 @@ class MetadataWorker : public Napi::AsyncWorker {
     Napi::Env env = Env();
     Napi::HandleScope scope(env);
 
-    // Handle warnings
+    for (auto& warning : baton->warnings) {
-    std::string warning = sharp::VipsWarningPop();
-    while (!warning.empty()) {
       debuglog.Call(Receiver().Value(), { Napi::String::New(env, warning) });
-      warning = sharp::VipsWarningPop();
     }
-
     if (baton->err.empty()) {
       Napi::Object info = Napi::Object::New(env);
       info.Set("format", baton->format);
-      if (!baton->mediaType.empty()) {
-        info.Set("mediaType", baton->mediaType);
-      }
       if (baton->input->bufferLength > 0) {
         info.Set("size", baton->input->bufferLength);
       }

src/metadata.h

@@ -19,7 +19,6 @@ struct MetadataBaton {
   sharp::InputDescriptor *input;
   // Output
   std::string format;
-  std::string mediaType;
   int width;
   int height;
   std::string space;
@@ -58,6 +57,7 @@ struct MetadataBaton {
   size_t gainMapLength;
   MetadataComments comments;
   std::string err;
+  std::vector<std::string> warnings;
 
   MetadataBaton():
     input(nullptr),

src/pipeline.cc

@@ -1280,12 +1280,21 @@ class PipelineWorker : public Napi::AsyncWorker {
       } else {
         if (baton->input->failOn == VIPS_FAIL_ON_WARNING) {
           (baton->err).append("Warning treated as error due to failOn setting");
-          baton->errUseWarning = true;
         } else {
           (baton->err).append("Unknown error");
         }
       }
     }
+    // Handle warnings
+    std::string warning = sharp::VipsWarningPop();
+    while (!warning.empty()) {
+      if (baton->input->failOn == VIPS_FAIL_ON_WARNING) {
+        (baton->err).append("\n").append(warning);
+      } else {
+        (baton->warnings).push_back(warning);
+      }
+      warning = sharp::VipsWarningPop();
+    }
     // Clean up libvips' per-request data and threads
     vips_error_clear();
     vips_thread_shutdown();
@@ -1295,16 +1304,9 @@ class PipelineWorker : public Napi::AsyncWorker {
     Napi::Env env = Env();
     Napi::HandleScope scope(env);
 
-    // Handle warnings
+    for (auto &warning : baton->warnings) {
-    std::string warning = sharp::VipsWarningPop();
-    while (!warning.empty()) {
-      if (baton->errUseWarning) {
-        (baton->err).append("\n").append(warning);
-      } else {
       debuglog.Call(Receiver().Value(), { Napi::String::New(env, warning) });
     }
-      warning = sharp::VipsWarningPop();
-    }
     if (baton->err.empty()) {
       int width = baton->width;
       int height = baton->height;

src/pipeline.h

@@ -204,6 +204,7 @@ struct PipelineBaton {
   bool jxlLossless;
   VipsBandFormat rawDepth;
   std::string err;
+  std::vector<std::string> warnings;
   bool errUseWarning;
   int keepMetadata;
   int withMetadataOrientation;

src/stats.cc

@@ -96,7 +96,12 @@ class StatsWorker : public Napi::AsyncWorker {
         (baton->err).append(err.what());
       }
     }
-
+    // Handle warnings
+    std::string warning = sharp::VipsWarningPop();
+    while (!warning.empty()) {
+      baton->warnings.push_back(warning);
+      warning = sharp::VipsWarningPop();
+    }
     // Clean up
     vips_error_clear();
     vips_thread_shutdown();
@@ -106,11 +111,8 @@ class StatsWorker : public Napi::AsyncWorker {
     Napi::Env env = Env();
     Napi::HandleScope scope(env);
 
-    // Handle warnings
+    for (auto& warning : baton->warnings) {
-    std::string warning = sharp::VipsWarningPop();
-    while (!warning.empty()) {
       debuglog.Call(Receiver().Value(), { Napi::String::New(env, warning) });
-      warning = sharp::VipsWarningPop();
     }
     if (baton->err.empty()) {
       // Stats Object

src/stats.h

@@ -45,6 +45,7 @@ struct StatsBaton {
   int dominantBlue;
 
   std::string err;
+  std::vector<std::string> warnings;
 
   StatsBaton():
     input(nullptr),

test/unit/avif.js

@@ -36,7 +36,6 @@ describe('AVIF', () => {
       density: 72,
       depth: 'uchar',
       format: 'jpeg',
-      mediaType: 'image/jpeg',
       hasAlpha: false,
       hasProfile: false,
       // 32 / (2048 / 858) = 13.40625
@@ -65,7 +64,6 @@ describe('AVIF', () => {
       compression: 'av1',
       depth: 'uchar',
       format: 'heif',
-      mediaType: 'image/avif',
       hasAlpha: false,
       hasProfile: false,
       height: 26,
@@ -95,7 +93,6 @@ describe('AVIF', () => {
       compression: 'av1',
       depth: 'uchar',
       format: 'heif',
-      mediaType: 'image/avif',
       hasAlpha: false,
       hasProfile: false,
       height: 24,
@@ -122,7 +119,6 @@ describe('AVIF', () => {
       compression: 'av1',
       depth: 'uchar',
       format: 'heif',
-      mediaType: 'image/avif',
       hasAlpha: false,
       hasProfile: false,
       height: 13,
@@ -152,7 +148,6 @@ describe('AVIF', () => {
       compression: 'av1',
       depth: 'uchar',
       format: 'heif',
-      mediaType: 'image/avif',
       hasAlpha: true,
       hasProfile: false,
       height: 300,
@@ -183,7 +178,6 @@ describe('AVIF', () => {
       compression: 'av1',
       depth: 'uchar',
       format: 'heif',
-      mediaType: 'image/avif',
       hasAlpha: false,
       hasProfile: false,
       height: 26,
@@ -242,7 +236,6 @@ describe('AVIF', () => {
     void exif;
     assert.deepStrictEqual(metadata, {
       format: 'heif',
-      mediaType: 'image/avif',
       width: 4096,
       height: 800,
       space: 'srgb',
@@ -266,7 +259,6 @@ describe('AVIF', () => {
     const { size, ...pngMetadata } = await sharp(data).metadata();
     assert.deepStrictEqual(pngMetadata, {
       format: 'png',
-      mediaType: 'image/png',
       width: 4096,
       height: 800,
       space: 'srgb',

test/unit/metadata.js

@@ -19,7 +19,6 @@ describe('Image metadata', () => {
     sharp(fixtures.inputJpg).metadata((err, metadata) => {
       if (err) throw err;
       assert.strictEqual('jpeg', metadata.format);
-      assert.strictEqual('image/jpeg', metadata.mediaType);
       assert.strictEqual('undefined', typeof metadata.size);
       assert.strictEqual(2725, metadata.width);
       assert.strictEqual(2225, metadata.height);
@@ -42,7 +41,6 @@ describe('Image metadata', () => {
     sharp(fixtures.inputJpgWithExif).metadata((err, metadata) => {
       if (err) throw err;
       assert.strictEqual('jpeg', metadata.format);
-      assert.strictEqual('image/jpeg', metadata.mediaType);
       assert.strictEqual('undefined', typeof metadata.size);
       assert.strictEqual(450, metadata.width);
       assert.strictEqual(600, metadata.height);
@@ -68,7 +66,6 @@ describe('Image metadata', () => {
       const profile = icc.parse(metadata.icc);
       assert.strictEqual('object', typeof profile);
       assert.strictEqual('Generic RGB Profile', profile.description);
-      assert.strictEqual('image/jpeg', metadata.mediaType);
       done();
     });
   });
@@ -95,7 +92,6 @@ describe('Image metadata', () => {
     sharp(fixtures.inputTiff).metadata((err, metadata) => {
       if (err) throw err;
       assert.strictEqual('tiff', metadata.format);
-      assert.strictEqual('image/tiff', metadata.mediaType);
       assert.strictEqual('undefined', typeof metadata.size);
       assert.strictEqual(2464, metadata.width);
       assert.strictEqual(3248, metadata.height);
@@ -115,7 +111,6 @@ describe('Image metadata', () => {
       assert.strictEqual('undefined', typeof metadata.xmp);
       assert.strictEqual('undefined', typeof metadata.xmpAsString);
       assert.strictEqual('inch', metadata.resolutionUnit);
-      assert.strictEqual('image/tiff', metadata.mediaType);
       done();
     });
   });
@@ -124,7 +119,6 @@ describe('Image metadata', () => {
     sharp(fixtures.inputTiffMultipage).metadata((err, metadata) => {
       if (err) throw err;
       assert.strictEqual('tiff', metadata.format);
-      assert.strictEqual('image/tiff', metadata.mediaType);
       assert.strictEqual('undefined', typeof metadata.size);
       assert.strictEqual(2464, metadata.width);
       assert.strictEqual(3248, metadata.height);
@@ -148,7 +142,6 @@ describe('Image metadata', () => {
     sharp(fixtures.inputPng).metadata((err, metadata) => {
       if (err) throw err;
       assert.strictEqual('png', metadata.format);
-      assert.strictEqual('image/png', metadata.mediaType);
       assert.strictEqual('undefined', typeof metadata.size);
       assert.strictEqual(2809, metadata.width);
       assert.strictEqual(2074, metadata.height);
@@ -173,7 +166,6 @@ describe('Image metadata', () => {
     sharp(fixtures.inputPngTestJoinChannel).metadata((err, metadata) => {
       if (err) throw err;
       assert.strictEqual('png', metadata.format);
-      assert.strictEqual('image/png', metadata.mediaType);
       assert.strictEqual('undefined', typeof metadata.size);
       assert.strictEqual(320, metadata.width);
       assert.strictEqual(240, metadata.height);
@@ -199,7 +191,6 @@ describe('Image metadata', () => {
     sharp(fixtures.inputPngWithTransparency).metadata((err, metadata) => {
       if (err) throw err;
       assert.strictEqual('png', metadata.format);
-      assert.strictEqual('image/png', metadata.mediaType);
       assert.strictEqual('undefined', typeof metadata.size);
       assert.strictEqual(2048, metadata.width);
       assert.strictEqual(1536, metadata.height);
@@ -234,7 +225,6 @@ describe('Image metadata', () => {
       height: 32,
       isPalette: false,
       isProgressive: false,
-      mediaType: 'image/png',
       space: 'b-w',
       width: 32,
       autoOrient: {
@@ -260,7 +250,6 @@ describe('Image metadata', () => {
       height: 32,
       isPalette: false,
       isProgressive: false,
-      mediaType: 'image/png',
       space: 'grey16',
       width: 32,
       autoOrient: {
@@ -274,7 +263,6 @@ describe('Image metadata', () => {
     sharp(fixtures.inputWebP).metadata((err, metadata) => {
       if (err) throw err;
       assert.strictEqual('webp', metadata.format);
-      assert.strictEqual('image/webp', metadata.mediaType);
       assert.strictEqual('undefined', typeof metadata.size);
       assert.strictEqual(1024, metadata.width);
       assert.strictEqual(772, metadata.height);
@@ -297,12 +285,11 @@ describe('Image metadata', () => {
     sharp(fixtures.inputWebPAnimated)
       .metadata()
       .then(({
-        format, mediaType, width, height, space, channels, depth,
+        format, width, height, space, channels, depth,
         isProgressive, pages, loop, delay, hasProfile,
         hasAlpha
       }) => {
         assert.strictEqual(format, 'webp');
-        assert.strictEqual(mediaType, 'image/webp');
         assert.strictEqual(width, 80);
         assert.strictEqual(height, 80);
         assert.strictEqual(space, 'srgb');
@@ -321,12 +308,11 @@ describe('Image metadata', () => {
     sharp(fixtures.inputWebPAnimated, { pages: -1 })
       .metadata()
       .then(({
-        format, mediaType, width, height, space, channels, depth,
+        format, width, height, space, channels, depth,
         isProgressive, pages, pageHeight, loop, delay,
         hasProfile, hasAlpha
       }) => {
         assert.strictEqual(format, 'webp');
-        assert.strictEqual(mediaType, 'image/webp');
         assert.strictEqual(width, 80);
         assert.strictEqual(height, 720);
         assert.strictEqual(space, 'srgb');
@@ -346,12 +332,11 @@ describe('Image metadata', () => {
     sharp(fixtures.inputWebPAnimatedLoop3)
       .metadata()
       .then(({
-        format, mediaType, width, height, space, channels, depth,
+        format, width, height, space, channels, depth,
         isProgressive, pages, loop, delay, hasProfile,
         hasAlpha
       }) => {
         assert.strictEqual(format, 'webp');
-        assert.strictEqual(mediaType, 'image/webp');
         assert.strictEqual(width, 370);
         assert.strictEqual(height, 285);
         assert.strictEqual(space, 'srgb');
@@ -370,7 +355,6 @@ describe('Image metadata', () => {
     sharp(fixtures.inputGif).metadata((err, metadata) => {
       if (err) throw err;
       assert.strictEqual('gif', metadata.format);
-      assert.strictEqual('image/gif', metadata.mediaType);
       assert.strictEqual('undefined', typeof metadata.size);
       assert.strictEqual(800, metadata.width);
       assert.strictEqual(533, metadata.height);
@@ -391,7 +375,6 @@ describe('Image metadata', () => {
     sharp(fixtures.inputGifGreyPlusAlpha).metadata((err, metadata) => {
       if (err) throw err;
       assert.strictEqual('gif', metadata.format);
-      assert.strictEqual('image/gif', metadata.mediaType);
       assert.strictEqual('undefined', typeof metadata.size);
       assert.strictEqual(2, metadata.width);
       assert.strictEqual(1, metadata.height);
@@ -412,12 +395,11 @@ describe('Image metadata', () => {
     sharp(fixtures.inputGifAnimated)
       .metadata()
       .then(({
-        format, mediaType, width, height, space, channels, depth,
+        format, width, height, space, channels, depth,
         isProgressive, pages, loop, delay, background,
         hasProfile, hasAlpha
       }) => {
         assert.strictEqual(format, 'gif');
-        assert.strictEqual(mediaType, 'image/gif');
         assert.strictEqual(width, 80);
         assert.strictEqual(height, 80);
         assert.strictEqual(space, 'srgb');
@@ -437,12 +419,11 @@ describe('Image metadata', () => {
     sharp(fixtures.inputGifAnimatedLoop3)
       .metadata()
       .then(({
-        format, mediaType, width, height, space, channels, depth,
+        format, width, height, space, channels, depth,
         isProgressive, pages, loop, delay, hasProfile,
         hasAlpha
       }) => {
         assert.strictEqual(format, 'gif');
-        assert.strictEqual(mediaType, 'image/gif');
         assert.strictEqual(width, 370);
         assert.strictEqual(height, 285);
         assert.strictEqual(space, 'srgb');
@@ -481,7 +462,6 @@ describe('Image metadata', () => {
   it('File in, Promise out', (_t, done) => {
     sharp(fixtures.inputJpg).metadata().then((metadata) => {
       assert.strictEqual('jpeg', metadata.format);
-      assert.strictEqual('image/jpeg', metadata.mediaType);
       assert.strictEqual('undefined', typeof metadata.size);
       assert.strictEqual(2725, metadata.width);
       assert.strictEqual(2225, metadata.height);
@@ -528,7 +508,6 @@ describe('Image metadata', () => {
     const pipeline = sharp();
     pipeline.metadata().then((metadata) => {
       assert.strictEqual('jpeg', metadata.format);
-      assert.strictEqual('image/jpeg', metadata.mediaType);
       assert.strictEqual(829183, metadata.size);
       assert.strictEqual(2725, metadata.width);
       assert.strictEqual(2225, metadata.height);
@@ -580,7 +559,6 @@ describe('Image metadata', () => {
     const pipeline = sharp().metadata((err, metadata) => {
       if (err) throw err;
       assert.strictEqual('jpeg', metadata.format);
-      assert.strictEqual('image/jpeg', metadata.mediaType);
       assert.strictEqual(829183, metadata.size);
       assert.strictEqual(2725, metadata.width);
       assert.strictEqual(2225, metadata.height);
@@ -605,7 +583,6 @@ describe('Image metadata', () => {
     image.metadata((err, metadata) => {
       if (err) throw err;
       assert.strictEqual('jpeg', metadata.format);
-      assert.strictEqual('image/jpeg', metadata.mediaType);
       assert.strictEqual('undefined', typeof metadata.size);
       assert.strictEqual(2725, metadata.width);
       assert.strictEqual(2225, metadata.height);
@@ -665,7 +642,7 @@ describe('Image metadata', () => {
   });
 
   it('keep existing ICC profile', async () => {
-    const data = await sharp(fixtures.inputJpgWithExif)
+    const data = await sharp(fixtures.inputJpgWithExif, { failOn: 'error' })
       .keepIccProfile()
       .toBuffer();
 
@@ -698,7 +675,7 @@ describe('Image metadata', () => {
   });
 
   it('keep existing ICC profile, avoid colour transform', async () => {
-    const [r, g, b] = await sharp(fixtures.inputPngWithProPhotoProfile)
+    const [r, g, b] = await sharp(fixtures.inputPngWithProPhotoProfile, { failOn: 'error' })
       .keepIccProfile()
       .raw()
       .toBuffer();
@@ -744,7 +721,7 @@ describe('Image metadata', () => {
   });
 
   it('transform to invalid ICC profile emits warning', async () => {
-    const img = sharp({ create })
+    const img = sharp({ create, failOn: 'error' })
       .png()
       .withIccProfile(fixtures.path('invalid-illuminant.icc'));
 
@@ -940,7 +917,6 @@ describe('Image metadata', () => {
       .metadata()
       .then(metadata => {
         assert.strictEqual(metadata.format, 'tiff');
-        assert.strictEqual(metadata.mediaType, 'image/tiff');
         assert.strictEqual(metadata.width, 317);
         assert.strictEqual(metadata.height, 211);
         assert.strictEqual(metadata.space, 'rgb16');
@@ -955,7 +931,6 @@ describe('Image metadata', () => {
     const metadata = await sharp(fixtures.inputAvif).metadata();
     assert.deepStrictEqual(metadata, {
       format: 'heif',
-      mediaType: 'image/avif',
       width: 2048,
       height: 858,
       space: 'srgb',
@@ -1039,7 +1014,6 @@ describe('Image metadata', () => {
     const metadata = await sharp(fixtures.inputJpgLossless).metadata();
     assert.deepStrictEqual(metadata, {
       format: 'jpeg',
-      mediaType: 'image/jpeg',
       width: 227,
       height: 149,
       space: 'srgb',

test/unit/png.js

@@ -145,7 +145,6 @@ describe('PNG', () => {
         width: 68
       },
       format: 'png',
-      mediaType: 'image/png',
       width: 68,
       height: 68,
       space: 'srgb',