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

Allow reticulate errors in cnd_type() (fixes #1664) #1693

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# rlang (development version)

* Fix for reticulate errors (#1664)

* `last_trace()` hyperlinks now use the modern `x-r-run` format (#1678).


Expand Down
3 changes: 2 additions & 1 deletion src/rlang/cnd.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ void r_interrupt(void) {

enum r_cnd_type r_cnd_type(r_obj* cnd) {
r_obj* classes = r_class(cnd);
if (r_typeof(cnd) != R_TYPE_list ||
if (
Copy link
Member

@lionel- lionel- May 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing this will cause R to crash if the caller assumes (rightfully IMO) that a condition is a list. Note that the code modified here is part of a C-level API used in multiple packages.

(r_typeof(cnd) != R_TYPE_list && r_typeof(cnd) != R_TYPE_environment) ||
r_typeof(classes) != R_TYPE_character) {
goto error;
}
Expand Down
7 changes: 5 additions & 2 deletions tests/testthat/test-cnd.R
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,8 @@ test_that("calls are consistently displayed on rethrow (#1240)", {
expr = force(expr),
error = function(cnd) {
rlang::abort(
message = "Problem while executing step.",
call = call(step_name),
message = "Problem while executing step.",
call = call(step_name),
parent = cnd
)
}
Expand Down Expand Up @@ -252,6 +252,9 @@ test_that("cnd_type() detects condition type", {
expect_identical(cnd_type(warning_cnd()), "warning")
expect_identical(cnd_type(error_cnd()), "error")
expect_identical(cnd_type(catch_cnd(interrupt())), "interrupt")
# reticulate stores error conditions as environments (#1664)
env_cnd <- structure(env(), class = c("error", "condition"))
expect_identical(cnd_type(env_cnd), "error")
})

test_that("bare conditions must be subclassed", {
Expand Down
Loading