From 433a65e53c31233e9827a2865eafbeddd425d087 Mon Sep 17 00:00:00 2001 From: Ryan Marren Date: Mon, 18 Jun 2018 20:53:59 -0400 Subject: [PATCH 1/8] More informative JSON not serializable errors with paths. --- dash/dash.py | 57 ++++++++++++++++++++++++++++++ dash/development/base_component.py | 25 +++++++++---- dash/exceptions.py | 4 +++ 3 files changed, 79 insertions(+), 7 deletions(-) diff --git a/dash/dash.py b/dash/dash.py index 143d1e802b..030a043df6 100644 --- a/dash/dash.py +++ b/dash/dash.py @@ -475,6 +475,62 @@ def _validate_callback(self, output, inputs, state, events): output.component_id, output.component_property).replace(' ', '')) + def _validate_callback_output(self, output_value, output): + valid = [str, dict, int, float, type(None), Component] + + def _raise_invalid(bad_val, outer_type, bad_type, path, index=None): + raise exceptions.ReturnValueNotJSONSerializable(''' + The callback for property `{:s}` of component `{:s}` + returned a tree with one value having type `{:s}` + which is not JSON serializable. + + The value in question is located at + + `{:s}` + + and has string representation + + `{}`. + + In general, Dash properties can only be + dash components, strings, dictionaries, numbers, None, + or lists of those. + '''.format( + output.component_property, + output.component_id, + bad_type, + ( + "outer list index {:d} ({:s}) -> " + .format(index, outer_type) + if index is not None + else (outer_type + " -> ") + ) + path, + bad_val).replace(' ', '')) + + def _value_is_valid(val): + return ( + # pylint: disable=unused-variable + any([isinstance(val, x) for x in valid]) or + type(val).__name__ == 'unicode' + ) + + def _validate_value(val, index=None): + if isinstance(val, Component): + for p, j in val.traverse_with_paths(): + if not _value_is_valid(j): + _raise_invalid(j, type(val).__name__, type(j).__name__, + p, index) + else: + if not _value_is_valid(val): + _raise_invalid(val, type(val).__name__, type(val).__name__, + '', index) + + if isinstance(output_value, list): + for i, val in enumerate(output_value): + _validate_value(val, index=i) + else: + _validate_value(output_value) + # TODO - Update nomenclature. # "Parents" and "Children" should refer to the DOM tree # and not the dependency tree. @@ -513,6 +569,7 @@ def wrap_func(func): def add_context(*args, **kwargs): output_value = func(*args, **kwargs) + self._validate_callback_output(output_value, output) response = { 'response': { 'props': { diff --git a/dash/development/base_component.py b/dash/development/base_component.py index d647df2411..e45fdb901c 100644 --- a/dash/development/base_component.py +++ b/dash/development/base_component.py @@ -151,22 +151,33 @@ def __delitem__(self, id): # pylint: disable=redefined-builtin def traverse(self): """Yield each item in the tree.""" + for t in self.traverse_with_paths(): + yield t[1] + + def traverse_with_paths(self): + """Yield each item with its path in the tree.""" children = getattr(self, 'children', None) + children_type = type(children).__name__ # children is just a component if isinstance(children, Component): - yield children - for t in children.traverse(): - yield t + yield children_type, children + for p, t in children.traverse_with_paths(): + yield " -> ".join([children_type, p]), t # children is a list of components elif isinstance(children, collections.MutableSequence): - for i in children: # pylint: disable=not-an-iterable - yield i + for idx, i in enumerate(children): + list_path = "{:s} index {:d} (type {:s})".format( + children_type, + idx, + type(i).__name__ + ) + yield list_path, i if isinstance(i, Component): - for t in i.traverse(): - yield t + for p, t in i.traverse_with_paths(): + yield " -> ".join([list_path, p]), t def __iter__(self): """Yield IDs in the tree of children.""" diff --git a/dash/exceptions.py b/dash/exceptions.py index a8827d1faa..caddc5e7f6 100644 --- a/dash/exceptions.py +++ b/dash/exceptions.py @@ -44,3 +44,7 @@ class CantHaveMultipleOutputs(CallbackException): class PreventUpdate(CallbackException): pass + + +class ReturnValueNotJSONSerializable(CallbackException): + pass From 6bba6e9158c1390f16f6450f76d9e1e7bf9af630 Mon Sep 17 00:00:00 2001 From: Ryan Marren Date: Fri, 22 Jun 2018 08:05:17 -0400 Subject: [PATCH 2/8] Edge cases for JSON exception, and better formatting. --- dash/dash.py | 93 ++++++++++++++++++++++-------- dash/development/base_component.py | 17 +++--- 2 files changed, 80 insertions(+), 30 deletions(-) diff --git a/dash/dash.py b/dash/dash.py index 030a043df6..b3091d27f3 100644 --- a/dash/dash.py +++ b/dash/dash.py @@ -478,34 +478,41 @@ def _validate_callback(self, output, inputs, state, events): def _validate_callback_output(self, output_value, output): valid = [str, dict, int, float, type(None), Component] - def _raise_invalid(bad_val, outer_type, bad_type, path, index=None): + def _raise_invalid(bad_val, outer_val, bad_type, path, index=None, + toplevel=False): + outer_id = "id={:s}".format(outer_val.id) \ + if getattr(outer_val, 'id', False) else '' + outer_type = type(outer_val).__name__ raise exceptions.ReturnValueNotJSONSerializable(''' - The callback for property `{:s}` of component `{:s}` - returned a tree with one value having type `{:s}` + The callback for property `{property:s}` of component `{id:s}` + returned a {object:s} having type `{type:s}` which is not JSON serializable. - The value in question is located at - - `{:s}` - + {location_header:s}{location:s} and has string representation - - `{}`. + `{bad_val}` In general, Dash properties can only be dash components, strings, dictionaries, numbers, None, or lists of those. '''.format( - output.component_property, - output.component_id, - bad_type, - ( - "outer list index {:d} ({:s}) -> " - .format(index, outer_type) - if index is not None - else (outer_type + " -> ") - ) + path, - bad_val).replace(' ', '')) + property=output.component_property, + id=output.component_id, + 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,''' + ), + location=( + "\n" + + ("[{:d}] {:s} {:s}\n".format(index, outer_type, outer_id) + if index is not None + else (outer_type + ' ' + outer_id + "\n")) + path + "\n" + ) if not toplevel else '', + bad_val=bad_val).replace(' ', '')) def _value_is_valid(val): return ( @@ -515,15 +522,55 @@ def _value_is_valid(val): ) def _validate_value(val, index=None): + # val is a Component if isinstance(val, Component): for p, j in val.traverse_with_paths(): + # check each component value in the tree if not _value_is_valid(j): - _raise_invalid(j, type(val).__name__, type(j).__name__, - p, index) + _raise_invalid( + bad_val=j, + outer_val=val, + bad_type=type(j).__name__, + path=p, + index=index + ) + + # Children that are not of type Component or + # collections.MutableSequence not returned by traverse + child = getattr(j, 'children', None) + if not isinstance(child, collections.MutableSequence): + if child and not _value_is_valid(child): + _raise_invalid( + bad_val=child, + outer_val=val, + bad_type=type(child).__name__, + path=p + type(child).__name__, + index=index + ) + + # Also check the child of val, as it will not be returned + child = getattr(val, 'children', None) + if not isinstance(child, collections.MutableSequence): + if child and not _value_is_valid(child): + _raise_invalid( + bad_val=child, + outer_val=val, + bad_type=type(child).__name__, + path=type(child).__name__, + index=index + ) + + # val is not a Component, but is at the top level of tree else: if not _value_is_valid(val): - _raise_invalid(val, type(val).__name__, type(val).__name__, - '', index) + _raise_invalid( + bad_val=val, + outer_val=type(val).__name__, + bad_type=type(val).__name__, + path='', + index=index, + toplevel=True + ) if isinstance(output_value, list): for i, val in enumerate(output_value): diff --git a/dash/development/base_component.py b/dash/development/base_component.py index e45fdb901c..36da23a566 100644 --- a/dash/development/base_component.py +++ b/dash/development/base_component.py @@ -158,26 +158,29 @@ def traverse_with_paths(self): """Yield each item with its path in the tree.""" children = getattr(self, 'children', None) children_type = type(children).__name__ + children_id = "id={:s}".format(children.id) \ + if getattr(children, 'id', False) else '' + children_string = children_type + ' ' + children_id # children is just a component if isinstance(children, Component): - yield children_type, children + yield children_string + "\n", children for p, t in children.traverse_with_paths(): - yield " -> ".join([children_type, p]), t + yield "\n".join([children_string, p]) + "\n", t # children is a list of components elif isinstance(children, collections.MutableSequence): for idx, i in enumerate(children): - list_path = "{:s} index {:d} (type {:s})".format( - children_type, + list_path = "[{:d}] {:s} {}".format( idx, - type(i).__name__ + type(i).__name__, + "id={:s}".format(i.id) if getattr(i, 'id', False) else '' ) - yield list_path, i + yield list_path + "\n", i if isinstance(i, Component): for p, t in i.traverse_with_paths(): - yield " -> ".join([list_path, p]), t + yield "\n".join([list_path, p]) + "\n", t def __iter__(self): """Yield IDs in the tree of children.""" From ba33c6e2ad4be2908386ff885830087ddd236c45 Mon Sep 17 00:00:00 2001 From: Ryan Marren Date: Fri, 22 Jun 2018 08:31:35 -0400 Subject: [PATCH 3/8] Minor formatting updates. --- dash/dash.py | 9 +++++---- dash/development/base_component.py | 12 ++++++------ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/dash/dash.py b/dash/dash.py index b3091d27f3..92871c07ba 100644 --- a/dash/dash.py +++ b/dash/dash.py @@ -480,7 +480,7 @@ def _validate_callback_output(self, output_value, output): def _raise_invalid(bad_val, outer_val, bad_type, path, index=None, toplevel=False): - outer_id = "id={:s}".format(outer_val.id) \ + outer_id = "(id={:s})".format(outer_val.id) \ if getattr(outer_val, 'id', False) else '' outer_type = type(outer_val).__name__ raise exceptions.ReturnValueNotJSONSerializable(''' @@ -508,9 +508,10 @@ def _raise_invalid(bad_val, outer_val, bad_type, path, index=None, ), location=( "\n" + - ("[{:d}] {:s} {:s}\n".format(index, outer_type, outer_id) + ("[{:d}] {:s} {:s}".format(index, outer_type, outer_id) if index is not None - else (outer_type + ' ' + outer_id + "\n")) + path + "\n" + else ('- ' + outer_type + ' ' + outer_id)) + + "\n" + path + "\n" ) if not toplevel else '', bad_val=bad_val).replace(' ', '')) @@ -544,7 +545,7 @@ def _validate_value(val, index=None): bad_val=child, outer_val=val, bad_type=type(child).__name__, - path=p + type(child).__name__, + path=p + "\n" + type(child).__name__, index=index ) diff --git a/dash/development/base_component.py b/dash/development/base_component.py index 36da23a566..92941afe93 100644 --- a/dash/development/base_component.py +++ b/dash/development/base_component.py @@ -158,15 +158,15 @@ def traverse_with_paths(self): """Yield each item with its path in the tree.""" children = getattr(self, 'children', None) children_type = type(children).__name__ - children_id = "id={:s}".format(children.id) \ + children_id = "(id={:s})".format(children.id) \ if getattr(children, 'id', False) else '' children_string = children_type + ' ' + children_id # children is just a component if isinstance(children, Component): - yield children_string + "\n", children + yield children_string, children for p, t in children.traverse_with_paths(): - yield "\n".join([children_string, p]) + "\n", t + yield "\n".join([children_string, p]), t # children is a list of components elif isinstance(children, collections.MutableSequence): @@ -174,13 +174,13 @@ def traverse_with_paths(self): list_path = "[{:d}] {:s} {}".format( idx, type(i).__name__, - "id={:s}".format(i.id) if getattr(i, 'id', False) else '' + "(id={:s})".format(i.id) if getattr(i, 'id', False) else '' ) - yield list_path + "\n", i + yield list_path, i if isinstance(i, Component): for p, t in i.traverse_with_paths(): - yield "\n".join([list_path, p]) + "\n", t + yield "\n".join([list_path, p]), t def __iter__(self): """Yield IDs in the tree of children.""" From ab441f42fd1a55eefc565d95ad52cee8e4c5e17f Mon Sep 17 00:00:00 2001 From: Ryan Marren Date: Fri, 22 Jun 2018 08:37:26 -0400 Subject: [PATCH 4/8] Tab out singular elements in json exceptions to match list elements. --- dash/dash.py | 2 +- dash/development/base_component.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dash/dash.py b/dash/dash.py index 92871c07ba..c12b64daa9 100644 --- a/dash/dash.py +++ b/dash/dash.py @@ -545,7 +545,7 @@ def _validate_value(val, index=None): bad_val=child, outer_val=val, bad_type=type(child).__name__, - path=p + "\n" + type(child).__name__, + path=p + "\n- " + type(child).__name__, index=index ) diff --git a/dash/development/base_component.py b/dash/development/base_component.py index 92941afe93..0c714c24ae 100644 --- a/dash/development/base_component.py +++ b/dash/development/base_component.py @@ -164,9 +164,9 @@ def traverse_with_paths(self): # children is just a component if isinstance(children, Component): - yield children_string, children + yield "- " + children_string, children for p, t in children.traverse_with_paths(): - yield "\n".join([children_string, p]), t + yield "\n".join(["- " + children_string, p]), t # children is a list of components elif isinstance(children, collections.MutableSequence): From 7f609e85a7427730147b75ecfb6c15b6499f33e9 Mon Sep 17 00:00:00 2001 From: Ryan Marren Date: Wed, 4 Jul 2018 10:07:58 -0400 Subject: [PATCH 5/8] Change prefix for singleton components from '- ' to '[*] ' --- dash/dash.py | 4 ++-- dash/development/base_component.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dash/dash.py b/dash/dash.py index c12b64daa9..a940aea9fe 100644 --- a/dash/dash.py +++ b/dash/dash.py @@ -510,7 +510,7 @@ def _raise_invalid(bad_val, outer_val, bad_type, path, index=None, "\n" + ("[{:d}] {:s} {:s}".format(index, outer_type, outer_id) if index is not None - else ('- ' + outer_type + ' ' + outer_id)) + else ('[*] ' + outer_type + ' ' + outer_id)) + "\n" + path + "\n" ) if not toplevel else '', bad_val=bad_val).replace(' ', '')) @@ -545,7 +545,7 @@ def _validate_value(val, index=None): bad_val=child, outer_val=val, bad_type=type(child).__name__, - path=p + "\n- " + type(child).__name__, + path=p + "\n" + "[*] " + type(child).__name__, index=index ) diff --git a/dash/development/base_component.py b/dash/development/base_component.py index 0c714c24ae..2b3ad778aa 100644 --- a/dash/development/base_component.py +++ b/dash/development/base_component.py @@ -164,9 +164,9 @@ def traverse_with_paths(self): # children is just a component if isinstance(children, Component): - yield "- " + children_string, children + yield "[*] " + children_string, children for p, t in children.traverse_with_paths(): - yield "\n".join(["- " + children_string, p]), t + yield "\n".join(["[*] " + children_string, p]), t # children is a list of components elif isinstance(children, collections.MutableSequence): From ee8a4898c2f8b2d1ce668bd97494957eeb1a57e9 Mon Sep 17 00:00:00 2001 From: Ryan Marren Date: Wed, 18 Jul 2018 21:50:48 -0400 Subject: [PATCH 6/8] Only validate callback to throw exception after failure to serialize. --- dash/dash.py | 12 +++++++++--- dev-requirements.txt | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/dash/dash.py b/dash/dash.py index a940aea9fe..bfaa23be87 100644 --- a/dash/dash.py +++ b/dash/dash.py @@ -617,7 +617,6 @@ def wrap_func(func): def add_context(*args, **kwargs): output_value = func(*args, **kwargs) - self._validate_callback_output(output_value, output) response = { 'response': { 'props': { @@ -626,9 +625,16 @@ def add_context(*args, **kwargs): } } + try: + jsonResponse = json.dumps( + response, + cls=plotly.utils.PlotlyJSONEncoder + ) + except TypeError: + self._validate_callback_output(output_value, output) + return flask.Response( - json.dumps(response, - cls=plotly.utils.PlotlyJSONEncoder), + jsonResponse, mimetype='application/json' ) diff --git a/dev-requirements.txt b/dev-requirements.txt index 282b362edc..f667e354c3 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -12,4 +12,4 @@ six plotly>=2.0.8 requests[security] flake8 -pylint +pylint==1.9.2 From ed4ecf0a979332ed36b58d843230b0e8e938f0b9 Mon Sep 17 00:00:00 2001 From: Ryan Marren Date: Wed, 18 Jul 2018 22:40:15 -0400 Subject: [PATCH 7/8] Throw default exception if `_validate_callback_output` doesn't catch --- dash/dash.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/dash/dash.py b/dash/dash.py index bfaa23be87..d508459706 100644 --- a/dash/dash.py +++ b/dash/dash.py @@ -632,6 +632,16 @@ def add_context(*args, **kwargs): ) except TypeError: self._validate_callback_output(output_value, output) + raise exceptions.ReturnValueNotJSONSerializable(''' + 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(property=output.component_property, + id=output.component_id)) return flask.Response( jsonResponse, From 718e2597e0e17918ac260e211878d020d463d4db Mon Sep 17 00:00:00 2001 From: Ryan Marren Date: Tue, 24 Jul 2018 19:29:18 -0400 Subject: [PATCH 8/8] Change `ReturnValueNotJSONSerializable` to `InvalidCallbackReturnValue` --- dash/dash.py | 4 ++-- dash/exceptions.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dash/dash.py b/dash/dash.py index d508459706..3acf9f35cc 100644 --- a/dash/dash.py +++ b/dash/dash.py @@ -483,7 +483,7 @@ def _raise_invalid(bad_val, outer_val, bad_type, 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.ReturnValueNotJSONSerializable(''' + raise exceptions.InvalidCallbackReturnValue(''' The callback for property `{property:s}` of component `{id:s}` returned a {object:s} having type `{type:s}` which is not JSON serializable. @@ -632,7 +632,7 @@ def add_context(*args, **kwargs): ) except TypeError: self._validate_callback_output(output_value, output) - raise exceptions.ReturnValueNotJSONSerializable(''' + raise exceptions.InvalidCallbackReturnValue(''' The callback for property `{property:s}` of component `{id:s}` returned a value which is not JSON serializable. diff --git a/dash/exceptions.py b/dash/exceptions.py index caddc5e7f6..809b0b6843 100644 --- a/dash/exceptions.py +++ b/dash/exceptions.py @@ -46,5 +46,5 @@ class PreventUpdate(CallbackException): pass -class ReturnValueNotJSONSerializable(CallbackException): +class InvalidCallbackReturnValue(CallbackException): pass