feat: print unretrieved global error messages (#4005)

In the ongoing effort to help newcomers to OIIO diagnose their own
problems, this uses a trick to print any global error messages that
never got retrieved by OIIO::geterror() at the time that the program
exits. (This is all similar to a change we made recently that adds an
analogous behavior to ImageBuf, but in this case it's for those error
messages that don't have an object like an ImageBuf to attach to.)

This should have no effect on anybody who diligently checks
OIIO::has_error() or OIIO::geterror() after any failed calls that would
send error messages to that global store. Also, it is possible to
disable the behavior by setting the new global attribute
"oiio:print_uncaught_errors" to 0.

But for users who don't know to check errors and are experiencing
baffling behavior with OIIO, maybe seing those error messages will
prompt them to understand what's going wrong before they feel the need
to file an issue against OIIO or need to ask us for help.

Doing this helped me to find two places where our own code in OIIO
failed to clear the global error at times that it should, and these are
also fixed as part of this PR.

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

src/include/OpenImageIO/imageio.h

@@ -2980,6 +2980,21 @@ OIIO_API std::string geterror(bool clear = true);
 ///    the log information. When the `log_times` attribute is disabled,
 ///    there is no additional performance cost.
 ///
+/// - `oiio:print_uncaught_errors` (1)
+///
+///   If nonzero, upon program exit, any error messages that would have been
+///   retrieved by a call to `OIIO::geterror()`, but never were, will be
+///   printed to stdout. While this may seem chaotic, we are presuming that
+///   any well-written library or application will proactively check error
+///   codes and retrieve errors, so this will never print anything upon exit.
+///   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.
+///
 /// - `imagebuf:print_uncaught_errors` (1)
 ///
 ///   If nonzero, an `ImageBuf` upon destruction will print any error messages

src/libOpenImageIO/imageio.cpp

@@ -60,6 +60,7 @@ std::string output_format_list;  // comma-separated list of writable formats
 std::string extension_list;      // list of all extensions for all formats
 std::string library_list;        // list of all libraries for all formats
 int oiio_log_times = Strutil::stoi(Sysutil::getenv("OPENIMAGEIO_LOG_TIMES"));
+int oiio_print_uncaught_errors(1);
 std::vector<float> oiio_missingcolor;
 }  // namespace pvt
 
@@ -225,9 +226,29 @@ openimageio_version()
 
 
 
+// ErrorHolder houses a string, with the addition that when it is destroyed,
+// it will disgorge any un-retrieved error messages, in an effort to help
+// beginning users diagnose their problems if they have forgotten to call
+// geterror().
+struct ErrorHolder {
+    std::string error_msg;
+
+    ~ErrorHolder()
+    {
+        if (!error_msg.empty() && pvt::oiio_print_uncaught_errors) {
+            OIIO::print(
+                "OpenImageIO exited with a pending error message that was never\n"
+                "retrieved via OIIO::geterror(). This was the error message:\n{}\n",
+                error_msg);
+        }
+    }
+};
+
+
+
 // To avoid thread oddities, we have the storage area buffering error
 // messages for append_error()/geterror() be thread-specific.
-static thread_local std::string error_msg;
+static thread_local ErrorHolder error_msg_holder;
 
 
 void
@@ -236,6 +257,7 @@ pvt::append_error(string_view message)
     // Remove a single trailing newline
     if (message.size() && message.back() == '\n')
         message.remove_suffix(1);
+    std::string& error_msg(error_msg_holder.error_msg);
     OIIO_ASSERT(
         error_msg.size() < 1024 * 1024 * 16
         && "Accumulated error messages > 16MB. Try checking return codes!");
@@ -256,6 +278,7 @@ pvt::append_error(string_view message)
 bool
 has_error()
 {
+    std::string& error_msg(error_msg_holder.error_msg);
     return !error_msg.empty();
 }
 
@@ -264,6 +287,7 @@ has_error()
 std::string
 geterror(bool clear)
 {
+    std::string& error_msg(error_msg_holder.error_msg);
     std::string e = error_msg;
     if (clear)
         error_msg.clear();
@@ -344,6 +368,10 @@ attribute(string_view name, TypeDesc type, const void* val)
         limit_imagesize_MB = *(const int*)val;
         return true;
     }
+    if (name == "oiio:print_uncaught_errors" && type == TypeInt) {
+        oiio_print_uncaught_errors = *(const int*)val;
+        return true;
+    }
     if (name == "imagebuf:print_uncaught_errors" && type == TypeInt) {
         imagebuf_print_uncaught_errors = *(const int*)val;
         return true;
@@ -480,6 +508,10 @@ getattribute(string_view name, TypeDesc type, void* val)
         *(int*)val = dds_bc5normal;
         return true;
     }
+    if (name == "oiio:print_uncaught_errors" && type == TypeInt) {
+        *(int*)val = oiio_print_uncaught_errors;
+        return true;
+    }
     if (name == "imagebuf:print_uncaught_errors" && type == TypeInt) {
         *(int*)val = imagebuf_print_uncaught_errors;
         return true;

src/libtexture/imagecache.cpp

@@ -2721,6 +2721,7 @@ ImageCacheImpl::get_image_info(ImageCacheFile* file,
             std::string* errptr = m_errormessage.get();
             if (errptr)
                 errptr->clear();
+            (void)OIIO::geterror(true);  // Suppress global errors
         }
         return true;
     }

src/oiiotool/oiiotool.cpp

@@ -5285,6 +5285,11 @@ input_file(Oiiotool& ot, cspan<const char*> argv)
             auto input = ImageInput::create(filename);
             if (input && input->supports("procedural"))
                 exists = 1;
+            if (!input) {
+                // If the create call failed, eat any stray global errors it
+                // may have issued.
+                (void)OIIO::geterror();
+            }
         }
         ImageBufRef substitute;  // possible substitute for missing image
         if (!exists) {