From ab1a0eec999be796272fc3f455654fa0e4f9f41e Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Mon, 9 Oct 2023 15:32:41 -0700 Subject: [PATCH] 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 --- src/include/OpenImageIO/imageio.h | 15 ++++++++++++++ src/libOpenImageIO/imageio.cpp | 34 ++++++++++++++++++++++++++++++- src/libtexture/imagecache.cpp | 1 + src/oiiotool/oiiotool.cpp | 5 +++++ 4 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/include/OpenImageIO/imageio.h b/src/include/OpenImageIO/imageio.h index ba92cba15d..c414d1d345 100644 --- a/src/include/OpenImageIO/imageio.h +++ b/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 diff --git a/src/libOpenImageIO/imageio.cpp b/src/libOpenImageIO/imageio.cpp index 09d11bf523..260da6b5bd 100644 --- a/src/libOpenImageIO/imageio.cpp +++ b/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 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; diff --git a/src/libtexture/imagecache.cpp b/src/libtexture/imagecache.cpp index 29f3ee405d..927dbd7fa9 100644 --- a/src/libtexture/imagecache.cpp +++ b/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; } diff --git a/src/oiiotool/oiiotool.cpp b/src/oiiotool/oiiotool.cpp index 9cd6851ec1..2e543176ac 100644 --- a/src/oiiotool/oiiotool.cpp +++ b/src/oiiotool/oiiotool.cpp @@ -5285,6 +5285,11 @@ input_file(Oiiotool& ot, cspan 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) {