feat(ImageBuf): auto print uncaught IB errors (#3949)

Advanced users know to check error returns and retrieve errors.

New users often just try OIIO calls with no error checks when they don't
get behavior they want, ask questions to the dev team. We hypothesize
that if they only saw the text of the error messages that they never
retrieved, they could probably self-diagnose many of their problems.

This patch makes an ImageBuf, when it is destryed, print any pending
error messages that were never retrieved. It happens by default, but can
be disabled by setting the new global attribute
"imagebuf:print_uncaught_errors" to 0 (it defaults to 1).

This doesn't cover everything, but for ImageBuf and ImageBufAlgo, maybe
it will help some users? It could annoy advanced users who intend to not
retrieve error messages (why?) but still don't want this printed. But
that's who the new attribute is for.

Signed-off-by: Larry Gritz <[email protected]>

src/include/OpenImageIO/imageio.h

@@ -2962,7 +2962,7 @@ OIIO_API std::string geterror(bool clear = true);
 ///    you should raise this limit. Setting the limit to 0 means having no
 ///    limit.
 ///
-/// - `int log_times`
+/// - `int log_times` (0)
 ///
 ///    When the `"log_times"` attribute is nonzero, `ImageBufAlgo` functions
 ///    are instrumented to record the number of times they were called and
@@ -2980,6 +2980,20 @@ OIIO_API std::string geterror(bool clear = true);
 ///    the log information. When the `log_times` attribute is disabled,
 ///    there is no additional performance cost.
 ///
+/// - `imagebuf:print_uncaught_errors` (1)
+///
+///   If nonzero, an `ImageBuf` upon destruction will print any error messages
+///   that were never retrieved by its `geterror()` method. While this may
+///   seem chaotic, we are presuming that any well-written library or
+///   application will proactively check error codes and retrieve errors, so
+///   will never print anything upon destruction. But for less sophisticated
+///   applications (or users), this is very useful for forcing display of
+///   error messages so that users can see relevant errors even if they never
+///   check them explicitly, thus self-diagnose their troubles before asking
+///   the project dev deam for help. Advanced users who for some reason desire
+///   to neither retrieve errors themselves nor have them printed in this
+///   manner can disable the behavior by setting this attribute to 0.
+///
 OIIO_API bool attribute(string_view name, TypeDesc type, const void* val);
 
 /// Shortcut attribute() for setting a single integer.

src/libOpenImageIO/imagebuf.cpp

@@ -485,6 +485,16 @@ ImageBufImpl::~ImageBufImpl()
     // else init_spec requested the system-wide shared cache, which
     // does not need to be destroyed.
     clear();
+
+    // Upon destruction, print uncaught errors to help users who don't know
+    // how to properly check for errors.
+    if (!m_err.empty() /* Note: safe becausethis is the dtr */
+        && pvt::imagebuf_print_uncaught_errors) {
+        OIIO::print(
+            "An ImageBuf was destroyed with a pending error message that was never\n"
+            "retrieved via ImageBuf::geterror(). This was the error message:\n{}\n",
+            m_err);
+    }
 }
 
 

src/libOpenImageIO/imagebuf_test.cpp

@@ -501,6 +501,16 @@ test_write_over()
 
 
 
+static void
+test_uncaught_error()
+{
+    ImageBuf buf;
+    buf.error("Boo!");
+    // buf exists scope and is destroyed without anybody retrieving the error.
+}
+
+
+
 int
 main(int /*argc*/, char* /*argv*/[])
 {
@@ -534,6 +544,8 @@ main(int /*argc*/, char* /*argv*/[])
 
     test_write_over();
 
+    test_uncaught_error();
+
     Filesystem::remove("A_imagebuf_test.tif");
     return unit_test_failures;
 }

src/libOpenImageIO/imageio.cpp

@@ -52,6 +52,7 @@ int dds_bc5normal(0);
 int limit_channels(1024);
 int limit_imagesize_MB(std::min(32 * 1024,
                                 int(Sysutil::physical_memory() >> 20)));
+int imagebuf_print_uncaught_errors(1);
 ustring font_searchpath(Sysutil::getenv("OPENIMAGEIO_FONTS"));
 ustring plugin_searchpath(OIIO_DEFAULT_PLUGIN_SEARCHPATH);
 std::string format_list;         // comma-separated list of all formats
@@ -344,6 +345,10 @@ attribute(string_view name, TypeDesc type, const void* val)
         limit_imagesize_MB = *(const int*)val;
         return true;
     }
+    if (name == "imagebuf:print_uncaught_errors" && type == TypeInt) {
+        imagebuf_print_uncaught_errors = *(const int*)val;
+        return true;
+    }
     if (name == "use_tbb" && type == TypeInt) {
         oiio_use_tbb = *(const int*)val;
         return true;
@@ -472,6 +477,10 @@ getattribute(string_view name, TypeDesc type, void* val)
         *(int*)val = dds_bc5normal;
         return true;
     }
+    if (name == "imagebuf:print_uncaught_errors" && type == TypeInt) {
+        *(int*)val = imagebuf_print_uncaught_errors;
+        return true;
+    }
     if (name == "use_tbb" && type == TypeInt) {
         *(int*)val = oiio_use_tbb;
         return true;

src/libOpenImageIO/imageio_pvt.h.in

@@ -44,6 +44,7 @@ extern int openexr_core;
 extern int limit_channels;
 extern int limit_imagesize_MB;
 extern int opencv_version;
+extern int imagebuf_print_uncaught_errors;
 extern OIIO_UTIL_API int oiio_use_tbb; // This lives in libOpenImageIO_Util
 OIIO_API const std::vector<std::string>& font_dirs();
 OIIO_API const std::vector<std::string>& font_file_list();