feat: warn when session$destroy(id) targets a non-existent module scope

session$destroy(id) now emits a warning (class "shiny.destroy.unknown_id")
when the id names a scope that was never created or has already been
destroyed, helping catch typos and stale ids. It's a warning rather than
an error so it doesn't break otherwise-working code, matching how Shiny
treats duplicate output ids (always warn).

Adds a private scopeExists() check to ShinySession and MockShinySession
that runs before makeScope(id) (which would otherwise register its own
callbacks and mask the check). Addresses @khusmann's request on #4372.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: schloerke

NEWS.md

@@ -8,7 +8,9 @@
   registered `onDestroy()` callbacks for that scope and its descendants,
   tearing down reactive values, expressions, and observers. A parent can
   also destroy a child module scope by id with `session$destroy(id)`, so it
-  can tear down a module using the same id it used to insert the UI (#4372).
+  can tear down a module using the same id it used to insert the UI.
+  Destroying an id with no live module scope (a typo or an already-destroyed
+  scope) emits a warning rather than an error (#4372).
 
 * New `startApp()` runs a Shiny app in non-blocking mode, returning a
   `ShinyAppHandle` object with `stop()`, `status()`, `url()`, and `result()`

R/mock-session.R

@@ -338,6 +338,7 @@ MockShinySession <- R6Class(
         stop("`$destroy()` cannot be called on the root session without an `id`. Pass a module `id` to tear down that scope (e.g. `session$destroy(\"my_module\")`), or call `$destroy()` on a module session.")
       }
       validateDestroyId(id)
+      if (!private$scopeExists(id)) warnDestroyUnknownId(id)
       self$makeScope(id)$destroy()
     },
 
@@ -587,6 +588,7 @@ MockShinySession <- R6Class(
             private$invokeDestroyCallbacks(namespace)
           } else {
             validateDestroyId(id)
+            if (!private$scopeExists(ns(id))) warnDestroyUnknownId(ns(id))
             self$makeScope(ns(id))$destroy()
           }
         }
@@ -770,6 +772,14 @@ MockShinySession <- R6Class(
       private$destroyCallbacksByNs$get(ns)
     },
 
+    # @description Does a module scope exist for this fully-qualified namespace?
+    #   See `ShinySession$scopeExists()`.
+    scopeExists = function(fullNs) {
+      keys <- private$destroyCallbacksByNs$keys()
+      prefix <- paste0(fullNs, ns.sep)
+      any(keys == fullNs | startsWith(keys, prefix))
+    },
+
     # @description Invoke destroy callbacks for the given namespace prefix
     #   and all child namespaces, deepest-first.
     # @param nsPrefix The namespace prefix to match.

R/shiny.R

@@ -172,6 +172,10 @@ workerId <- local({
 #'
 #'   On the root session an `id` is required; `session$destroy()` with no
 #'   `id` is an error (the root session is torn down via `close()`).
+#'
+#'   Destroying an `id` that has no module scope (because it was never
+#'   created or has already been destroyed) emits a warning rather than an
+#'   error, to help catch typos and stale ids.
 #' }
 #' \item{onEnded(callback)}{
 #'   Synonym for `onSessionEnded`.
@@ -449,6 +453,20 @@ validateDestroyId <- function(id) {
   invisible(id)
 }
 
+# Warn (rather than error) when `session$destroy(id)` targets a module scope
+# that doesn't exist -- either never created or already destroyed. Helps catch
+# typos and stale ids without breaking otherwise-working code. `fullNs` is the
+# fully-qualified namespace of the target scope.
+warnDestroyUnknownId <- function(fullNs) {
+  rlang::warn(
+    paste0(
+      "`session$destroy()`: no module scope found for id '", fullNs,
+      "'. It may have already been destroyed, or the id may be a typo."
+    ),
+    class = "shiny.destroy.unknown_id"
+  )
+}
+
 #' @include utils.R
 ShinySession <- R6Class(
   'ShinySession',
@@ -822,6 +840,16 @@ ShinySession <- R6Class(
       }
       private$destroyCallbacksByNs$get(key)
     },
+    # Does a module scope exist for this fully-qualified namespace? A scope is
+    # "alive" if it has destroy callbacks registered for itself or a descendant
+    # (every `moduleServer()`/`makeScope()` registers at least one). Returns
+    # FALSE for scopes that were never created or have already been destroyed
+    # (their callbacks are removed by `invokeDestroyCallbacks()`).
+    scopeExists = function(fullNs) {
+      keys <- private$destroyCallbacksByNs$keys()
+      prefix <- paste0(fullNs, ns.sep)
+      any(keys == fullNs | startsWith(keys, prefix))
+    },
     invokeDestroyCallbacks = function(nsPrefix = "") {
       allNs <- private$destroyCallbacksByNs$keys()
       isRoot <- !nzchar(nsPrefix)
@@ -1089,6 +1117,7 @@ ShinySession <- R6Class(
             private$invokeDestroyCallbacks(namespace)
           } else {
             validateDestroyId(id)
+            if (!private$scopeExists(ns(id))) warnDestroyUnknownId(ns(id))
             self$makeScope(ns(id))$destroy()
           }
         }
@@ -1293,6 +1322,7 @@ ShinySession <- R6Class(
         stop("`$destroy()` cannot be called on the root ShinySession without an `id`. Pass a module `id` to tear down that scope (e.g. `session$destroy(\"my_module\")`), or call `$destroy()` on a module session.")
       }
       validateDestroyId(id)
+      if (!private$scopeExists(id)) warnDestroyUnknownId(id)
       self$makeScope(id)$destroy()
     },
     onInputReceived = function(callback) {

man/session.Rd

@@ -481,9 +481,38 @@ test_that("module session$destroy(id) tears down a nested child scope", {
   expect_false(parent_called)
 })
 
-test_that("session$destroy(id) on an unknown id is a harmless no-op", {
+test_that("session$destroy(id) warns (non-fatally) on an unknown id", {
   session <- MockShinySession$new()
-  expect_no_error(session$destroy("never_created"))
+  expect_warning(
+    session$destroy("never_created"),
+    class = "shiny.destroy.unknown_id"
+  )
+})
+
+test_that("session$destroy(id) does not warn for a live scope", {
+  session <- MockShinySession$new()
+  session$makeScope("mod1")
+  expect_no_warning(session$destroy("mod1"))
+})
+
+test_that("session$destroy(id) warns when destroying an already-destroyed scope", {
+  session <- MockShinySession$new()
+  session$makeScope("mod1")
+  expect_no_warning(session$destroy("mod1"))
+  # Second destroy targets a stale id
+  expect_warning(
+    session$destroy("mod1"),
+    class = "shiny.destroy.unknown_id"
+  )
+})
+
+test_that("module session$destroy(id) warns on an unknown child id", {
+  session <- MockShinySession$new()
+  parent <- session$makeScope("parent")
+  expect_warning(
+    parent$destroy("nope"),
+    class = "shiny.destroy.unknown_id"
+  )
 })
 
 test_that("session$destroy(id) validates the id argument", {