Skip to content

Add trigger_exception_handler() for post-mortem debugging of caught exceptions#1996

Open
nshepperd wants to merge 10 commits into
microsoft:mainfrom
nshepperd:easy-postmortem
Open

Add trigger_exception_handler() for post-mortem debugging of caught exceptions#1996
nshepperd wants to merge 10 commits into
microsoft:mainfrom
nshepperd:easy-postmortem

Conversation

@nshepperd

@nshepperd nshepperd commented Feb 1, 2026

Copy link
Copy Markdown

Adds a convenient trigger_exception_handler() function (previously named post_mortem(); renamed per review discussion) that can be called to immediately trigger the graphical postmortem debugger given any exc_info structure. We need to suspend sys.monitoring callbacks while doing this, because the built in reentrancy protection doesn't apply when pydevd suspends directly from normal code instead of a monitoring callback. Functions were added in _pydevd_sys_monitoring to do that.

@nshepperd

Copy link
Copy Markdown
Author

See #1530.

@rchiodo

rchiodo commented Feb 2, 2026

Copy link
Copy Markdown
Contributor

Thanks for the submission but I think this would need some sort of automated test before we'd accept it. And maybe a section in the readme.md explaining how to use it.

@nshepperd

Copy link
Copy Markdown
Author

Actually I was wondering how to setup tests for this. I figured it out today. Hooray!

@rchiodo

rchiodo commented Feb 3, 2026

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@rchiodo

rchiodo commented Feb 3, 2026

Copy link
Copy Markdown
Contributor

I'm confused about something. pdb.post_mortem attaches the debugger to the process and starts debugging it. This won't. You have to be already attached, right?

What would be the point of this then? Is it just so you don't have to break on exceptions in the debugger, but instead control when the break happens?

@rchiodo

rchiodo commented Feb 3, 2026

Copy link
Copy Markdown
Contributor

I think ideally this would actually start VS code and open the debugger. Which might be doable, but would probably have to be configurable somehow.

@nshepperd

Copy link
Copy Markdown
Author

Yes, the point is so that you can eg. make a context manager or function decorator to debug exceptions that bubble past it instead of setting bespoke breakpoint, which isn't really directly possible for this. I find something like this indispensable for debugging things like api servers or unit tests (pytest) that have exception handlers that eat exceptions. With this you can just wrap your route handler or set an exception handler with conftest.py and launch the program under the debugger and everything just works.

I think previously debugpy had the 'user uncaught exceptions' breakpoint added for similar use cases, but that's much less specific and in some cases has too many false positives to be useful.

You're right this is a bit different from pdb.post_mortem() in that that actually starts the debugger. I'm not sure about starting vscode though, heh. Maybe it's possible via some sort of vscode:// url? What we could do is at least run debugpy.listen and wait for the debugger to attach? I'm not sure i would actually use that though, usually if i'd want to debug something with vscode I'm already debugging it (eg. I press the 'debug unit test' button) and the issue is just getting the breakpoint.

@nshepperd

Copy link
Copy Markdown
Author

Either way each use cases should work, i think. If you wanted to launch vscode and attach the debugger you can use existing debugpy functions for that and then call post_mortem. Or if we include that in the function you can check if debugpy.is_client_connected() before calling it if you don't want that.

@rchiodo

rchiodo commented Feb 4, 2026

Copy link
Copy Markdown
Contributor

With this you can just wrap your route handler or set an exception handler with conftest.py and launch the program under the debugger and everything just works.

If this is the intended use case, maybe the name is off then. It's not post_mortem, it's more like 'force_exception_handler' or something like that. I feel like 'post_mortem' is intended to be like pdb means it. It starts a debugger even when it's not already attached.

@rchiodo

rchiodo commented Feb 4, 2026

Copy link
Copy Markdown
Contributor

Your unit tests also seem to be failing. Not sure if you can see the errors or not

@nshepperd

Copy link
Copy Markdown
Author

trigger_exception_handler(), maybe?

I see the test failures. Cython issue. I forgot to regenerate the cython files. Confusing because it worked locally.

@rchiodo

rchiodo commented Feb 4, 2026

Copy link
Copy Markdown
Contributor

trigger_exception_handler sounds good to me :)

@nshepperd nshepperd changed the title Easy post_mortem Add trigger_exception_handler() for post-mortem debugging of caught exceptions Jul 14, 2026
@rchiodo

rchiodo commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@rchiodo
rchiodo marked this pull request as ready for review July 14, 2026 20:19
@rchiodo
rchiodo requested a review from a team as a code owner July 14, 2026 20:19
@rchiodo

rchiodo commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

LGTM. I'll get somebody else to take a look.

@rchiodo

rchiodo commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

🔒 Automated review in progress — @rchiodo is auto-reviewing this PR.

@heejaechang

Copy link
Copy Markdown
Contributor

🔒 Automated review in progress — @heejaechang is auto-reviewing this PR.

@heejaechang heejaechang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved via Review Center.

Comment thread src/debugpy/_vendored/pydevd/pydevd.py Outdated
# explicitly disabled here, the first monitoring event on this thread
# would create it with tracing enabled.
thread_info = _get_thread_info(True, 1)
if thread_info is None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 src/debugpy/_vendored/pydevd/_pydevd_sys_monitoring/_pydevd_sys_monitoring.py:1875
When the except branch freshly creates a ThreadInfo via _get_thread_info(True, 1) (tracing enabled), it still reports previous_state = True. The caller's finally then sees saved == True and re-enables tracing on a thread that was not previously traced, contradicting "restore previous state" and potentially causing unexpected breakpoint stops on background threads. When the ThreadInfo is created here rather than found, return False so the caller skips the resume. Mirror the same fix in the .pyx (and regenerate the .c).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think how it is currently is actually correct. True is the default state (for everything except pydevd-daemon threads), in the sense that every other monitoring event sets thread_info like that if it doesn't exist. So that's what we should restore.

Comment thread tests/debugpy/test_trigger_exception_handler.py
Comment thread src/debugpy/public_api.py

@rchiodo rchiodo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved via Review Center.

@heejaechang heejaechang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved via Review Center.

"description": "something went wrong",
"breakMode": "unhandled",
}
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 tests/debugpy/test_trigger_exception_handler.py:40

The Rules reviewer flagged that four tests (_basic, _basic_with_exception, _excinfo, _not_as_uncaught) assert with some.dict.containing({...}) and some.str.matching(r"(.+\.)?ValueError") without justification, violating docs/pylancewiki/review/tests-no-partial-asserts.md (partial-dict/substring matches require a comment explaining why the result cannot be made deterministic). This is a confirmed rule violation and is non-downgradeable. These some.* matchers are debugpy's idiomatic harness, and exceptionId legitimately varies across Python versions (ValueError vs builtins.ValueError) — so the minimal compliant fix is to add a short comment stating why exact equality isn't possible (and tighten to == on the parts of the exceptionInfo response that are deterministic).

[verified]

Comment thread src/debugpy/server/api.py

if not (
isinstance(exctype, type)
and isinstance(value, BaseException)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 src/debugpy/server/api.py:380

Unresolved from the prior round. The identical multi-line "excinfo must be an exception instance or a (type, value, traceback) tuple ..." message is raised by both the tuple-shape guard and the element-type guard. When the second guard fires the tuple shape is already correct, so the message is misleading. Hoist it into a single constant, or give the element-type guard a distinct message (e.g. reporting the actual element types).

[verified]

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems to be a pylance thing

Comment thread tests/debugpy/test_trigger_exception_handler.py
return previous_state


# fmt: off

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📍 src/debugpy/_vendored/pydevd/_pydevd_sys_monitoring/_pydevd_sys_monitoring.py:1887
The except branch still creates thread_info via _get_thread_info(True, 1) (default trace=True) and returns previous_state = True, contradicting the docstring's "returns the previous state" for a thread that was never traced (Skeptic, Architect, and Rules all re-confirm this). However, the Advocate's caller-trace shows the literal return False requested last round is not a clean fix: returning False before setting thread_info.trace = False skips suppression during the stop (defeating the PR's PEP-669 purpose on this path), while returning False after suspending would leave the fresh thread permanently at trace=False (a suppression leak). The current behavior — suppress during the stop, then resume to the thread's True default — is actually functionally correct. Resolve the contract honestly: either clarify the docstring that for a freshly-created ThreadInfo "previous state" is the thread's default (enabled), or, if a fresh thread should end untraced, suspend during the stop AND return False so resume is skipped. Mirror whatever you choose in the .pyx and regenerate the .c.

[verified]

@nshepperd

Copy link
Copy Markdown
Author

I think this is good now. Thank Claude for letting me stop procrastinating on this. I'll get CLA permission from my employer.

@rchiodo

rchiodo commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Looks good overall. A couple of low-priority, non-blocking notes on a docstring/contract mismatch and a duplicated error message; safe to merge.

@rchiodo rchiodo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved via Review Center.

@heejaechang heejaechang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved via Review Center.

if exception_breakpoint.expression is not None:
py_db.handle_breakpoint_expression(exception_breakpoint, additional_info, user_frame)
finally:
remove_exception_from_frame(user_frame)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Python 3.14, an existing __exception__ local makes remove_exception_from_frame() raise ValueError, aborting the default stop and potentially masking the caught exception after an unconditional stop. Preserve and restore any previous binding rather than unconditionally removing it, with regression tests for both as_uncaught modes.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you're right. It does fail on 3.14. I've fixed this with a context manager that restores the previous state, using it in both places.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly, Claude says this is also a problem on HEAD. It shows me this repro testcase for the userUnhandled exception handler, which seemingly hangs the unit test unless the context manager/restore method is used there:

import pytest

from tests import debug
from tests.patterns import some
from tests.timeline import Event


def test_uu_repro(pyfile, target, run):
    @pyfile
    def code_to_debug():
        import threading

        import debuggee

        debuggee.setup()

        def thread_target():
            __exception__ = "sentinel"  # real local in the user boundary frame
            raise ValueError("boom")  # @raise

        t = threading.Thread(target=thread_target)
        t.start()
        t.join()
        print("main continued after thread")  # @done

    with debug.Session() as session:
        session.config["justMyCode"] = True
        session.expected_exit_code = some.int
        with run(session, target(code_to_debug)):
            session.request(
                "setExceptionBreakpoints", {"filters": ["userUnhandled"]}
            )
            session.set_breakpoints(code_to_debug, ["done"])

        session.wait_for_stop("exception")
        session.request_continue()
        session.wait_for_next(Event("exited") | Event("terminated"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests do run in 3.14 as far as I know, so it should be working in main

@nshepperd nshepperd Jul 17, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but you didn't have a unit test that puts an existing local named __exception__, as far as i can see.


if user_frame is None:
pydev_log.warn("trigger_exception_handler: no user frame found in traceback")
return

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using get_file_type(...) is None excludes LIB_FILE frames as well as debugger-internal frames; a library-only traceback therefore never stops despite as_uncaught=False. Exclude only PYDEV_FILE frames so unconditional mode can select ordinary library frames.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

nshepperd and others added 2 commits July 18, 2026 06:18
pydevd injects __exception__ into the stopped frame's locals for the
Locals view, then removed it unconditionally. On Python 3.14 deleting a
real frame local raises ValueError (FrameLocalsProxy), so if the user's
own code has a variable named __exception__, the stop crashes; on any
version the user's value was silently clobbered.

Add an exception_on_frame context manager that saves the prior binding
and restores it by assignment (never deleting), and use it in the two
trigger_exception_handler stop paths. Regression test covers both
as_uncaught modes with an __exception__ local in the stopped frame.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@rchiodo

rchiodo commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@nshepperd

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree company="Anlatan"

@rchiodo

rchiodo commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

happens when the user's own code has a variable named '__exception__'."""

def __init__(self, frame, exception_info):
self.frame = frame

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm wondering if we can just stick the exception on the frame as an attribute. Modifying the locals is usually not so great because then the user sees it in the locals list (and the potential conflict). Although it did that before. But that might explain the error you were talking about with 3.14.

Something like this instead:

self.frame.__stored_exception = exception_info

Then removing it would just be deleting that attribute.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH I'm not sure what __exception__ is used for. I assumed the point was for users to see it in the locals list (eg. vscode debugging terminal). Or maybe it's a legacy thing used by Eclipse?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at it, it seems it takes special precautions to remove it from the locals just for that reason. So that users don't see it. I think we can probably just change addExceptionToFrame and removeExceptionFromFrame by just adding an attribute on it. Then we wouldn't have to special case locals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants