Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RERUN_STRICT environment variable #3861

Merged
merged 7 commits into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion docs/code-examples/roundtrips.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pixi.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
] }

Expand Down
42 changes: 39 additions & 3 deletions rerun_cpp/src/rerun/error.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
37 changes: 18 additions & 19 deletions rerun_cpp/src/rerun/error.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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`.
Expand Down
10 changes: 5 additions & 5 deletions rerun_cpp/src/rerun/recording_stream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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() {
Expand Down
10 changes: 4 additions & 6 deletions rerun_cpp/src/rerun/recording_stream.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand Down
24 changes: 21 additions & 3 deletions rerun_py/rerun_sdk/rerun/error_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import functools
import inspect
import os
import threading
import warnings
from types import TracebackType
Expand All @@ -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()

Expand All @@ -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:
Expand All @@ -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

Expand Down
7 changes: 5 additions & 2 deletions scripts/run_python_e2e_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion tests/roundtrips.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
Loading