diff --git a/DESCRIPTION b/DESCRIPTION index 8638a351..25e6b581 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -32,6 +32,7 @@ Suggests: pander, parallel, R.utils, + rlang, rmarkdown, roxygen2, RPushbullet, diff --git a/NEWS.md b/NEWS.md index 5edf5e75..dfb790eb 100644 --- a/NEWS.md +++ b/NEWS.md @@ -7,6 +7,7 @@ * `log_appender()`, `log_layout()` and `log_formatter()` now check that you are calling them with a function, and return the previously set value (#170, @hadley) * new function to return number of log indices (#194, @WurmPeter) * `appender_async` is now using `mirai` instead of a custom background process and queue system (#214, @hadley @shikokuchuo) +* If `rlang` is installed, errors in `formatter_glue()` and `formatter_glue_safe()` get improved appearance ## Fixes diff --git a/R/formatters.R b/R/formatters.R index 34502d12..df6f0f68 100644 --- a/R/formatters.R +++ b/R/formatters.R @@ -44,21 +44,13 @@ formatter_glue <- function(..., .topenv = parent.frame()) { fail_on_missing_package("glue") - withCallingHandlers( + out <- wrap_glue( glue::glue(..., .envir = .topenv), - error = function(e) { - args <- paste0(capture.output(str(...)), collapse = "\n") - - stop(paste0( - "`glue` failed in `formatter_glue` on:\n\n", - args, - "\n\nRaw error message:\n\n", - conditionMessage(e), - "\n\nPlease consider using another `log_formatter` or ", - "`skip_formatter` on strings with curly braces." - )) - } + inputs = list(...), + glue_fun = "glue", + logger_fun = "formatter_glue" ) + as.character(out) } attr(formatter_glue, "generator") <- quote(formatter_glue()) @@ -75,24 +67,64 @@ formatter_glue_safe <- function(..., .topcall = sys.call(-1), .topenv = parent.frame()) { fail_on_missing_package("glue") - as.character( - tryCatch( - glue::glue_safe(..., .envir = .topenv), - error = function(e) { - stop(paste( - "`glue_safe` failed in `formatter_glue_safe` on:\n\n", - capture.output(str(...)), + + out <- wrap_glue( + glue::glue_safe(..., .envir = .topenv), + inputs = list(...), + glue_fun = "glue_safe", + logger_fun = "formatter_glue_safe" + ) + as.character(out) +} +attr(formatter_glue_safe, "generator") <- quote(formatter_glue_safe()) + +wrap_glue <- function(code, inputs, glue_fun, logger_fun) { + if (has_rlang()) { + withCallingHandlers( + code, + error = function(cnd) { + rlang::abort( + c( + glue::glue("`{glue_fun}()` failed."), + i = glue_hint(conditionMessage(cnd)) + ), + parent = cnd, + call = rlang::call2(logger_fun) + ) + } + ) + } else { + withCallingHandlers( + code, + error = function(cnd) { + hint <- glue_hint(conditionMessage(cnd)) + if (!is.null(hint)) hint <- paste0("\n\n", hint) + + stop(paste0( + "`", glue_fun, "()` failed in `", logger_fun, "()` on:\n\n", + paste0(capture.output(str(inputs)), collapse = ""), "\n\nRaw error message:\n\n", - e$message, - "\n\nPlease consider using another `log_formatter` or", - "`skip_formatter` on strings with curly braces." - )) + conditionMessage(cnd), + hint + ), call. = FALSE) } ) + } +} + +glue_hint <- function(message) { + if (!grepl("}", message, fixed = TRUE)) { + return() + } + paste0( + "For strings containing `{` or `}` consider using ", + "`skip_formatter()` or another `log_formatter`." ) } -attr(formatter_glue_safe, "generator") <- quote(formatter_glue_safe()) +has_rlang <- function() { + requireNamespace("rlang", quietly = TRUE) +} #' Apply `glue` and `sprintf` #' diff --git a/tests/testthat/_snaps/formatters.md b/tests/testthat/_snaps/formatters.md index 5bbfc0ec..23157bd5 100644 --- a/tests/testthat/_snaps/formatters.md +++ b/tests/testthat/_snaps/formatters.md @@ -19,16 +19,27 @@ Code log_info("hi{") Condition - Error in `h()`: - ! `glue` failed in `formatter_glue` on: + Error in `formatter_glue()`: + ! `glue()` failed. + i For strings containing `{` or `}` consider using `skip_formatter()` or another `log_formatter`. + Caused by error in `glue_data()`: + ! Expecting '}' + +--- + + Code + log_info("hi{") + Condition + Error: + ! `glue()` failed in `formatter_glue()` on: - chr "hi{" + List of 1 $ : chr "hi{" Raw error message: Expecting '}' - Please consider using another `log_formatter` or `skip_formatter` on strings with curly braces. + For strings containing `{` or `}` consider using `skip_formatter()` or another `log_formatter`. # glue_safe works diff --git a/tests/testthat/test-formatters.R b/tests/testthat/test-formatters.R index 165fa910..46b15be7 100644 --- a/tests/testthat/test-formatters.R +++ b/tests/testthat/test-formatters.R @@ -38,6 +38,9 @@ test_that("glue works", { test_that("glue gives informative error if message contains curlies", { local_test_logger(formatter = formatter_glue) expect_snapshot(log_info("hi{"), error = TRUE) + + local_mocked_bindings(has_rlang = function() FALSE) + expect_snapshot(log_info("hi{"), error = TRUE) }) test_that("glue_safe works", {