Force closing of context before exiting CLIs to provoque callback cal…

…ls and prevent state leaks

Refs: pallets/click#2680

changelog.md

@@ -7,7 +7,7 @@ This version is not released yet and is under active development.
 ```
 
 - Allow standalone `--version` option to output its debug messages.
-- Force callback calls in all standalone options before exiting to prevent state leaks in non-`ExtraContext`-based CLIs.
+- Force closing of context before exiting CLIs to provoque callback calls and prevent state leaks.
 - Run tests on `macos-14`. Remove tests on `macos-12`.
 
 ## [4.7.3 (2024-01-06)](https://github.com/kdeldycke/click-extra/compare/v4.7.2...v4.7.3)

click_extra/colorize.py

@@ -342,12 +342,6 @@ def print_help(ctx: Context, param: Parameter, value: bool) -> None:
         """
         if value and not ctx.resilient_parsing:
             echo(ctx.get_help(), color=ctx.color)
-            # XXX We need to explicitly close the context before exiting, so all callbacks
-            # from options which cleans up CLI state are properly invoked.
-            # We need to perform this here so combinations of standalone options works
-            # well, and not only in our custom ExtraContext. This is a follow up on
-            # 936a8f5eff916bad5dfef1b4696c09b42a23ca61.
-            ctx.close()
             ctx.exit()
 
 

click_extra/commands.py

@@ -44,6 +44,28 @@
 if TYPE_CHECKING:
     from typing import NoReturn
 
+from click.exceptions import Exit
+
+
+def patched_exit(self, code: int = 0) -> NoReturn:
+    """Exits the application with a given exit code.
+
+    Forces the context to close before exiting, so callbacks attached to parameters
+    will be called to clean up their state. This is not important in normal CLI
+    execution as the Python process will just be destroyed. But it will lead to leaky
+    states in unitttests.
+
+    .. seealso::
+        This fix has been `proposed upstream to Click
+        <https://github.com/pallets/click/pull/2680>`_.
+    """
+    self.close()
+    raise Exit(code)
+
+
+cloup.Context.exit = patched_exit
+"""Monkey-patch ``cloup.Context.exit``."""
+
 
 class ExtraContext(cloup.Context):
     """Like ``cloup._context.Context``, but with the ability to populate the context's
@@ -101,18 +123,6 @@ def color(self) -> None:
         """Reset the color value so it defaults to inheritance from parent's."""
         self._color = None
 
-    def exit(self, code: int = 0) -> NoReturn:
-        """Exits the application with a given exit code.
-
-        Forces the context to close before exiting, so callbacks attached to parameters
-        will be called to clean up their state.
-
-        .. todo::
-            Propose this fix upstream to Click.
-        """
-        self.close()
-        super().exit(code)
-
 
 def default_extra_params():
     """Default additional options added to ``extra_command`` and ``extra_group``.

click_extra/config.py

@@ -429,12 +429,6 @@ def load_conf(self, ctx: Context, param: Parameter, path_pattern: str) -> None:
             message = "No configuration file found."
             if explicit_conf:
                 logger.critical(message)
-                # XXX We need to explicitly close the context before exiting, so all
-                # callbacks from options which cleans up CLI state are properly
-                # invoked. We need to perform this here so combinations of standalone
-                # options works well, and not only in our custom ExtraContext. This is
-                # a follow up on 936a8f5eff916bad5dfef1b4696c09b42a23ca61.
-                ctx.close()
                 ctx.exit(2)
             else:
                 logger.debug(message)

click_extra/parameters.py

@@ -730,10 +730,4 @@ def sort_by_depth(line):
         )
         echo(output, color=ctx.color)
 
-        # XXX We need to explicitly close the context before exiting, so all callbacks
-        # from options which cleans up CLI state are properly invoked.
-        # We need to perform this here so combinations of standalone options works
-        # well, and not only in our custom ExtraContext. This is a follow up on
-        # 936a8f5eff916bad5dfef1b4696c09b42a23ca61.
-        ctx.close()
         ctx.exit()

click_extra/version.py

@@ -36,6 +36,7 @@
 if TYPE_CHECKING:
     from collections.abc import Sequence
     from types import FrameType, ModuleType
+    from typing import NoReturn
 
     from cloup.styling import IStyle
 
@@ -448,7 +449,7 @@ def print_and_exit(
         ctx: Context,
         param: Parameter,
         value: bool,
-    ) -> None:
+    ) -> NoReturn:
         """Print the version string and exits.
 
         Also stores all version string elements in the Context's ``meta`` `dict`.
@@ -457,18 +458,17 @@ def print_and_exit(
         for var in self.template_fields:
             ctx.meta[f"click_extra.{var}"] = getattr(self, var)
 
+        # Always print debug messages, even if --version is not called.
         self.print_debug_message()
 
         if not value or ctx.resilient_parsing:
+            # Do not print the version and continue normal CLI execution.
             return
 
         echo(self.render_message(), color=ctx.color)
 
-        # XXX We need to explicitly close the context before exiting, so all callbacks
-        # from options which cleans up CLI state are properly invoked.
-        # We need to perform this here so combinations of standalone options works
-        # well, and not only in our custom ExtraContext. This is a follow up on
-        # 936a8f5eff916bad5dfef1b4696c09b42a23ca61.
+        # XXX Despite monkey-patching of click.Context.exit to force closing before
+        # exit, we still need to force it here for unknown reason. 🤷
+        # See: https://github.com/pallets/click/pull/2680
         ctx.close()
         ctx.exit()
-        return  # type: ignore[unreachable]

docs/issues.md

@@ -12,6 +12,7 @@ Here is the list of issues and bugs from other projects that `click-extra` has a
 
 ## [`click`](https://github.com/pallets/click)
 
+- [`#2680` - Fix closing of callbacks on CLI exit](https://github.com/pallets/click/pull/2680)
 - [`#2523` - Keep track of `<stderr>` and `<stdout>` mix in `CliRunner` results ](https://github.com/pallets/click/pull/2523)
 - [`#2522` - `CliRunner`: restrict `mix_stderr` influence to `<output>`; keep `<stderr>` and `<stdout>` stable](https://github.com/pallets/click/issues/2522)
 - [`#2483` - Missing auto-generated environment variables in help screen & case-sensitivity](https://github.com/pallets/click/issues/2483)