WIP: Switch from custom VError to standard runtime_error

As of 8.18.0, libvips C++ wrapper retrieves error messages at
exception construction time rather than lazily when accessed.

On Windows this led to error messages being referenced rather
than copied, leading to access beyond their lifetime and possible
corruption.

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.
 
-* Improve thread-safety of error (and warning) messages.
+* Ensure TIFF output `bitdepth` option is limited to 1, 2 or 4.
 
 * Deprecate Windows 32-bit (win32-ia32) prebuilt binaries.
 

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, default 8) */
-        bitdepth?: 1 | 2 | 4 | 8 | undefined;
+        /** Reduce bitdepth to 1, 2 or 4 bit (optional) */
+        bitdepth?: 1 | 2 | 4 | undefined;
         /** Write 1-bit images as miniswhite (optional, default false) */
         miniswhite?: boolean | undefined;
         /** Resolution unit options: inch, cm (optional, default 'inch') */

src/binding.gyp

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

src/metadata.cc

@@ -152,12 +152,7 @@ class MetadataWorker : public Napi::AsyncWorker {
       // PNG comments
       vips_image_map(image.get_image(), readPNGComment, &baton->comments);
     }
-    // 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();
@@ -167,9 +162,13 @@ class MetadataWorker : public Napi::AsyncWorker {
     Napi::Env env = Env();
     Napi::HandleScope scope(env);
 
-    for (auto& warning : baton->warnings) {
+    // Handle 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);

src/metadata.h

@@ -57,7 +57,6 @@ struct MetadataBaton {
   size_t gainMapLength;
   MetadataComments comments;
   std::string err;
-  std::vector<std::string> warnings;
 
   MetadataBaton():
     input(nullptr),

src/pipeline.cc

@@ -1280,21 +1280,12 @@ 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();
@@ -1304,8 +1295,15 @@ class PipelineWorker : public Napi::AsyncWorker {
     Napi::Env env = Env();
     Napi::HandleScope scope(env);
 
-    for (auto &warning : baton->warnings) {
-      debuglog.Call(Receiver().Value(), { Napi::String::New(env, warning) });
+    // Handle 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;

src/pipeline.h

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

src/stats.cc

@@ -96,12 +96,7 @@ 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();
@@ -111,8 +106,11 @@ class StatsWorker : public Napi::AsyncWorker {
     Napi::Env env = Env();
     Napi::HandleScope scope(env);
 
-    for (auto& warning : baton->warnings) {
+    // Handle 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,7 +45,6 @@ struct StatsBaton {
   int dominantBlue;
 
   std::string err;
-  std::vector<std::string> warnings;
 
   StatsBaton():
     input(nullptr),

test/unit/metadata.js

@@ -642,7 +642,7 @@ describe('Image metadata', () => {
   });
 
   it('keep existing ICC profile', async () => {
-    const data = await sharp(fixtures.inputJpgWithExif, { failOn: 'error' })
+    const data = await sharp(fixtures.inputJpgWithExif)
       .keepIccProfile()
       .toBuffer();
 
@@ -675,7 +675,7 @@ describe('Image metadata', () => {
   });
 
   it('keep existing ICC profile, avoid colour transform', async () => {
-    const [r, g, b] = await sharp(fixtures.inputPngWithProPhotoProfile, { failOn: 'error' })
+    const [r, g, b] = await sharp(fixtures.inputPngWithProPhotoProfile)
       .keepIccProfile()
       .raw()
       .toBuffer();
@@ -721,7 +721,7 @@ describe('Image metadata', () => {
   });
 
   it('transform to invalid ICC profile emits warning', async () => {
-    const img = sharp({ create, failOn: 'error' })
+    const img = sharp({ create })
       .png()
       .withIccProfile(fixtures.path('invalid-illuminant.icc'));