diff --git a/NEWS.md b/NEWS.md index f885fe5d..8ab06134 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) diff --git a/R/content.R b/R/content.R index 22efc210..486c7e39 100644 --- a/R/content.R +++ b/R/content.R @@ -1434,7 +1434,38 @@ 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) + 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 { + 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 + ) + } + } + ) + # nolint end invisible(NULL) } diff --git a/tests/testthat/test-content.R b/tests/testthat/test-content.R index bdc9b4e4..164e4706 100644 --- a/tests/testthat/test-content.R +++ b/tests/testthat/test-content.R @@ -662,3 +662,99 @@ 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" + ) + ) +}) + +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_warning(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" + ) + ) +})