From 0e473ba45392a7963ca5534594368fb86b66eedf Mon Sep 17 00:00:00 2001 From: Kara Woo Date: Thu, 12 Feb 2026 14:04:39 -0800 Subject: [PATCH 1/4] add failing test --- tests/testthat/test-content.R | 54 +++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/tests/testthat/test-content.R b/tests/testthat/test-content.R index bdc9b4e40..4d4822725 100644 --- a/tests/testthat/test-content.R +++ b/tests/testthat/test-content.R @@ -662,3 +662,57 @@ test_that("lock_content() and unlock_content() error on Connect < 2024.08.0", { "ERROR: This feature requires Posit Connect version 2024.08.0 but you are using 2024.07.0." ) }) + +test_that("content_restart retries if delete fails", { + mock_connect <- MockConnect$new("2025.09.0") + mock_connect$mock_response( + "GET", + "v1/content/test-guid", + content = list( + guid = "test-guid", + name = "test-content", + title = "Test Content", + app_mode = "shiny", + content_url = "https://connect.example/content/test-guid/", + dashboard_url = "https://connect.example/connect/#/apps/test-guid" + ) + ) + + # First PATCH to set the env var (succeeds) + mock_connect$mock_response( + "PATCH", + "v1/content/test-guid/environment", + content = list(), + status_code = 200L + ) + + # Second PATCH to delete the env var (fails with 500) + mock_connect$mock_response( + "PATCH", + "v1/content/test-guid/environment", + content = list(error = "Internal server error"), + status_code = 500L + ) + + # Third PATCH to delete the env var (retry succeeds) + mock_connect$mock_response( + "PATCH", + "v1/content/test-guid/environment", + content = list(), + status_code = 200L + ) + + # Get the content item + item <- content_item(mock_connect, "test-guid") + + expect_no_error(content_restart(item)) + expect_equal( + mock_connect$call_log, + c( + "GET https://connect.example/__api__/v1/content/test-guid", + "PATCH https://connect.example/__api__/v1/content/test-guid/environment", + "PATCH https://connect.example/__api__/v1/content/test-guid/environment", + "PATCH https://connect.example/__api__/v1/content/test-guid/environment" + ) + ) +}) From 0c91b065bf52669c781f93bebde29e94ec485b51 Mon Sep 17 00:00:00 2001 From: Kara Woo Date: Thu, 12 Feb 2026 16:06:35 -0800 Subject: [PATCH 2/4] add a retry --- R/content.R | 18 ++++++++++++++- tests/testthat/test-content.R | 42 +++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/R/content.R b/R/content.R index 22efc2101..e1e63c6fe 100644 --- a/R/content.R +++ b/R/content.R @@ -1434,7 +1434,23 @@ content_restart <- function(content) { # https://rlang.r-lib.org/reference/glue-operators.html#using-glue-syntax-in-packages env_var_name <- glue::glue("_CONNECT_RESTART_{unix_epoch_in_seconds}") content$environment_set("{env_var_name}" := unix_epoch_in_seconds) - content$environment_set("{env_var_name}" := NA) + tryCatch( + { + content$environment_set("{env_var_name}" := NA) + }, + error = function(e) { + # we sometimes see the rapid set/unset lead to failures, which leave the + # env var in place. If we get a 500 error, wait a second and try one more + # time + if (grepl("500", conditionMessage(e))) { + Sys.sleep(1) + content$environment_set("{env_var_name}" := NA) + } else { + stop(e) + } + } + ) + # nolint end invisible(NULL) } diff --git a/tests/testthat/test-content.R b/tests/testthat/test-content.R index 4d4822725..63b96c1bc 100644 --- a/tests/testthat/test-content.R +++ b/tests/testthat/test-content.R @@ -716,3 +716,45 @@ test_that("content_restart retries if delete fails", { ) ) }) + +test_that("content_restart doesn't retry if error isn't a 500", { + mock_connect <- MockConnect$new("2025.09.0") + mock_connect$mock_response( + "GET", + "v1/content/test-guid", + content = list( + guid = "test-guid", + name = "test-content", + title = "Test Content", + app_mode = "shiny", + content_url = "https://connect.example/content/test-guid/", + dashboard_url = "https://connect.example/connect/#/apps/test-guid" + ) + ) + + mock_connect$mock_response( + "PATCH", + "v1/content/test-guid/environment", + content = list(), + status_code = 200L + ) + + mock_connect$mock_response( + "PATCH", + "v1/content/test-guid/environment", + content = list(error = "Bad request"), + status_code = 400L + ) + + item <- content_item(mock_connect, "test-guid") + expect_error(content_restart(item)) + # only 2 PATCHes + expect_equal( + mock_connect$call_log, + c( + "GET https://connect.example/__api__/v1/content/test-guid", + "PATCH https://connect.example/__api__/v1/content/test-guid/environment", + "PATCH https://connect.example/__api__/v1/content/test-guid/environment" + ) + ) +}) From 2e673db3901fb8ae755d66dc9b3673a8a53e968b Mon Sep 17 00:00:00 2001 From: Kara Woo Date: Thu, 12 Feb 2026 16:10:14 -0800 Subject: [PATCH 3/4] add news --- NEWS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS.md b/NEWS.md index f885fe5dc..8ab061348 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ # connectapi (development version) +- Added a single retry to `content_restart()` to more robustly clean up + temporary environment variables. (#498) + # connectapi 0.10.0 - Removed `prefix` argument to `connect()` function, which was a convenience utility for testing. (#477) From b8974425ed1d3e63e0fb17291f2b0f72ae8018a8 Mon Sep 17 00:00:00 2001 From: Kara Woo Date: Fri, 13 Feb 2026 11:01:33 -0800 Subject: [PATCH 4/4] warn instead of error if unable to clean up env var --- R/content.R | 19 +++++++++++++++++-- tests/testthat/test-content.R | 2 +- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/R/content.R b/R/content.R index e1e63c6fe..486c7e398 100644 --- a/R/content.R +++ b/R/content.R @@ -1444,9 +1444,24 @@ content_restart <- function(content) { # time if (grepl("500", conditionMessage(e))) { Sys.sleep(1) - content$environment_set("{env_var_name}" := NA) + tryCatch( + content$environment_set("{env_var_name}" := NA), + error = function(e) { + warning( + glue::glue( + "Restarted content by setting environment variable {env_var_name}, but was unable to clean it up. See ?set_environment_remove for how to manually remove this variable." + ), + call. = FALSE + ) + } + ) } else { - stop(e) + warning( + glue::glue( + "Restarted content by setting environment variable {env_var_name}, but was unable to clean it up. See ?set_environment_remove for how to manually remove this variable." + ), + call. = FALSE + ) } } ) diff --git a/tests/testthat/test-content.R b/tests/testthat/test-content.R index 63b96c1bc..164e47064 100644 --- a/tests/testthat/test-content.R +++ b/tests/testthat/test-content.R @@ -747,7 +747,7 @@ test_that("content_restart doesn't retry if error isn't a 500", { ) item <- content_item(mock_connect, "test-guid") - expect_error(content_restart(item)) + expect_warning(content_restart(item)) # only 2 PATCHes expect_equal( mock_connect$call_log,