Skip to content

Commit 9101fcd

Browse files
authored
Fix: helpful error from need() when message and label are missing (#4386)
* Fix: helpful error from `need()` when `message` and `label` are missing Previously, calling `need(expr)` without a `message` or `label` errored with the cryptic `argument "label" is missing, with no default`. Now it fails with a clear message naming the missing arguments. Fixes #2509 * refactor: address PR review feedback on `need()` error patch - Use `cli::cli_abort()` with inline markup instead of `stop(call. = FALSE)` to match the newer pattern used elsewhere in this dev cycle (e.g. `downloadButton()` in R/bootstrap.R). - Use `fixed = TRUE` in the `expect_error()` matchers instead of escaping parens in a regex. - Drop the stray blank line inside the original `need() works as expected` test_that block.
1 parent 603ac23 commit 9101fcd

3 files changed

Lines changed: 41 additions & 1 deletion

File tree

NEWS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434

3535
* Loading shiny no longer creates `.Random.seed` in the global environment as a side effect. (#4382)
3636

37+
* `need()` now gives a clearer error when called without either a `message` or `label` argument, instead of the cryptic "argument \"label\" is missing, with no default". (thanks @chasemc, #2509)
38+
3739
# shiny 1.13.0
3840

3941
## New features

R/utils.R

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1063,7 +1063,12 @@ validate <- function(..., errorClass = character(0)) {
10631063
#' @rdname validate
10641064
need <- function(expr, message = paste(label, "must be provided"), label) {
10651065

1066-
force(message) # Fail fast on message/label both being missing
1066+
if (missing(message) && missing(label)) {
1067+
cli::cli_abort(
1068+
"{.fn need} requires either a {.arg message} or {.arg label} argument."
1069+
)
1070+
}
1071+
force(message) # Fail fast in case `label` is missing but referenced via `message`
10671072

10681073
if (!isTruthy(expr))
10691074
return(message)

tests/testthat/test-utils.R

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,39 @@ test_that("need() works as expected", {
108108
expect_null(need(c(FALSE, FALSE, TRUE), FALSE))
109109
})
110110

111+
test_that("need() message/label handling (#2509)", {
112+
# Errors clearly when both `message` and `label` are missing, regardless of
113+
# whether `expr` is truthy or falsy (the message default is evaluated eagerly).
114+
expect_error(
115+
need(1 + 1),
116+
"`need()` requires either a `message` or `label` argument",
117+
fixed = TRUE
118+
)
119+
expect_error(
120+
need(NULL),
121+
"`need()` requires either a `message` or `label` argument",
122+
fixed = TRUE
123+
)
124+
125+
# `label` alone: default message is built from `label`
126+
expect_identical(need(NULL, label = "input$x"), "input$x must be provided")
127+
expect_null(need(1, label = "input$x"))
128+
129+
# `message` alone: used verbatim, no `label` needed
130+
expect_identical(need(NULL, message = "custom message"), "custom message")
131+
expect_null(need(1, message = "custom message"))
132+
133+
# `message = FALSE` is a documented "fail with no message" mode, and must
134+
# work without a `label`
135+
expect_false(need(NULL, FALSE))
136+
137+
# When both are provided, `message` wins (no reference to `label`)
138+
expect_identical(
139+
need(NULL, message = "explicit", label = "ignored"),
140+
"explicit"
141+
)
142+
})
143+
111144
test_that("req works", {
112145
expect_error(req(TRUE, FALSE))
113146
expect_error(req(TRUE, stop("boom")))

0 commit comments

Comments
 (0)