Improve error messages for missing required input arguments#4389
Open
samuelbharti wants to merge 3 commits into
Open
Improve error messages for missing required input arguments#4389samuelbharti wants to merge 3 commits into
samuelbharti wants to merge 3 commits into
Conversation
Input constructors (textInput(), sliderInput(), selectInput(), actionButton(), etc.) now report which function and which argument were missing instead of surfacing a downstream R error like 'argument "label" is missing, with no default'. Adds an internal check_required() helper in R/input-utils.R that uses rlang::is_missing() against the caller's frame and cli::cli_abort() to name both the function and the missing argument(s). The helper is wired into the top of every *Input() constructor and actionButton() / actionLink() for arguments without defaults. Fixes rstudio#1423.
- Replace nested rlang call construction with eval(call("missing", ...))
for readability; behavior unchanged.
- Classed error: cli_abort() now raises with class
"shiny_missing_arg_error" so callers can catch it specifically.
- Test additions: verify the classed error, verify behavior under
do.call(), and tighten happy-path assertions to confirm each
constructor returns a shiny.tag.
Match the existing cadence for issue-reporter credits, e.g. "(thanks @ismirsehregal, rstudio#4318)" on NEWS.md:177.
1193f11 to
81af13b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1423.
Summary
Input constructors (
textInput(),sliderInput(),selectInput(),actionButton(), etc.) currently surface cryptic downstream R errors likeargument "label" is missing, with no defaultwhen a required argument is omitted, without naming the function or the argument that was actually missing.This PR adds a small internal
check_required()helper inR/input-utils.Rthat useseval(call("missing", ...), envir = caller_env())to detect unsupplied promises without forcing them, then raises a classed error viacli::cli_abort()that names both the calling function and the missing argument(s). The helper is wired into the top of every*Input()constructor plusactionButton()/actionLink()for arguments without defaults (inputId,label, plus per-function specifics such aschoices,min/max/value,data).The error carries class
shiny_missing_arg_errorso downstream code can catch it specifically. ExplicitNULLandNAare treated as "supplied" so documented opt-outs likelabel = NULLcontinue to work. No public API changes; only the error wording changes for the previously-broken cases.Scope
In scope: every standalone input constructor and
actionButton()/actionLink()— 15 functions in total.Out of scope (intentionally):
inputId = c("a", "b")). The issue is about clearer missing-arg messages, not a general validator.sliderInput("forgot my id!", 1, 10, 5)will still reportvalueas missing because we can't infer the user's intent.update*Input()family. Same problem exists there but the surface area is larger; tracked as a follow-up.Follow-ups
check_required()treatment toupdateTextInput(),updateSliderInput(),updateSelectInput(), etc.check_required()(or replacing withrlang::check_required()) if other parts of shiny adopt the pattern.Verification
Before:
After:
Test plan
tests/testthat/test-input-required-args.Radds 7test_that()blocks (48 expectations) covering: the helper's own behavior, the new error class, behavior underdo.call(), regex-based regression assertions, explicit-NULL opt-out, happy-pathshiny.tagreturns for all 15 patched constructors, and full-message snapshots.devtools::test(filter = "input"): 231 PASS, 0 FAIL.devtools::test()suite earlier: 2018 PASS, 0 FAIL (12 environmental skips, unrelated).