r-lib / rlang

Low-level API for programming with R
https://rlang.r-lib.org
Other
502 stars 140 forks source link

Possible rlang/reticulate issue causing test traceback failure in R CMD CHK #1637

Closed pbulsink closed 1 year ago

pbulsink commented 1 year ago

Hello

We have a periodic failure in build checks at scasanova/f1dataR an issue in our reticulate/R interface. I don't think the specifics of that are important, but what happens in the build when we have this issue is a failure to properly present any traceback:

  [ FAIL 2 | WARN 0 | SKIP 0 | PASS 87 ]

  ══ Failed tests ════════════════════════════════════════════════════════════════
  Error in if (is.null(x[["trace"]]) || trace_length(x[["trace"]]) == 0L) { : 
    missing value where TRUE/FALSE needed
  Calls: test_check ... vapply -> FUN -> paste0 -> format -> format.expectation
  Execution halted

Test traceback failure can be observed at this link: https://github.com/SCasanova/f1dataR/actions/runs/5385032984/jobs/9773619355

As far as I can tell this issue randomly pops up after failures in our reticulate to R usage and is not reproducible wrt os or R version (> 4.0). The exact cause of the failure is difficult to know because of the failure of traceback.

If you feel this isn't related to rlang please feel free to close.

harupy commented 1 year ago

Same here: https://github.com/mlflow/mlflow/actions/runs/5473056182/jobs/9966022819?pr=8978

Error in if (is.null(x[["trace"]]) || trace_length(x[["trace"]]) == 0L) { : 
  missing value where TRUE/FALSE needed
Calls: source ... vapply -> FUN -> paste0 -> format -> format.expectation
Execution halted
══ Failed tests ════════════════════════════════════════════════════════════════
lionel- commented 1 year ago

cc @t-kalinowski

t-kalinowski commented 1 year ago

I've observed something similar intermittently, but don't have a reprex currently and haven't had a chance to investigate yet. I think it's most likely due to dueling calling handlers between testthat and reticulate.

Running the code being tested directly, outside a testthat context, will show the actual error message.

t-kalinowski commented 1 year ago

Attempting a reprex, I'm seeing a different but related issue w/ testthat injecting rlang condition handlers that error on R conditions wrapping Python Exceptions:

test_that("test", {
  skip_if_no_python()

  expect_no_error({
    py_fn <- py_run_string("def fn(): raise Exception()")$fn
    py_fn()
  })
})

produces:

── Error (Line 4): test ────────────────────────────────────────────────────────
Error in `cnd_type(cnd)`: `cnd` is not a condition object.
Backtrace:
     ▆
  1. ├─testthat::expect_no_error(...)
  2. │ └─testthat:::expect_no_(...)
  3. │   └─testthat:::quasi_capture(enquo(object), NULL, capture)
  4. │     ├─testthat (local) .capture(...)
  5. │     │ └─rlang::try_fetch(...)
  6. │     │   ├─base::tryCatch(...)
  7. │     │   │ └─base (local) tryCatchList(expr, classes, parentenv, handlers)
  8. │     │   │   └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
  9. │     │   │     └─base (local) doTryCatch(return(expr), name, parentenv, handler)
 10. │     │   └─base::withCallingHandlers(...)
 11. │     └─rlang::eval_bare(quo_get_expr(.quo), quo_get_env(.quo))
 12. ├─reticulate (local) py_fn()
 13. │ └─reticulate:::py_call_impl(callable, call_args$unnamed, call_args$named) at reticulate/R/python.R:1437:4
 14. ├─base::stop(`<python.builtin.Exception>`)
 15. ├─rlang (local) `<fn>`(`<python.builtin.Exception>`)
 16. │ └─handlers[[1L]](cnd)
 17. │   ├─base::paste0(...)
 18. │   ├─testthat:::indent_lines(rlang::cnd_message(cnd, prefix = TRUE))
 19. │   │ ├─base::paste0("  ", gsub("\n", "\n  ", x))
 20. │   │ └─base::gsub("\n", "\n  ", x)
 21. │   │   └─base::is.factor(x)
 22. │   └─rlang::cnd_message(cnd, prefix = TRUE)
 23. │     └─rlang:::cnd_message_format_prefixed(cnd, ..., parent = FALSE)
 24. │       └─rlang::cnd_type(cnd)
 25. └─rlang::abort(message = message)

A more minimal reprex:

ex <-reticulate::py_eval("Exception('msg')")

# ex is a condition, fulfills the condition contract
inherits(ex, "condition")
#> [1] TRUE
cat(conditionMessage(ex))
#> Exception: msg
#> Run `reticulate::py_last_error()` for details.
conditionCall(ex)
#> NULL

# cnd_type(ex) signals an error
rlang::cnd_type(ex)
#> Error in `rlang::cnd_type()`:
#> ! `cnd` is not a condition object.
#> Backtrace:
#>     ▆
#>  1. ├─rlang::cnd_type(ex)
#>  2. └─rlang::abort(message = message)

Created on 2023-07-06 with reprex v2.0.2

t-kalinowski commented 1 year ago

The actual error is being raised from testthat: https://github.com/r-lib/testthat/blob/53cd6dd53eeaadd4229c4702f380ca283ec74b52/R/expectation.R#L165

My best guess is that trace_length() is returning an NA or length 0 vector, which is causing || to error.

pbulsink commented 1 year ago

As this looks like a testthat issue I've raised it there.

t-kalinowski commented 1 year ago

This was fixed with https://github.com/r-lib/rlang/issues/1637; this issue can be closed.

lionel- commented 1 year ago

Thanks @t-kalinowski!