some exceptions in Flask aren't colorized by our monkey-patching
dmalan opened this issue · 4 comments
- Copy https://gist.github.com/dmalan/14ed9ec6548df079ea4468f435c7dc6b into a directory.
- Run
pip3 install -r requirements.txt. - Run
flask run. - Visit
/foo, and you should see that traceback is highlighted by CS50 library: lines that aren't in site-packages are highlighted in yellow. - Visit
/bar, and you should see that traceback isn't highlighted.
The lack of highlighting in the last step appears to be the result of https://github.com/pallets/werkzeug/blob/master/werkzeug/serving.py#L284-L295, whereby the traceback being printed at that point in the app's lifecycle is coming from werkzeug.debug.tbtools.get_current_traceback, specifically traceback.plaintext. Would be ideal to format traceback.plaintext using https://github.com/cs50/python-cs50/blob/develop/src/cs50/cs50.py#L68 as well, though currently our function's signature expects (type, value, tb).
Perhaps simplest to monkey-patch werkzeug.debug.tbtools.get_current_traceback in https://github.com/cs50/python-cs50/blob/develop/src/cs50/flask.py, just as we do flask.logging.default_handler.formatter.formatException? Or monkey-patch the Traceback.plaintext property instead, https://github.com/pallets/werkzeug/blob/master/werkzeug/debug/tbtools.py#L379?
@dmalan I mentioned this in our Slack discussion but I'll add it here too. SyntaxErrors occur before any code is executed, so I don't believe that anything we put in the library can fix this (since the error is thrown before any of our library code is executed). I think the only solution to this would be to write a wrapper around flask run I know we currently have a shell wrapper, but I mean a Python one that can do this patching.
Those SyntaxErrors are being caught and printed by https://github.com/pallets/werkzeug/blob/master/werkzeug/serving.py#L284-L295, though? So we should be able to hook into werkzeug's code, no?
It is being caught by werkzeug, yes, but only when you run it via flask run. That's beside the point though.
In order to hook into werkzeug, the cs50 library needs to be imported (so that the code that does the hooking can run). However, the SyntaxError is thrown before any code is run, hence we never get a chance to hook into werkzeug. Motivating example:
__import__("os")._exit(0)
import cs50
from flask import Flask
app = Flask("foo")
def foo():
x = [
os._exit is the first thing that could possibly be run in this program (i.e. it runs before cs50 is imported which is what causes the exception to get colored), but it never gets run. The SyntaxError gets thrown first.
Similarly:
raise Exception
import cs50
The exceptions here don't get colored either, precisely because the import cs50 line never gets executed.
This isn't specific to flask at all. If you run that snippet in just the regular Python interpreter, you get an uncolored exception (swapping the two lines gives you a colored one).
Ah, gotcha, I worry I chose a bad example. Pretty sure the lack of highlighting reared its head for a student in another context; my introduction of the syntax error was just meant to distill their code into something simpler that would still raise an exception, but clearly a red herring. Will try to resurrect the actual symptom.