Skip to content

Do not raise an Exception when exiting pdb #124703

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gaogaotiantian opened this issue Sep 27, 2024 · 11 comments
Closed

Do not raise an Exception when exiting pdb #124703

gaogaotiantian opened this issue Sep 27, 2024 · 11 comments
Labels
type-feature A feature request or enhancement

Comments

@gaogaotiantian
Copy link
Member

gaogaotiantian commented Sep 27, 2024

Feature or enhancement

Proposal:

Currently, if you set a breakpoint() in your code, and exit with q or ctrl+D, the output is just ugly:

Traceback (most recent call last):
  File "/Users/gaotian/programs/cpython/example.py", line 11, in <module>
    while True:
          ^^^^
  File "/opt/homebrew/Cellar/[email protected]/3.12.6/Frameworks/Python.framework/Versions/3.12/lib/python3.12/bdb.py", line 90, in trace_dispatch
    return self.dispatch_line(frame)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/[email protected]/3.12.6/Frameworks/Python.framework/Versions/3.12/lib/python3.12/bdb.py", line 115, in dispatch_line
    if self.quitting: raise BdbQuit
                      ^^^^^^^^^^^^^
bdb.BdbQuit

It makes no sense to print a trackback of an exception just because I want to quit pdb and stop debugging. Now that we have an indicator for inline mode, we can deal with this.

What I propose is - for inline mode debugging, any quit attempt(q, ctrl+D, etc.) will trigger a confirm prompt (like gdb or lldb), the user can say no and go back to debugging, or they can confirm and quit the program.

There are a few design details which I will list in the following PR.

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

No response

Linked PRs

@ADThomas-astro
Copy link
Contributor

ADThomas-astro commented Feb 7, 2025

I'd like to reopen this issue.

The committed changes mean that quitting pdb also quits the REPL. This was mentioned on PR 124704 but I don't think there was enough discussion. It's been longstanding in Python that you can call breakpoint() in a REPL session and then return to the REPL prompt after debugging. I agree that the traceback was ugly but I think not being able to return to the REPL from pdb is a regression and a worse user experience problem than a short traceback being printed. There could be a lot of state in a REPL session that a user of older versions of Python would not expect to lose. And I at least like to jump in and out of the debugger in a REPL session a lot (admittedly in ipython, but with the new PyREPL I'd like to do it in PyREPL too!)

I propose a compromise (PR 129768). The recently merged PR 124704 adds a prompt "Quit anyway? [y/n]". Since we're prompting the user now, why not let them choose how to quit? If they're stuck in an infinite loop or finished with interactive work they can choose to kill the process immediately and cleanly, but they can also exit normally (raise BdbQuit) and return to the REPL.

In my proposed changes I also added some code to shorten the BdbQuit traceback. It's still ugly though, and I don't know how to remove or further improve it.

@gaogaotiantian
Copy link
Member Author

I think the correct way to "return from breakpoint to repl" should be continue, not quit - you can do that now.

We can see how many people are affected by this change, because personally I never use breakpoint() in REPL, debugging some interactive code seems a bit weird to me, but other people might use it.

I don't believe the PR works though, we should keep the normal path clean (y/n is much more common than some random options). If we had to deal with this, we should make pdb or PyREPL consider this a special case and deal with it. The last spot is to revert the change, shortening the traceback does not help that much.

@ADThomas-astro
Copy link
Contributor

Thanks for the reply!

I don't think I did a good job of explaining my perspective. Here's a common workflow that I use for data analysis exploration/scripting:

  • Open an ipython terminal and text editor
  • Load in the data. This is often slow, depending on the data volume. Keeping the data loaded in the REPL session/process is convenient to not need to reload the data.
  • Write functions/snippets to explore and process the data. I copy-paste from the script to the ipython terminal, perhaps using the %paste magic. I might make interactive matplotlib plots, or use pd.DataFrame.to_clipboard() to quickly copy tables to Excel to explore, etc.
  • If an exception occurs, I can use the fantastic "%debug" magic in ipython to immediately enter post-mortem debugging
  • If I want to debug the state before an error occurs, or just explore the state at a certain point in the program, then I add... the built-in "breakpoint" function before copying the code to the REPL. It's blessed as a built-in, and for years it's worked well in this workflow.
  • After this change, leaving the debugger will exit the process, killing the ipython session and the accumulated state, and requiring slow re-loading of data and potentially slow re-processing of data.

I prefer this workflow to a Jupyter notebook (which may also be affected), and I think I'm using iPython as designed.

I think the correct way to "return from breakpoint to repl" should be continue, not quit - you can do that now.

That would often work, but not always. E.g. In a slow data-processing loop, I wouldn't want to continue executing the code from the breakpoint. Also, I would argue that q/quit/exit is the obvious way to leave the debugger, and it's always worked up to this point.

Hopefully the user experience problem is more clear now.

We can see how many people are affected by this change, because personally I never use breakpoint() in REPL, debugging some interactive code seems a bit weird to me, but other people might use it.

I think it will affect plenty of folks with similar workflows. And I'm sceptical that removing the feature is consistent with Python's backward compatibility policy (tbh I haven't read the PEP to check).

I don't believe the PR works though, we should keep the normal path clean (y/n is much more common than some random options).
If we had to deal with this, we should make pdb or PyREPL consider this a special case and deal with it.

A special case in PyREPL/REPLs could work, but I think a special case in pdb would make the most sense (or reverting, but I still think a compromise makes sense).

Reading PR123757, how about adding another pdb mode, say "inline-interactive"? We can check for interactive/REPL mode in set_trace, and then when exiting in "inline-interactive" mode raise BdbQuit. When in non-interactive "inline" mode we would keep the code to kill the process and avoid the traceback.

Would you be interested in a PR along those lines?

@gaogaotiantian
Copy link
Member Author

Quick question - did you try this on ipython and confirm it will exit the shell? I don't think it will exit ipython as ipython catches SystemExit.

@ADThomas-astro
Copy link
Contributor

Quick question - did you try this on ipython and confirm it will exit the shell?

Nope 🤦 I intended to but forgot. Indeed ipython does catch SystemExit. This is what it looks like:

Image

Thankfully it doesn't exit the process but I still think this is a worse experience than the BdbQuit traceback printed previously.

Summary

When using breakpoint()/set_trace() in an interactive REPL session then entering q/quit/exit in pdb, previously in all cases a BdbQuit traceback was printed and the user returned to the REPL. But now:

  • iPython - There is a prompt that warns that the process will be terminated; if the user proceeds then the process isn't terminated; SystemError is caught and iPython issues a confusing UserWarning when returning to the REPL
  • PyREPL - There is a prompt that warns the process will be terminated; if the user proceeds the process is terminated, with no reliable way to exit back to the REPL.
  • Vanilla Python REPL - Same as PyREPL.

I think the change makes the experience of doing serious work in REPLs worse.

I think we could return to the previous behaviour with the following code in set_trace:

        mode = 'inline-repl' if hasattr(sys, 'ps1') else 'inline'
        pdb = Pdb(mode=mode)

This would keep much of the benefit of the existing code while preventing the experience in REPLs from becoming worse.

@ADThomas-astro
Copy link
Contributor

Actually on further investigation, iPython has been catching bdb.BdbQuit for a while now, so users don't even see a traceback. (I've mostly been using older versions).

@gaogaotiantian
Copy link
Member Author

We don't need another mode, that's just too complicated. We can add the hasattr(sys, "ps1") check in do_quit to keep the previous behavior for interactive consoles.

@ADThomas-astro
Copy link
Contributor

That also works; want me to send a PR?

@gaogaotiantian
Copy link
Member Author

Sure, it's your idea, feel free to send a PR. I don't know how easy it is to create a regression test with repl. If it's possible, that would also be appreciated.

@asottile
Copy link
Contributor

hi -- just ran into this for the first time -- I feel like exiting 0 is problematic because previously it would exit nonzero and cancel the process

consider something like:

python some_program_with_debugger.py && do-dangerous-thing

previously when debugging if you quit in the middle of the program that would result in an unsuccessful exit code -- but after this patch it now exits 0 (despite the program not being successful -- in fact it was explicitly cancelled)

@gaogaotiantian
Copy link
Member Author

I think you are right. The example might be a bit artificial but I agree a return code of something other than 0 makes more sense. #133013 is made to change it - it's lucky that you caught this before beta freeze, everything is simpler :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

3 participants