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.

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

Nicer traceback formatting #2667

Merged
merged 68 commits into from Mar 6, 2023
Merged

Nicer traceback formatting #2667

merged 68 commits into from Mar 6, 2023

Conversation

Tronic
Copy link
Member

@Tronic Tronic commented Jan 27, 2023

Shinier error pages for Sanic, using the sanic-org/tracerite module for traceback formatting and the new BasePage layout merged in #2662.

Any exception in an app, while running Sanic in debug mode gives a formatted traceback:
Screenshot

An attempt is made to show only the relevant parts of call chains and exception chains, pointing to where the error is in user code instead, but other details are available under tab headers (e.g. Sanic.handle_request in the above screenshot - but the current version would hide that entirely and the style has also changed since).

@Tronic Tronic requested a review from a team as a code owner January 27, 2023 06:53
@Tronic
Copy link
Member Author

Tronic commented Jan 27, 2023

Hmm, not possible to make draft PR to another PR? Anyway, all be reminded that this is very explosive draft code build on top of another draft PR.

@ahopkins
Copy link
Member

Not sure about adding two deps for this right now. Also, changing travebacks is a fairly opinionated item. That to me feels more like something that should be in sanic-ext, with this focusing more on adding the styling changes.

Tracyca209
Tracyca209 previously approved these changes Jan 27, 2023
…rint also headers, and for other than 500 errors as well. 500 error message text UX workaround.
@Tronic
Copy link
Member Author

Tronic commented Jan 27, 2023

Yes, definitely not going to push it, but I might leave it floating around for a bit later should there be interest. I don't see it living in sanic-ext because either you get better exception handling in default install or you never get one (I have many years of prior experience with niceback, trying to get Jupyter Notebook, Google Colab and others to use it but not even regular data scientists do).

I made some cleanup today. The last night thing was about 15 minutes quick adoption (after some hours porting other parts of the old error page). Instead of spending more time porting the TB formatter, using niceback was essentially just one function call.

Long term plan: few minor improvements on niceback (that I've done today, maybe a bit more needed), get that moved to sanic-org as well, and change the name. Turns out that all Internet search engines search for Nickelback when you want Niceback, and even when forced give you physiotherapy instead, so the name clearly needs to be changed.

@Tronic
Copy link
Member Author

Tronic commented Jan 27, 2023

With the cleanup done today, you don't get the two white tabs as shown in the screenshot above, in that situation. Also the Request object is hidden in variable inspector because it provided no information not already known. Thus, for a simple bug you get very simplified output that directly points out what happened and where.

Screenshot

For more complicated cases it still prints tabs for the full call chain (with variable inspector on each tab), and "the above exception occurred after catching ..." kind of thing as well (try causing a 404 error with sanic --simple --dev and you get some aiofiles/thread internals displayed that way).

@ahopkins
Copy link
Member

Real quick thought I've had, then I'll respond in more detail when I'm at my computer.

Let's keep the formatting in, but only conditionally. We have a note that says you can have nicer travebacks if you install this. Like how we deal with Jinja. You are not required to download it, but we support it if you do.

Tronic and others added 8 commits February 8, 2023 01:26
* Add color sections for dark mode

* Tweak colors for light mode

* Tweak colors for dark mode

* Remove tab styling

* Remove forced hanging emoji from error header

* Add padding for main container so text doesn’t touch screen boundary

* invalid text justify css

* Tweak color scheme for dark themes

* Add margins for h3. Probably should be done in the tracerite package.

* Add tracerite tab color with new sanic-tab variable.

* Soften lightmode text color

* Define sanic-background, sanic-text as top-level colors. Restructure color schemes to top of definition.

* Define highlight and tab colors as variables

* Add tab hover shadow definition. Intended to have better hover accents for light theme.

* Ensure sanic brand color be the same in dark mode.

* Add header background, change all dark theme grays to neutral grays.

* Switch blue to code block orange in sanic docs. Add sanic highlight override. Add darker sanic background (111) as option but in the comments so we can switch back if need be.

* Add sanic id to top level document.

* Use hsl and rgba for dark themed background so that they can be easily changed and nudged.

* Change colors to Adam’s preferences.

* Remove important from css after adding sanic as id,

* Remove remnants of “important” for pre and code tags. Add Meslo in font.
@Tronic Tronic removed the on hold label Mar 6, 2023
@codecov
Copy link

codecov bot commented Mar 6, 2023

Codecov Report

Patch coverage: 100.000% and project coverage change: -0.075 ⚠️

Comparison is base (259e458) 88.716% compared to head (ff0f3e6) 88.642%.

Additional details and impacted files
@@              Coverage Diff              @@
##              main     #2667       +/-   ##
=============================================
- Coverage   88.716%   88.642%   -0.075%     
=============================================
  Files           87        87               
  Lines         6851      6815       -36     
  Branches      1177      1171        -6     
=============================================
- Hits          6078      6041       -37     
- Misses         532       533        +1     
  Partials       241       241               
Impacted Files Coverage Δ
sanic/app.py 90.053% <ø> (ø)
sanic/application/logo.py 100.000% <ø> (ø)
sanic/errorpages.py 98.765% <100.000%> (-0.733%) ⬇️
sanic/router.py 95.833% <100.000%> (+0.043%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ahopkins
Copy link
Member

ahopkins commented Mar 6, 2023

image
We should either figure out how to strip the tags going into <title> or just skip using <strong> altogether.

sanic/pages/error.py Outdated Show resolved Hide resolved
sanic/pages/error.py Outdated Show resolved Hide resolved
sanic/pages/error.py Outdated Show resolved Hide resolved
sanic/pages/error.py Outdated Show resolved Hide resolved
sanic/app.py Show resolved Hide resolved
sanic/pages/error.py Outdated Show resolved Hide resolved
Copy link
Member

@ahopkins ahopkins left a comment

Choose a reason for hiding this comment

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

Looks good, I am excited to get this in. Just a few questions. Also, I am proposing a simpler color scheme here: #2703. Also it corrects a couple items around debug mode, and adds a few helpful links when debug=True.

Tronic and others added 3 commits March 6, 2023 11:01
* tracerite var colors

* Simplified parse_content_header escaping (#2707)

* Move comments, darken bg color, move debug links to contextmanager

* Lineup H2s

* Make pretty

---------

Co-authored-by: L. Kärkkäinen <98187+Tronic@users.noreply.github.com>
@ahopkins ahopkins merged commit a5d7d03 into main Mar 6, 2023
32 checks passed
@ahopkins ahopkins deleted the niceback-error-handling branch March 6, 2023 19:24
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.

None yet

4 participants