From 0d27f352259108809146722c17a151455b3ca9a4 Mon Sep 17 00:00:00 2001 From: Samuel Bharti Date: Wed, 27 May 2026 12:34:24 -0500 Subject: [PATCH 1/7] includeCSS(): clearer error when file is missing Check file.exists(path) up front and raise a structured rlang::abort() naming the missing file, instead of letting readLines() surface the generic "cannot open the connection" message. --- NEWS.md | 2 ++ R/tags.R | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/NEWS.md b/NEWS.md index 53e61e64..19f20d62 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # htmltools (development version) +* `includeCSS()` now raises a clear error naming the missing file when `path` does not exist, instead of the vague "cannot open the connection" message from `readLines()`. (rstudio/shiny#1757) + # htmltools 0.5.9 * Fix test for testthat 3.3.0. (#442) diff --git a/R/tags.R b/R/tags.R index 8848b393..ff9c9d74 100644 --- a/R/tags.R +++ b/R/tags.R @@ -1804,6 +1804,12 @@ includeMarkdown <- function(path) { #' @rdname include #' @export includeCSS <- function(path, ...) { + if (!file.exists(path)) { + rlang::abort(c( + "CSS file does not exist.", + "x" = paste0("Path: ", path) + )) + } lines <- readLines(path, warn=FALSE, encoding='UTF-8') args <- dots_list(...) if (is.null(args$type)) From 1513cf8e79d04939b5ce07df33ff0fc7f768f901 Mon Sep 17 00:00:00 2001 From: Samuel Bharti Date: Wed, 27 May 2026 12:42:42 -0500 Subject: [PATCH 2/7] Extend missing-file error to other include*() helpers Factor the file.exists() check into check_include_path() and apply it to includeHTML(), includeText(), includeMarkdown(), and includeScript() so they all raise a clear error naming the missing path, matching the behavior just added to includeCSS(). --- NEWS.md | 2 +- R/tags.R | 20 ++++++++++++++------ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/NEWS.md b/NEWS.md index 19f20d62..1dedb54b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,6 @@ # htmltools (development version) -* `includeCSS()` now raises a clear error naming the missing file when `path` does not exist, instead of the vague "cannot open the connection" message from `readLines()`. (rstudio/shiny#1757) +* `includeCSS()`, `includeHTML()`, `includeText()`, `includeMarkdown()`, and `includeScript()` now raise a clear error naming the missing file when `path` does not exist, instead of the vague "cannot open the connection" message from `readLines()`. (rstudio/shiny#1757) # htmltools 0.5.9 diff --git a/R/tags.R b/R/tags.R index ff9c9d74..5be2517d 100644 --- a/R/tags.R +++ b/R/tags.R @@ -1733,6 +1733,7 @@ knit_print.html_dependency <- knit_print.shiny.tag #' @aliases includeHTML #' @export includeHTML <- function(path) { + check_include_path(path, "HTML file") lines <- readLines(path, warn=FALSE, encoding='UTF-8') if (detect_html_document(lines)) { @@ -1781,6 +1782,7 @@ detect_html_document <- function(lines) { #' @rdname include #' @export includeText <- function(path) { + check_include_path(path, "Text file") lines <- readLines(path, warn=FALSE, encoding='UTF-8') return(paste8(lines, collapse='\n')) } @@ -1790,6 +1792,7 @@ includeText <- function(path) { #' @rdname include #' @export includeMarkdown <- function(path) { + check_include_path(path, "Markdown file") # markdown >= v1.3 switched from markdownToHTML() to mark() html <- if (packageVersion("markdown") < "1.3") { markdown::markdownToHTML(path, fragment.only = TRUE) @@ -1804,12 +1807,7 @@ includeMarkdown <- function(path) { #' @rdname include #' @export includeCSS <- function(path, ...) { - if (!file.exists(path)) { - rlang::abort(c( - "CSS file does not exist.", - "x" = paste0("Path: ", path) - )) - } + check_include_path(path, "CSS file") lines <- readLines(path, warn=FALSE, encoding='UTF-8') args <- dots_list(...) if (is.null(args$type)) @@ -1821,10 +1819,20 @@ includeCSS <- function(path, ...) { #' @rdname include #' @export includeScript <- function(path, ...) { + check_include_path(path, "Script file") lines <- readLines(path, warn=FALSE, encoding='UTF-8') return(tags$script(HTML(paste8(lines, collapse='\n')), ...)) } +check_include_path <- function(path, what = "File") { + if (!file.exists(path)) { + rlang::abort(c( + paste0(what, " does not exist."), + "x" = paste0("Path: ", path) + )) + } +} + #' Include content only once #' #' Use `singleton` to wrap contents (tag, text, HTML, or lists) that should From 67f4d2f12bc7c6e2ed38c188832d319c49af7819 Mon Sep 17 00:00:00 2001 From: Samuel Bharti Date: Wed, 27 May 2026 14:55:06 -0500 Subject: [PATCH 3/7] Add tests for missing-file error in include*() helpers Verify includeCSS(), includeHTML(), includeText(), includeMarkdown(), and includeScript() each raise a clear error naming both the file type and the missing path when given a non-existent file. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/testthat/test-tags.r | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/testthat/test-tags.r b/tests/testthat/test-tags.r index 195c1f86..6248f151 100644 --- a/tests/testthat/test-tags.r +++ b/tests/testthat/test-tags.r @@ -1226,3 +1226,20 @@ test_that("includeHTML() warns if full document is detected", { save_html(p("test"), tmp_html) expect_warning(includeHTML(tmp_html)) }) + +test_that("include*() helpers raise a clear error when the file is missing", { + missing <- tempfile("does-not-exist-", fileext = ".txt") + expect_false(file.exists(missing)) + + expect_error(includeCSS(missing), "CSS file does not exist") + expect_error(includeHTML(missing), "HTML file does not exist") + expect_error(includeText(missing), "Text file does not exist") + expect_error(includeScript(missing), "Script file does not exist") + skip_if_not_installed("markdown") + expect_error(includeMarkdown(missing), "Markdown file does not exist") +}) + +test_that("include*() error message names the missing path", { + missing <- tempfile("does-not-exist-", fileext = ".txt") + expect_error(includeCSS(missing), missing, fixed = TRUE) +}) From 9de3434a445ec5e53af0a7f71f707714e950eb9f Mon Sep 17 00:00:00 2001 From: Samuel Bharti Date: Wed, 27 May 2026 15:10:56 -0500 Subject: [PATCH 4/7] test(include): split markdown test and drop redundant assertion Move skip_if_not_installed("markdown") into its own test_that() so the skip sits at the top of the test, and remove the second test that only re-checked includeCSS(). Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/testthat/test-tags.r | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/testthat/test-tags.r b/tests/testthat/test-tags.r index 6248f151..e3b68f32 100644 --- a/tests/testthat/test-tags.r +++ b/tests/testthat/test-tags.r @@ -1231,15 +1231,14 @@ test_that("include*() helpers raise a clear error when the file is missing", { missing <- tempfile("does-not-exist-", fileext = ".txt") expect_false(file.exists(missing)) - expect_error(includeCSS(missing), "CSS file does not exist") - expect_error(includeHTML(missing), "HTML file does not exist") - expect_error(includeText(missing), "Text file does not exist") - expect_error(includeScript(missing), "Script file does not exist") - skip_if_not_installed("markdown") - expect_error(includeMarkdown(missing), "Markdown file does not exist") + expect_error(includeCSS(missing), "CSS file does not exist") + expect_error(includeHTML(missing), "HTML file does not exist") + expect_error(includeText(missing), "Text file does not exist") + expect_error(includeScript(missing), "Script file does not exist") }) -test_that("include*() error message names the missing path", { +test_that("includeMarkdown() raises a clear error when the file is missing", { + skip_if_not_installed("markdown") missing <- tempfile("does-not-exist-", fileext = ".txt") - expect_error(includeCSS(missing), missing, fixed = TRUE) + expect_error(includeMarkdown(missing), "Markdown file does not exist") }) From daa84524e2e1a04ac8fed9c927ea86db26c783c4 Mon Sep 17 00:00:00 2001 From: Samuel Bharti Date: Wed, 27 May 2026 15:11:44 -0500 Subject: [PATCH 5/7] fix(include): attribute missing-file errors to the public helper Pass call = rlang::caller_env() into rlang::abort() so the error header reads "Error in includeCSS():" (or the relevant helper) rather than "Error in check_include_path():", making the source of the problem easier for users to find. Co-Authored-By: Claude Opus 4.7 (1M context) --- R/tags.R | 13 ++++++++----- tests/testthat/test-tags.r | 6 ++++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/R/tags.R b/R/tags.R index 5be2517d..57dde044 100644 --- a/R/tags.R +++ b/R/tags.R @@ -1824,12 +1824,15 @@ includeScript <- function(path, ...) { return(tags$script(HTML(paste8(lines, collapse='\n')), ...)) } -check_include_path <- function(path, what = "File") { +check_include_path <- function(path, what = "File", call = rlang::caller_env()) { if (!file.exists(path)) { - rlang::abort(c( - paste0(what, " does not exist."), - "x" = paste0("Path: ", path) - )) + rlang::abort( + c( + paste0(what, " does not exist."), + "x" = paste0("Path: ", path) + ), + call = call + ) } } diff --git a/tests/testthat/test-tags.r b/tests/testthat/test-tags.r index e3b68f32..81bf3f68 100644 --- a/tests/testthat/test-tags.r +++ b/tests/testthat/test-tags.r @@ -1242,3 +1242,9 @@ test_that("includeMarkdown() raises a clear error when the file is missing", { missing <- tempfile("does-not-exist-", fileext = ".txt") expect_error(includeMarkdown(missing), "Markdown file does not exist") }) + +test_that("include*() missing-file error is attributed to the public helper", { + missing <- tempfile("does-not-exist-", fileext = ".txt") + err <- rlang::catch_cnd(includeCSS(missing)) + expect_match(rlang::expr_deparse(err$call), "includeCSS") +}) From 3c635cfb4b3b9048473dece9613b25ed260715d4 Mon Sep 17 00:00:00 2001 From: Samuel Bharti Date: Wed, 27 May 2026 15:12:07 -0500 Subject: [PATCH 6/7] fix(include): validate path input and reject directories Guard check_include_path() against two edge cases that previously surfaced confusing errors: - NULL, character(0), multi-element vectors, "", and NA_character_ inputs now raise a clear "path must be a single non-empty string" message instead of "argument is of length zero" from file.exists(). - A directory at `path` now raises the same "does not exist" error instead of the cryptic message from readLines(). Co-Authored-By: Claude Opus 4.7 (1M context) --- R/tags.R | 8 +++++++- tests/testthat/test-tags.r | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/R/tags.R b/R/tags.R index 57dde044..62cb9f5c 100644 --- a/R/tags.R +++ b/R/tags.R @@ -1825,7 +1825,13 @@ includeScript <- function(path, ...) { } check_include_path <- function(path, what = "File", call = rlang::caller_env()) { - if (!file.exists(path)) { + if (!rlang::is_string(path) || !nzchar(path)) { + rlang::abort( + paste0(what, " path must be a single non-empty string."), + call = call + ) + } + if (!file.exists(path) || dir.exists(path)) { rlang::abort( c( paste0(what, " does not exist."), diff --git a/tests/testthat/test-tags.r b/tests/testthat/test-tags.r index 81bf3f68..4fd3f8bf 100644 --- a/tests/testthat/test-tags.r +++ b/tests/testthat/test-tags.r @@ -1248,3 +1248,17 @@ test_that("include*() missing-file error is attributed to the public helper", { err <- rlang::catch_cnd(includeCSS(missing)) expect_match(rlang::expr_deparse(err$call), "includeCSS") }) + +test_that("include*() helpers reject invalid path inputs", { + expect_error(includeCSS(NULL), "must be a single non-empty string") + expect_error(includeCSS(character(0)), "must be a single non-empty string") + expect_error(includeCSS(c("a", "b")), "must be a single non-empty string") + expect_error(includeCSS(""), "must be a single non-empty string") + expect_error(includeCSS(NA_character_), "must be a single non-empty string") +}) + +test_that("include*() helpers reject a directory path", { + dir <- withr::local_tempdir() + expect_true(dir.exists(dir)) + expect_error(includeCSS(dir), "CSS file does not exist") +}) From d3975822c3a4f83de534c1eb57d77d63d7db6a33 Mon Sep 17 00:00:00 2001 From: Samuel Bharti Date: Wed, 27 May 2026 15:18:07 -0500 Subject: [PATCH 7/7] chore: nudge to trigger CI Co-Authored-By: Claude Opus 4.7 (1M context) --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index 1dedb54b..10ddeb9c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,7 @@ * `includeCSS()`, `includeHTML()`, `includeText()`, `includeMarkdown()`, and `includeScript()` now raise a clear error naming the missing file when `path` does not exist, instead of the vague "cannot open the connection" message from `readLines()`. (rstudio/shiny#1757) + # htmltools 0.5.9 * Fix test for testthat 3.3.0. (#442)