From b5081c7fe22cdc550b64b1fcf25a9c41b70c2e22 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 30 Jul 2026 16:38:49 +0200 Subject: [PATCH 1/3] fix(exceptions): restore exception hooks safely --- .sampo/changesets/resolute-seer-ukko.md | 5 ++ posthog/exception_capture.py | 55 ++++++++++-- posthog/test/test_exception_capture.py | 114 ++++++++++++++++++++++++ 3 files changed, 169 insertions(+), 5 deletions(-) create mode 100644 .sampo/changesets/resolute-seer-ukko.md diff --git a/.sampo/changesets/resolute-seer-ukko.md b/.sampo/changesets/resolute-seer-ukko.md new file mode 100644 index 000000000..11cf195f5 --- /dev/null +++ b/.sampo/changesets/resolute-seer-ukko.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: patch +--- + +Restore exception hooks safely diff --git a/posthog/exception_capture.py b/posthog/exception_capture.py index 627e8e47b..926ff242d 100644 --- a/posthog/exception_capture.py +++ b/posthog/exception_capture.py @@ -33,7 +33,9 @@ def __init__( refill_interval_seconds=DEFAULT_REFILL_INTERVAL_SECONDS, ): self.client = client + self._closed = False self.original_excepthook = sys.excepthook + self.original_threading_excepthook = threading.excepthook sys.excepthook = self.exception_handler threading.excepthook = self.thread_exception_handler # opt-in client-side rate limiting: per exception type, allow a burst @@ -47,17 +49,60 @@ def __init__( ) def close(self): - sys.excepthook = self.original_excepthook + if self._closed: + return + + self._closed = True + if self._is_own_hook(sys.excepthook, "exception_handler"): + sys.excepthook = self._resolve_hook( + self.original_excepthook, + "exception_handler", + "original_excepthook", + ) + if self._is_own_hook(threading.excepthook, "thread_exception_handler"): + threading.excepthook = self._resolve_hook( + self.original_threading_excepthook, + "thread_exception_handler", + "original_threading_excepthook", + ) if self._rate_limiter is not None: self._rate_limiter.stop() def exception_handler(self, exc_type, exc_value, exc_traceback): - # don't affect default behaviour. - self.capture_exception((exc_type, exc_value, exc_traceback)) - self.original_excepthook(exc_type, exc_value, exc_traceback) + if not self._closed: + self.capture_exception((exc_type, exc_value, exc_traceback)) + previous_hook = self._resolve_hook( + self.original_excepthook, + "exception_handler", + "original_excepthook", + ) + previous_hook(exc_type, exc_value, exc_traceback) def thread_exception_handler(self, args): - self.capture_exception((args.exc_type, args.exc_value, args.exc_traceback)) + if not self._closed: + self.capture_exception((args.exc_type, args.exc_value, args.exc_traceback)) + previous_hook = self._resolve_hook( + self.original_threading_excepthook, + "thread_exception_handler", + "original_threading_excepthook", + ) + previous_hook(args) + + def _is_own_hook(self, hook, handler_name): + return getattr(hook, "__self__", None) is self and hook == getattr( + self, handler_name + ) + + @staticmethod + def _resolve_hook(hook, handler_name, previous_hook_name): + """Skip closed ExceptionCapture hooks while preserving the hook chain.""" + while True: + owner = getattr(hook, "__self__", None) + if not isinstance(owner, ExceptionCapture) or not owner._closed: + return hook + if hook != getattr(owner, handler_name): + return hook + hook = getattr(owner, previous_hook_name) def exception_receiver(self, exc_info, extra_properties): if "distinct_id" in extra_properties: diff --git a/posthog/test/test_exception_capture.py b/posthog/test/test_exception_capture.py index 05e3d05ac..ef54a9f30 100644 --- a/posthog/test/test_exception_capture.py +++ b/posthog/test/test_exception_capture.py @@ -1,6 +1,8 @@ import subprocess import sys +import threading from textwrap import dedent +from types import SimpleNamespace from unittest.mock import MagicMock import pytest @@ -133,6 +135,118 @@ def test_rate_limit_keys_on_outermost_of_chained_exceptions(): capture.close() +def test_exception_hooks_delegate_and_restore_previous_hooks(monkeypatch): + from posthog.exception_capture import ExceptionCapture + + sys_hook = MagicMock() + thread_hook = MagicMock() + monkeypatch.setattr(sys, "excepthook", sys_hook) + monkeypatch.setattr(threading, "excepthook", thread_hook) + client = MagicMock() + capture = ExceptionCapture(client) + exc_info = _exc_info(ValueError("boom")) + thread_args = SimpleNamespace( + exc_type=exc_info[0], + exc_value=exc_info[1], + exc_traceback=exc_info[2], + thread=threading.current_thread(), + ) + + capture.exception_handler(*exc_info) + capture.thread_exception_handler(thread_args) + capture.close() + + assert client.capture_exception.call_count == 2 + sys_hook.assert_called_once_with(*exc_info) + thread_hook.assert_called_once_with(thread_args) + assert sys.excepthook is sys_hook + assert threading.excepthook is thread_hook + + +def test_close_does_not_overwrite_hooks_installed_later(monkeypatch): + from posthog.exception_capture import ExceptionCapture + + monkeypatch.setattr(sys, "excepthook", MagicMock()) + monkeypatch.setattr(threading, "excepthook", MagicMock()) + capture = ExceptionCapture(MagicMock()) + replacement_sys_hook = MagicMock() + replacement_thread_hook = MagicMock() + sys.excepthook = replacement_sys_hook + threading.excepthook = replacement_thread_hook + + capture.close() + + assert sys.excepthook is replacement_sys_hook + assert threading.excepthook is replacement_thread_hook + + +def test_multiple_captures_support_out_of_order_close(monkeypatch): + from posthog.exception_capture import ExceptionCapture + + sys_hook = MagicMock() + thread_hook = MagicMock() + monkeypatch.setattr(sys, "excepthook", sys_hook) + monkeypatch.setattr(threading, "excepthook", thread_hook) + first_client = MagicMock() + second_client = MagicMock() + first = ExceptionCapture(first_client) + second = ExceptionCapture(second_client) + + first.close() + assert sys.excepthook == second.exception_handler + assert threading.excepthook == second.thread_exception_handler + + exc_info = _exc_info(RuntimeError("boom")) + thread_args = SimpleNamespace( + exc_type=exc_info[0], + exc_value=exc_info[1], + exc_traceback=exc_info[2], + thread=threading.current_thread(), + ) + second.exception_handler(*exc_info) + second.thread_exception_handler(thread_args) + + assert first_client.capture_exception.call_count == 0 + assert second_client.capture_exception.call_count == 2 + sys_hook.assert_called_once_with(*exc_info) + thread_hook.assert_called_once_with(thread_args) + + second.close() + assert sys.excepthook is sys_hook + assert threading.excepthook is thread_hook + + +def test_uncaught_thread_exception_preserves_default_diagnostic(): + script = dedent( + """ + import threading + from posthog.exception_capture import ExceptionCapture + + class Client: + def capture_exception(self, exception, distinct_id=None): + print(f"captured:{exception[0].__name__}") + + capture = ExceptionCapture(Client()) + + def fail(): + raise RuntimeError("thread failure") + + thread = threading.Thread(target=fail, name="failing-thread") + thread.start() + thread.join() + capture.close() + """ + ) + + result = subprocess.run( + [sys.executable, "-c", script], capture_output=True, text=True, check=True + ) + + assert "captured:RuntimeError" in result.stdout + assert "Exception in thread failing-thread" in result.stderr + assert "RuntimeError: thread failure" in result.stderr + + def test_excepthook(tmpdir): app = tmpdir.join("app.py") app.write( From 3e414c98ffd7188f4cfcfcc3617cd34c67c2d3fd Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 30 Jul 2026 18:42:08 +0200 Subject: [PATCH 2/3] fix(exceptions): keep threading hook state private --- posthog/exception_capture.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/posthog/exception_capture.py b/posthog/exception_capture.py index 926ff242d..71fefbc87 100644 --- a/posthog/exception_capture.py +++ b/posthog/exception_capture.py @@ -35,7 +35,7 @@ def __init__( self.client = client self._closed = False self.original_excepthook = sys.excepthook - self.original_threading_excepthook = threading.excepthook + self._original_threading_excepthook = threading.excepthook sys.excepthook = self.exception_handler threading.excepthook = self.thread_exception_handler # opt-in client-side rate limiting: per exception type, allow a burst @@ -61,9 +61,9 @@ def close(self): ) if self._is_own_hook(threading.excepthook, "thread_exception_handler"): threading.excepthook = self._resolve_hook( - self.original_threading_excepthook, + self._original_threading_excepthook, "thread_exception_handler", - "original_threading_excepthook", + "_original_threading_excepthook", ) if self._rate_limiter is not None: self._rate_limiter.stop() @@ -82,9 +82,9 @@ def thread_exception_handler(self, args): if not self._closed: self.capture_exception((args.exc_type, args.exc_value, args.exc_traceback)) previous_hook = self._resolve_hook( - self.original_threading_excepthook, + self._original_threading_excepthook, "thread_exception_handler", - "original_threading_excepthook", + "_original_threading_excepthook", ) previous_hook(args) From 1697b188563a5c73da21f2a1d2838cad3e712603 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> Date: Fri, 31 Jul 2026 14:31:19 +0700 Subject: [PATCH 3/3] fix(exceptions): avoid hook restoration races --- posthog/exception_capture.py | 43 ++++++++++++++------------ posthog/test/test_exception_capture.py | 25 +++++++++++++++ 2 files changed, 49 insertions(+), 19 deletions(-) diff --git a/posthog/exception_capture.py b/posthog/exception_capture.py index 71fefbc87..9c4c723a9 100644 --- a/posthog/exception_capture.py +++ b/posthog/exception_capture.py @@ -36,8 +36,10 @@ def __init__( self._closed = False self.original_excepthook = sys.excepthook self._original_threading_excepthook = threading.excepthook - sys.excepthook = self.exception_handler - threading.excepthook = self.thread_exception_handler + self._sys_excepthook = self.exception_handler + self._threading_excepthook = self.thread_exception_handler + sys.excepthook = self._sys_excepthook + threading.excepthook = self._threading_excepthook # opt-in client-side rate limiting: per exception type, allow a burst # of captures, then refill over time self._rate_limiter = None @@ -53,18 +55,26 @@ def close(self): return self._closed = True - if self._is_own_hook(sys.excepthook, "exception_handler"): - sys.excepthook = self._resolve_hook( - self.original_excepthook, - "exception_handler", - "original_excepthook", - ) - if self._is_own_hook(threading.excepthook, "thread_exception_handler"): - threading.excepthook = self._resolve_hook( - self._original_threading_excepthook, - "thread_exception_handler", - "_original_threading_excepthook", - ) + original_excepthook = self._resolve_hook( + self.original_excepthook, + "exception_handler", + "original_excepthook", + ) + original_threading_excepthook = self._resolve_hook( + self._original_threading_excepthook, + "thread_exception_handler", + "_original_threading_excepthook", + ) + + # Keep each final ownership check and assignment together without a + # Python call between them. On supported GIL-enabled CPython builds, + # ordinary Python thread scheduling has no switch point inside either + # straight-line pair, minimizing the window for external hook writers. + if sys.excepthook is self._sys_excepthook: + sys.excepthook = original_excepthook + if threading.excepthook is self._threading_excepthook: + threading.excepthook = original_threading_excepthook + if self._rate_limiter is not None: self._rate_limiter.stop() @@ -88,11 +98,6 @@ def thread_exception_handler(self, args): ) previous_hook(args) - def _is_own_hook(self, hook, handler_name): - return getattr(hook, "__self__", None) is self and hook == getattr( - self, handler_name - ) - @staticmethod def _resolve_hook(hook, handler_name, previous_hook_name): """Skip closed ExceptionCapture hooks while preserving the hook chain.""" diff --git a/posthog/test/test_exception_capture.py b/posthog/test/test_exception_capture.py index ef54a9f30..85b50ac16 100644 --- a/posthog/test/test_exception_capture.py +++ b/posthog/test/test_exception_capture.py @@ -180,6 +180,31 @@ def test_close_does_not_overwrite_hooks_installed_later(monkeypatch): assert threading.excepthook is replacement_thread_hook +def test_close_does_not_overwrite_hooks_replaced_while_resolving(monkeypatch): + from posthog.exception_capture import ExceptionCapture + + monkeypatch.setattr(sys, "excepthook", MagicMock()) + monkeypatch.setattr(threading, "excepthook", MagicMock()) + capture = ExceptionCapture(MagicMock()) + replacement_sys_hook = MagicMock() + replacement_thread_hook = MagicMock() + original_resolve_hook = capture._resolve_hook + + def replace_hook_while_resolving(hook, handler_name, previous_hook_name): + if handler_name == "exception_handler": + sys.excepthook = replacement_sys_hook + else: + threading.excepthook = replacement_thread_hook + return original_resolve_hook(hook, handler_name, previous_hook_name) + + monkeypatch.setattr(capture, "_resolve_hook", replace_hook_while_resolving) + + capture.close() + + assert sys.excepthook is replacement_sys_hook + assert threading.excepthook is replacement_thread_hook + + def test_multiple_captures_support_out_of_order_close(monkeypatch): from posthog.exception_capture import ExceptionCapture