Add RERUN_STRICT environment variable (#3861)

### What
If `RERUN_STRICT` is set to a truthy value (`1, ON, YES, TRUE`), then
errors in C++ and Python will be handled by throwing an exception.

We enabled this for our tests.

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/3861) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/3861)
- [Docs
preview](https://rerun.io/preview/80b86b08a4aa0ea8f6e68ce37fd92ce0b4512e22/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/80b86b08a4aa0ea8f6e68ce37fd92ce0b4512e22/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)

docs/code-examples/roundtrips.py

@@ -210,11 +210,15 @@ def run_example(example: str, language: str, args: argparse.Namespace) -> None:
 
 
 def roundtrip_env(*, save_path: str | None = None) -> dict[str, str]:
+    env = os.environ.copy()
+
     # NOTE: Make sure to disable batching, otherwise the Arrow concatenation logic within
     # the batcher will happily insert uninitialized padding bytes as needed!
-    env = os.environ.copy()
     env["RERUN_FLUSH_NUM_ROWS"] = "0"
 
+    # Turn on strict mode to catch errors early
+    env["RERUN_STRICT"] = "1"
+
     if save_path:
         # NOTE: Force the recording stream to write to disk!
         env["_RERUN_TEST_FORCE_SAVE"] = save_path

pixi.toml

@@ -37,7 +37,7 @@ cpp-prepare = "cmake -G Ninja -B build -S . -DCMAKE_BUILD_TYPE=Debug"
 cpp-build = { cmd = "cmake --build build --config Debug --target rerun_sdk_tests", depends_on = [
   "cpp-prepare",
 ] }
-cpp-test = { cmd = "./build/rerun_cpp/tests/rerun_sdk_tests", depends_on = [
+cpp-test = { cmd = "export RERUN_STRICT=1 && ./build/rerun_cpp/tests/rerun_sdk_tests", depends_on = [
   "cpp-build",
 ] }
 

rerun_cpp/src/rerun/error.cpp

@@ -3,7 +3,34 @@
 #include <arrow/status.h>
 #include <rerun.h>
 
+#include <algorithm> // For std::transform
+#include <cstdlib>   // For getenv
+#include <string>
+
 namespace rerun {
+    bool is_strict_mode() {
+        const char* env = getenv("RERUN_STRICT");
+        if (env == nullptr) {
+            return false;
+        }
+
+        std::string v = env;
+        std::transform(v.begin(), v.end(), v.begin(), [](char c) { return std::tolower(c); });
+
+        if (v == "1" || v == "true" || v == "yes" || v == "on") {
+            return true;
+        } else if (v == "0" || v == "false" || v == "no" || v == "off") {
+            return false;
+        } else {
+            fprintf(
+                stderr,
+                "Expected env-var RERUN_STRICT to be 0/1 true/false yes/no on/off, found '%s'",
+                env
+            );
+            return false;
+        }
+    }
+
     static StatusLogHandler global_log_handler = nullptr;
     static void* global_log_handler_user_data = nullptr;
 
@@ -75,11 +102,20 @@ namespace rerun {
         global_log_handler_user_data = userdata;
     }
 
-    void Error::log() const {
-        if (global_log_handler) {
+    void Error::handle() const {
+        if (is_ok()) {
+            // ok!
+        } else if (global_log_handler) {
             global_log_handler(*this, global_log_handler_user_data);
+        } else if (is_strict_mode()) {
+#ifdef __cpp_exceptions
+            throw_on_failure();
+#else
+            fprintf(stderr, "Rerun ERROR: %s\n", description.c_str());
+            abort();
+#endif
         } else {
-            fprintf(stderr, "ERROR: %s\n", description.c_str());
+            fprintf(stderr, "Rerun ERROR: %s\n", description.c_str());
         }
     }
 } // namespace rerun

rerun_cpp/src/rerun/error.hpp

@@ -14,12 +14,12 @@ namespace arrow {
 struct rr_error;
 
 /// Return error if a given rerun::Error producing expression is not rerun::ErrorCode::Ok.
-#define RR_RETURN_NOT_OK(status_expr)      \
-    do {                                   \
-        const auto _status_ = status_expr; \
-        if (_status_.is_err()) {           \
-            return _status_;               \
-        }                                  \
+#define RR_RETURN_NOT_OK(status_expr)              \
+    do {                                           \
+        const rerun::Error _status_ = status_expr; \
+        if (_status_.is_err()) {                   \
+            return _status_;                       \
+        }                                          \
     } while (false)
 
 namespace rerun {
@@ -115,29 +115,28 @@ namespace rerun {
             return code != ErrorCode::Ok;
         }
 
-        /// Sets global log handler called for `log` and `log_on_failure`.
+        /// Sets global log handler called for `handle`.
         ///
-        /// The default will log to stderr.
+        /// The default will log to stderr, unless `RERUN_STRICT` is set to something truthy.
         ///
         /// @param handler The handler to call, or `nullptr` to reset to the default.
         /// @param userdata Userdata pointer that will be passed to each invocation of the handler.
         ///
         /// @see log, log_on_failure
         static void set_log_handler(StatusLogHandler handler, void* userdata = nullptr);
 
-        /// Logs this status via the global log handler.
+        /// Handle this error based on the set log handler.
         ///
-        /// @see set_log_handler
-        void log() const;
-
-        /// Logs this status if failed via the global log handler.
+        /// If there is no error, nothing happens.
         ///
-        /// @see set_log_handler
-        void log_on_failure() const {
-            if (is_err()) {
-                log();
-            }
-        }
+        /// If you have set a log handler with `set_log_handler`, it will be called.
+        /// Else if the `RERUN_STRICT` env-var is set to something truthy,
+        /// an exception will be thrown (if `__cpp_exceptions` are enabled),
+        /// or the program will abort.
+        ///
+        /// If no log handler is installed, and we are not in strict mode,
+        /// the error will be logged to stderr.
+        void handle() const;
 
 #ifdef __cpp_exceptions
         /// Throws a `std::runtime_error` if the status is not `Ok`.

rerun_cpp/src/rerun/recording_stream.cpp

@@ -32,7 +32,7 @@ namespace rerun {
 
         rr_error status = {};
         this->_id = rr_recording_stream_new(&store_info, &status);
-        Error(status).log_on_failure();
+        Error(status).handle();
     }
 
     RecordingStream::RecordingStream(RecordingStream&& other)
@@ -97,25 +97,25 @@ namespace rerun {
     void RecordingStream::set_time_sequence(const char* timeline_name, int64_t sequence_nr) {
         rr_error status = {};
         rr_recording_stream_set_time_sequence(_id, timeline_name, sequence_nr, &status);
-        Error(status).log_on_failure(); // Too unlikely to fail to make it worth forwarding.
+        Error(status).handle(); // Too unlikely to fail to make it worth forwarding.
     }
 
     void RecordingStream::set_time_seconds(const char* timeline_name, double seconds) {
         rr_error status = {};
         rr_recording_stream_set_time_seconds(_id, timeline_name, seconds, &status);
-        Error(status).log_on_failure(); // Too unlikely to fail to make it worth forwarding.
+        Error(status).handle(); // Too unlikely to fail to make it worth forwarding.
     }
 
     void RecordingStream::set_time_nanos(const char* timeline_name, int64_t nanos) {
         rr_error status = {};
         rr_recording_stream_set_time_nanos(_id, timeline_name, nanos, &status);
-        Error(status).log_on_failure(); // Too unlikely to fail to make it worth forwarding.
+        Error(status).handle(); // Too unlikely to fail to make it worth forwarding.
     }
 
     void RecordingStream::disable_timeline(const char* timeline_name) {
         rr_error status = {};
         rr_recording_stream_disable_timeline(_id, timeline_name, &status);
-        Error(status).log_on_failure(); // Too unlikely to fail to make it worth forwarding.
+        Error(status).handle(); // Too unlikely to fail to make it worth forwarding.
     }
 
     void RecordingStream::reset_time() {

rerun_cpp/src/rerun/recording_stream.hpp

@@ -179,7 +179,7 @@ namespace rerun {
 
         /// Logs one or more archetype and/or component batches.
         ///
-        /// Logs any failure via `Error::log_on_failure`
+        /// Failures are handled with `Error::handle`.
         ///
         /// @param archetypes_or_component_batches
         /// Any type for which the `AsComponents<T>` trait is implemented.
@@ -188,8 +188,7 @@ namespace rerun {
         /// @see try_log
         template <typename... Ts>
         void log(const char* entity_path, const Ts&... archetypes_or_component_batches) {
-            try_log_with_timeless(entity_path, false, archetypes_or_component_batches...)
-                .log_on_failure();
+            try_log_with_timeless(entity_path, false, archetypes_or_component_batches...).handle();
         }
 
         /// Logs one or more archetype and/or component batches as timeless data.
@@ -198,7 +197,7 @@ namespace rerun {
         /// far into the past. All timestamp data associated with this message will be dropped right
         /// before sending it to Rerun.
         ///
-        /// Logs any failure via `Error::log_on_failure`
+        /// Failures are handled with `Error::handle`.
         ///
         /// @param archetypes_or_component_batches
         /// Any type for which the `AsComponents<T>` trait is implemented.
@@ -207,8 +206,7 @@ namespace rerun {
         /// @see try_log
         template <typename... Ts>
         void log_timeless(const char* entity_path, const Ts&... archetypes_or_component_batches) {
-            try_log_with_timeless(entity_path, true, archetypes_or_component_batches...)
-                .log_on_failure();
+            try_log_with_timeless(entity_path, true, archetypes_or_component_batches...).handle();
         }
 
         /// Logs one or more archetype and/or component batches.

rerun_py/rerun_sdk/rerun/error_utils.py

@@ -2,6 +2,7 @@
 
 import functools
 import inspect
+import os
 import threading
 import warnings
 from types import TracebackType
@@ -15,9 +16,24 @@
 
 _TFunc = TypeVar("_TFunc", bound=Callable[..., Any])
 
+
+def default_strict_mode() -> bool:
+    if "RERUN_STRICT" in os.environ:
+        var = os.environ["RERUN_STRICT"].lower()
+        if var in ("0", "false", "off", "no"):
+            return False
+        elif var in ("1", "true", "on", "yes"):
+            return True
+        else:
+            print(f"Expected RERUN_STRICT to be one of 0/1 false/true off/on no/yes, found {var}")
+            return _strict_mode
+    else:
+        return False
+
+
 # If `True`, we raise exceptions on use error (wrong parameter types, etc.).
 # If `False` we catch all errors and log a warning instead.
-_strict_mode = False
+_strict_mode = default_strict_mode()
 
 _rerun_exception_ctx = threading.local()
 
@@ -30,7 +46,8 @@ def strict_mode() -> bool:
     will result in exception being raised.
     When strict mode is on, such problems are instead logged as warnings.
 
-    The default is OFF.
+    The default is controlled with the `RERUN_STRICT` environment variable,
+    or `False` if it is not set.
     """
     # If strict was set explicitly, we are in struct mode
     if getattr(_rerun_exception_ctx, "strict_mode", None) is not None:
@@ -47,7 +64,8 @@ def set_strict_mode(mode: bool) -> None:
     will result in exception being raised.
     When strict mode is off, such problems are instead logged as warnings.
 
-    The default is OFF.
+    The default is controlled with the `RERUN_STRICT` environment variable,
+    or `False` if it is not set.
     """
     global _strict_mode
 

scripts/run_python_e2e_test.py

@@ -81,7 +81,7 @@ def main() -> None:
     print("All tests passed successfully!")
 
 
-def run_example(example: str, args: list[str]) -> None:
+def run_example(example: str, extra_args: list[str]) -> None:
     # sys.executable: the absolute path of the executable binary for the Python interpreter
     python_executable = sys.executable
     if python_executable is None:
@@ -92,7 +92,10 @@ def run_example(example: str, args: list[str]) -> None:
     )
     time.sleep(0.3)  # Wait for rerun server to start to remove a logged warning
 
-    python_process = subprocess.Popen([python_executable, example, "--connect", "--addr", f"127.0.0.1:{PORT}"] + args)
+    run_env = os.environ.copy()
+    run_env["RERUN_STRICT"] = "1"
+    cmd = [python_executable, example, "--connect", "--addr", f"127.0.0.1:{PORT}"] + extra_args
+    python_process = subprocess.Popen(cmd, env=run_env)
 
     print("Waiting for python process to finish…")
     returncode = python_process.wait(timeout=30)

tests/roundtrips.py

@@ -155,10 +155,15 @@ def main() -> None:
 
 
 def roundtrip_env() -> dict[str, str]:
+    env = os.environ.copy()
+
     # NOTE: Make sure to disable batching, otherwise the Arrow concatenation logic within
     # the batcher will happily insert uninitialized padding bytes as needed!
-    env = os.environ.copy()
     env["RERUN_FLUSH_NUM_ROWS"] = "0"
+
+    # Turn on strict mode to catch errors early
+    env["RERUN_STRICT"] = "1"
+
     return env