Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add easy "debug-on-exception" debugger middleware functionality (and remote debugging) #11493

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add easy "debug-on-exception" debugger middleware functionality (and remote debugging) #11493

wants to merge 6 commits into from

Conversation

izo0x90
Copy link

@izo0x90 izo0x90 commented Apr 27, 2024 •
edited

Intent:

  • Quicker to replicate issues, less cycles to fix

    • Often webdev workflow involve dealing with mistakes that require some set of step to replicate, having the a debugger start up when the issue occurs initially can really mitigate the number of iterations on figuring out and fixing issues.

  • Easy remote/ docker-ized debugging

    • Having easy access to a remote debugger such as webdev also makes debugging docker-ized (and/ or dev. envs. running on a remote machine) straightforward.

  • Additionally having a debugger right in the browser can make the "ergonomics" of this cycle less cumbersome.

This PR attempts to achieve the above by providing an easy way to add debug-on-exception feature to Fastapi along side with some sensible default debugger options.

Overview:

  • Adds middleware class to allow intercepting exceptions in endpoints and starting a debugger in those stack frames

  • Debugger is customizable by passing in a custom callable that can run debugger of choice

  • Default debugger if not customized is PDB

  • Adds support to use web-PDB by including pre made callable that can be passed in as a middleware arg

Hristo Gueorguiev and others added 6 commits April 27, 2024 11:17
Add debugger middleware functionality
b95824c 
- Adds middleware class to allow intercepting exceptions in endpoints
  and starting a debugger in those stack frames

- Debugger is customizable by passing in a custom callable that can run
  debugger of choice

- Default debugger if not customized is PDB

- Adds support to use web-PDB by including pre made callable that can be
  passed in as a middleware arg
@izo0x90
Merge branch 'master' into master
ea0e428
Add tests for debugger middleware
431d576
Merge branch 'master' of github.com:izo0x90/fastapi
767f095
Update debugger middleware test, value will not be returned
894f7c3
Debugger middleware tests make pytest happy about unreachable code
79a090f
@izo0x90 izo0x90 changed the title Add easy "debug-on-exception" debugger middleware functionality Add easy "debug-on-exception" (and remote debugging) debugger middleware functionality Apr 27, 2024
@izo0x90 izo0x90 changed the title Add easy "debug-on-exception" (and remote debugging) debugger middleware functionality Add easy "debug-on-exception" debugger middleware functionality (and remote debugging) Apr 27, 2024
@Jaza
Copy link

Jaza commented May 1, 2024

I guess this is nice-to-have. But:

  • I don't really see why this needs to live directly in FastAPI or any of its dependencies
  • Even if you ignore my first point, this could live in Starlette rather than in FastAPI, Starlette already includes a number of middlewares, and AFAICT this doesn't depend on anything in FastAPI, it only depends on Starlette
  • Personally I'm happy enough just with stack traces showing in the browser / console in my local environment, and when I want to step it up a notch, temporarily chucking a pdb.set_trace() manually at one spot in my code is enough for me (but maybe other folks feel the need for a richer debugging experience to be included out-of-the-box?)
  • My preference would be for this to live in in its own little library / repo (or even just as a gist or similar)

Do other people have thoughts on the matter?

@izo0x90
Copy link
Author

izo0x90 commented May 2, 2024 •
edited

I guess this is nice-to-have. But:

  • I don't really see why this needs to live directly in FastAPI or any of its dependencies
  • Even if you ignore my first point, this could live in Starlette rather than in FastAPI, Starlette already includes a number of middlewares, and AFAICT this doesn't depend on anything in FastAPI, it only depends on Starlette
  • Personally I'm happy enough just with stack traces showing in the browser / console in my local environment, and when I want to step it up a notch, temporarily chucking a pdb.set_trace() manually at one spot in my code is enough for me (but maybe other folks feel the need for a richer debugging experience to be included out-of-the-box?)
  • My preference would be for this to live in in its own little library / repo (or even just as a gist or similar)

Do other people have thoughts on the matter?

Thank @Jaza appreciate the feedback.
Very fair points. I do still think having something like this out of the box can be very convenient, esp. as ppl spin up new projects. If the consensus ends up being that it doesn't make sense to house in Fastapi I will move it over to Starlette and see how that goes.

But curious to see if there is any other input/ opinions here?

@alejsdev alejsdev added feature New feature or request p4 labels May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
feature New feature or request p4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@izo0x90 @Jaza @alejsdev