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

dev_tools_prune_errors #860

Merged
merged 5 commits into from
Aug 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions dash/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## [UNRELEASED]
### Added
- [#860](https://github.com/plotly/dash/pull/860) Adds a new arg `dev_tools_prune_errors` to `app.run_server` and `app.enable_dev_tools`. Default `True`, tracebacks only include user code and below. Set it to `False` for the previous behavior showing all the Dash and Flask parts of the stack.

## [1.1.1] - 2019-08-06
### Changed
- Bumped dash-core-components version from 1.1.0 to [1.1.1](https://github.com/plotly/dash-core-components/blob/master/CHANGELOG.md#111---2019-08-06)
Expand Down
40 changes: 36 additions & 4 deletions dash/dash.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import flask
from flask import Flask, Response
from flask_compress import Compress
from werkzeug.debug.tbtools import get_current_traceback

import plotly
import dash_renderer
Expand Down Expand Up @@ -1168,7 +1169,8 @@ def callback(self, output, inputs=[], state=[]):
def wrap_func(func):
@wraps(func)
def add_context(*args, **kwargs):
output_value = func(*args, **kwargs)
# don't touch the comment on the next line - used by debugger
output_value = func(*args, **kwargs) # %% callback invoked %%
if multi:
if not isinstance(output_value, (list, tuple)):
raise exceptions.InvalidCallbackReturnValue(
Expand Down Expand Up @@ -1390,7 +1392,8 @@ def _setup_dev_tools(self, **kwargs):
'props_check',
'serve_dev_bundles',
'hot_reload',
'silence_routes_logging'
'silence_routes_logging',
'prune_errors'
):
dev_tools[attr] = get_combined_config(
attr, kwargs.get(attr, None), default=debug
Expand Down Expand Up @@ -1419,7 +1422,8 @@ def enable_dev_tools(
dev_tools_hot_reload_interval=None,
dev_tools_hot_reload_watch_interval=None,
dev_tools_hot_reload_max_retry=None,
dev_tools_silence_routes_logging=None):
dev_tools_silence_routes_logging=None,
dev_tools_prune_errors=None):
"""
Activate the dev tools, called by `run_server`. If your application is
served by wsgi and you want to activate the dev tools, you can call
Expand Down Expand Up @@ -1483,6 +1487,11 @@ def enable_dev_tools(
env: ``DASH_SILENCE_ROUTES_LOGGING``
:type dev_tools_silence_routes_logging: bool

:param dev_tools_prune_errors: Reduce tracebacks to just user code,
stripping out Flask and Dash pieces. `True` by default, set to
`False` to see the complete traceback.
:type dev_tools_prune_errors: bool

:return: debug
"""
if debug is None:
Expand All @@ -1497,7 +1506,8 @@ def enable_dev_tools(
hot_reload_interval=dev_tools_hot_reload_interval,
hot_reload_watch_interval=dev_tools_hot_reload_watch_interval,
hot_reload_max_retry=dev_tools_hot_reload_max_retry,
silence_routes_logging=dev_tools_silence_routes_logging
silence_routes_logging=dev_tools_silence_routes_logging,
prune_errors=dev_tools_prune_errors
)

if dev_tools.silence_routes_logging:
Expand Down Expand Up @@ -1527,6 +1537,21 @@ def enable_dev_tools(
_reload.watch_thread.daemon = True
_reload.watch_thread.start()

if debug and dev_tools.prune_errors:
@self.server.errorhandler(Exception)
def _wrap_errors(_):
# find the callback invocation, if the error is from a callback
# and skip the traceback up to that point
# if the error didn't come from inside a callback, we won't
# skip anything.
tb = get_current_traceback()
skip = 0
for i, line in enumerate(tb.plaintext.splitlines()):
if "%% callback invoked %%" in line:
skip = int((i + 1) / 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

why the skip is about half of code to the callback stub? just curious about the algorithm

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The plaintext traceback has 2 lines for each stack frame: first line lists the file, line #, and function; second line is the actual line of code. +1 gives the first line of the next pair - the first frame we want to report - then /2 gives the number of frames above it.

break
return get_current_traceback(skip=skip).render_full(), 500

if (debug and dev_tools.serve_dev_bundles and
not self.scripts.config.serve_locally):
# Dev bundles only works locally.
Expand Down Expand Up @@ -1594,6 +1619,7 @@ def run_server(
dev_tools_hot_reload_watch_interval=None,
dev_tools_hot_reload_max_retry=None,
dev_tools_silence_routes_logging=None,
dev_tools_prune_errors=None,
**flask_run_options):
"""
Start the flask server in local mode, you should not run this on a
Expand Down Expand Up @@ -1652,6 +1678,11 @@ def run_server(
env: ``DASH_SILENCE_ROUTES_LOGGING``
:type dev_tools_silence_routes_logging: bool

:param dev_tools_prune_errors: Reduce tracebacks to just user code,
stripping out Flask and Dash pieces. Only available with debugging.
`True` by default, set to `False` to see the complete traceback.
:type dev_tools_prune_errors: bool

:param flask_run_options: Given to `Flask.run`

:return:
Expand All @@ -1666,6 +1697,7 @@ def run_server(
dev_tools_hot_reload_watch_interval,
dev_tools_hot_reload_max_retry,
dev_tools_silence_routes_logging,
dev_tools_prune_errors
)

if self._dev_tools.silence_routes_logging:
Expand Down
72 changes: 70 additions & 2 deletions tests/integration/devtools/test_devtools_error_handling.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from dash.exceptions import PreventUpdate


def test_dveh001_python_errors(dash_duo):
def app_with_errors():
app = dash.Dash(__name__)

app.layout = html.Div(
Expand All @@ -19,10 +19,26 @@ def test_dveh001_python_errors(dash_duo):
@app.callback(Output("output", "children"), [Input("python", "n_clicks")])
def update_output(n_clicks):
if n_clicks == 1:
1 / 0
return bad_sub()
elif n_clicks == 2:
raise Exception("Special 2 clicks exception")

def bad_sub():
return 1 / 0

return app


def get_error_html(dash_duo, index):
# error is in an iframe so is annoying to read out - get it from the store
return dash_duo.driver.execute_script(
"return store.getState().error.backEnd[{}].error.html;".format(index)
)


def test_dveh001_python_errors(dash_duo):
app = app_with_errors()

dash_duo.start_server(
app,
debug=True,
Expand All @@ -49,6 +65,58 @@ def update_output(n_clicks):
dash_duo.find_element(".test-devtools-error-toggle").click()
dash_duo.percy_snapshot("devtools - python exception - 2 errors open")

# the top (first) error is the most recent one - ie from the second click
error0 = get_error_html(dash_duo, 0)
# user part of the traceback shown by default
assert 'in update_output' in error0
assert 'Special 2 clicks exception' in error0
assert 'in bad_sub' not in error0
# dash and flask part of the traceback not included
assert '%% callback invoked %%' not in error0
assert 'self.wsgi_app' not in error0

error1 = get_error_html(dash_duo, 1)
assert 'in update_output' in error1
assert 'in bad_sub' in error1
assert 'ZeroDivisionError' in error1
assert '%% callback invoked %%' not in error1
assert 'self.wsgi_app' not in error1


def test_dveh006_long_python_errors(dash_duo):
app = app_with_errors()

dash_duo.start_server(
app,
debug=True,
use_reloader=False,
use_debugger=True,
dev_tools_hot_reload=False,
dev_tools_prune_errors=False,
)

dash_duo.find_element("#python").click()
dash_duo.find_element("#python").click()
dash_duo.wait_for_text_to_equal(dash_duo.devtools_error_count_locator, "2")

dash_duo.find_element(".test-devtools-error-toggle").click()

error0 = get_error_html(dash_duo, 0)
assert 'in update_output' in error0
assert 'Special 2 clicks exception' in error0
assert 'in bad_sub' not in error0
# dash and flask part of the traceback ARE included
# since we set dev_tools_prune_errors=False
assert '%% callback invoked %%' in error0
assert 'self.wsgi_app' in error0

error1 = get_error_html(dash_duo, 1)
assert 'in update_output' in error1
assert 'in bad_sub' in error1
assert 'ZeroDivisionError' in error1
assert '%% callback invoked %%' in error1
assert 'self.wsgi_app' in error1


def test_dveh002_prevent_update_not_in_error_msg(dash_duo):
# raising PreventUpdate shouldn't display the error message
Expand Down
8 changes: 4 additions & 4 deletions tests/integration/devtools/test_props_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,22 @@
"fail": True,
"name": 'missing required "value" inside options',
"component": dcc.Checklist,
"props": {"options": [{"label": "hello"}], "values": ["test"]},
"props": {"options": [{"label": "hello"}], "value": ["test"]},
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is what infuriates me about Percy... OK, here it's just a test problem, not a real bug, but the images were completely different from what they should have been, and got auto-approved, I assume, because the change came from DCC and so likely happened first in master.

},
"invalid-nested-prop": {
"fail": True,
"name": "invalid nested prop",
"component": dcc.Checklist,
"props": {
"options": [{"label": "hello", "value": True}],
"values": ["test"],
"value": ["test"],
},
},
"invalid-arrayOf": {
"fail": True,
"name": "invalid arrayOf",
"component": dcc.Checklist,
"props": {"options": "test", "values": []},
"props": {"options": "test", "value": []},
},
"invalid-oneOf": {
"fail": True,
Expand Down Expand Up @@ -82,7 +82,7 @@
"component": dcc.Checklist,
"props": {
"options": [{"label": "hello", "value": "test"}],
"values": "test",
"value": "test",
},
},
"no-properties": {
Expand Down