Skip to content

Commit

Permalink
Merge pull request #821 from plotly/791-cb-error-fix
Browse files Browse the repository at this point in the history 
Fix callback error reporting
  • Loading branch information
@byronz
byronz authored Jul 15, 2019
2 parents 5132fc5 + fbec9dc commit 11619d6
Show file tree
Hide file tree
Showing 4 changed files with 490 additions and 405 deletions.
3 changes: 3 additions & 0 deletions dash/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
## Unreleased
### Fixed
- [#821](https://github.com/plotly/dash/pull/821) Fix a bug with callback error reporting, [#791](https://github.com/plotly/dash/issues/791).

## [1.0.1] - 2019-07-09
### Changed
Expand Down
62 changes: 29 additions & 33 deletions dash/dash.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import pprint

from functools import wraps
from textwrap import dedent

import flask
from flask import Flask, Response
Expand Down Expand Up @@ -808,13 +809,13 @@ def _validate_callback(self, output, inputs, state):
if (layout is None and not self.config.suppress_callback_exceptions):
# Without a layout, we can't do validation on the IDs and
# properties of the elements in the callback.
raise exceptions.LayoutIsNotDefined('''
raise exceptions.LayoutIsNotDefined(dedent('''
Attempting to assign a callback to the application but
the `layout` property has not been assigned.
Assign the `layout` property before assigning callbacks.
Alternatively, suppress this warning by setting
`suppress_callback_exceptions=True`
'''.replace(' ', ''))
'''))

outputs = output if is_multi else [output]
for args, obj, name in [(outputs, Output, 'Output'),
Expand Down Expand Up @@ -849,7 +850,10 @@ def _validate_callback(self, output, inputs, state):
arg_id = arg.component_id
arg_prop = getattr(arg, 'component_property', None)
if (arg_id not in layout and arg_id != layout_id):
raise exceptions.NonExistentIdException('''
all_ids = [k for k in layout]
if layout_id:
all_ids.append(layout_id)
raise exceptions.NonExistentIdException(dedent('''
Attempting to assign a callback to the
component with the id "{0}" but no
components with id "{0}" exist in the
Expand All @@ -860,12 +864,7 @@ def _validate_callback(self, output, inputs, state):
(and therefore not in the initial layout), then
you can suppress this exception by setting
`suppress_callback_exceptions=True`.
'''.format(
arg_id,
list(layout.keys()) + (
[layout_id] if layout_id else []
)
).replace(' ', ''))
''').format(arg_id, all_ids))

component = (
layout if layout_id == arg_id else layout[arg_id]
Expand All @@ -875,34 +874,34 @@ def _validate_callback(self, output, inputs, state):
arg_prop not in component.available_properties and
not any(arg_prop.startswith(w) for w in
component.available_wildcard_properties)):
raise exceptions.NonExistentPropException('''
raise exceptions.NonExistentPropException(dedent('''
Attempting to assign a callback with
the property "{0}" but the component
"{1}" doesn't have "{0}" as a property.\n
Here are the available properties in "{1}":
{2}
'''.format(
''').format(
arg_prop, arg_id, component.available_properties
).replace(' ', ''))
))

if hasattr(arg, 'component_event'):
raise exceptions.NonExistentEventException('''
raise exceptions.NonExistentEventException(dedent('''
Events have been removed.
Use the associated property instead.
'''.replace(' ', ''))
'''))

if state and not inputs:
raise exceptions.MissingInputsException('''
raise exceptions.MissingInputsException(dedent('''
This callback has {} `State` {}
but no `Input` elements.\n
Without `Input` elements, this callback
will never get called.\n
(Subscribing to input components will cause the
callback to be called whenever their values change.)
'''.format(
''').format(
len(state),
'elements' if len(state) > 1 else 'element'
).replace(' ', ''))
))

for i in inputs:
bad = None
Expand Down Expand Up @@ -953,25 +952,22 @@ def duplicate_check():
return callback_id in callbacks
if duplicate_check():
if is_multi:
msg = '''
msg = dedent('''
Multi output {} contains an `Output` object
that was already assigned.
Duplicates:
{}
'''.format(
''').format(
callback_id,
pprint.pformat(ns['duplicates'])
).replace(' ', '')
)
else:
msg = '''
msg = dedent('''
You have already assigned a callback to the output
with ID "{}" and property "{}". An output can only have
a single callback function. Try combining your inputs and
callback functions together into one function.
'''.format(
output.component_id,
output.component_property
).replace(' ', '')
''').format(output.component_id, output.component_property)
raise exceptions.DuplicateCallbackOutput(msg)

@staticmethod
Expand All @@ -984,7 +980,7 @@ def _raise_invalid(bad_val, outer_val, path, index=None,
outer_id = "(id={:s})".format(outer_val.id) \
if getattr(outer_val, 'id', False) else ''
outer_type = type(outer_val).__name__
raise exceptions.InvalidCallbackReturnValue('''
raise exceptions.InvalidCallbackReturnValue(dedent('''
The callback for `{output:s}`
returned a {object:s} having type `{type:s}`
which is not JSON serializable.
Expand All @@ -996,15 +992,15 @@ def _raise_invalid(bad_val, outer_val, path, index=None,
In general, Dash properties can only be
dash components, strings, dictionaries, numbers, None,
or lists of those.
'''.format(
''').format(
output=repr(output),
object='tree with one value' if not toplevel else 'value',
type=bad_type,
location_header=(
'The value in question is located at'
if not toplevel else
'''The value in question is either the only value returned,
or is in the top level of the returned list,'''
'The value in question is either the only value returned,'
'\nor is in the top level of the returned list,'
),
location=(
"\n" +
Expand All @@ -1014,7 +1010,7 @@ def _raise_invalid(bad_val, outer_val, path, index=None,
+ "\n" + path + "\n"
) if not toplevel else '',
bad_val=bad_val
).replace(' ', ''))
))

def _value_is_valid(val):
return (
Expand Down Expand Up @@ -1228,18 +1224,18 @@ def add_context(*args, **kwargs):
)
except TypeError:
self._validate_callback_output(output_value, output)
raise exceptions.InvalidCallbackReturnValue('''
raise exceptions.InvalidCallbackReturnValue(dedent('''
The callback for property `{property:s}`
of component `{id:s}` returned a value
which is not JSON serializable.
In general, Dash properties can only be
dash components, strings, dictionaries, numbers, None,
or lists of those.
'''.format(
''').format(
property=output.component_property,
id=output.component_id
).replace(' ', ''))
))

return jsonResponse

Expand Down
28 changes: 17 additions & 11 deletions dash/testing/browser.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def __exit__(self, exc_type, exc_val, traceback):
self.driver.quit()
self.percy_runner.finalize_build()
except WebDriverException:
logger.exception("webdriver quit was not successfully")
logger.exception("webdriver quit was not successful")
except percy.errors.Error:
logger.exception("percy runner failed to finalize properly")

Expand Down Expand Up @@ -247,16 +247,22 @@ def open_new_tab(self, url=None):
)

def get_webdriver(self, remote):
return (
getattr(self, "_get_{}".format(self._browser))()
if remote is None
else webdriver.Remote(
command_executor=remote,
desired_capabilities=getattr(
DesiredCapabilities, self._browser.upper()
),
)
)
# occasionally the browser fails to start - give it 3 tries
for i in reversed(range(3)):
try:
return (
getattr(self, "_get_{}".format(self._browser))()
if remote is None
else webdriver.Remote(
command_executor=remote,
desired_capabilities=getattr(
DesiredCapabilities, self._browser.upper()
),
)
)
except WebDriverException:
if not i:
raise

def _get_wd_options(self):
options = (
Expand Down
Loading

0 comments on commit 11619d6

Please sign in to comment.