Add callback on_error handler #2903

T4rk1n · 2024-06-26T19:48:11Z

Add two error handler to call when a callback raise an exception, the return value of the handler is instead used as output. The callback context can be used to differentiate the erroring callbacks and access the original inputs and states.

A global error handler can be added with a function reference to the Dash constructor: eg: app = Dash(on_error=handler).
And a local handler for particular callback can be added to individual callback: eg: @callback(..., on_error=handler).

The signature of the handler function is: def func(error: Exception) -> None. set_props can be used to set extra props or the original output.

Example:

def global_callback_error_handler(err):
    set_props("output-global": {"children": "error: {err}")

app = Dash(on_error=global_callback_error_handler)

app.layout = [
    html.Button("start", id="start-local"),
    html.Button("start-global", id="start-global"),
    html.Div(id="output"),
    html.Div(id="output-global"),
]

def on_callback_error(err):
    set_props("output": {"children": "callback error: {err}")

@app.callback(
    Output("output", "children"),
    Input("start-local", "n_clicks"),
    on_error=on_callback_error,
    prevent_initial_call=True,
)
def on_start(_):
    raise Exception("local error")

@app.callback(
    Output("output-global", "children"),
    Input("start-global", "n_clicks"),
    prevent_initial_call=True,
)
def on_start_global(_):
    raise Exception("global error")

Partially based of #2829 to catch exception and call a function instead of a decorator pattern and also works with background callbacks.

Closes #2348

emilykl · 2024-06-26T22:10:08Z

dash/_callback.py

@@ -67,6 +68,7 @@ def callback(
    cancel=None,
    manager=None,
    cache_args_to_ignore=None,
+    on_error: Optional[Callable[[Exception], Any]] = None,


Type hints ❤️ ❤️

emilykl · 2024-06-26T22:12:18Z

.pylintrc

@@ -414,7 +414,7 @@ max-bool-expr=5
 max-branches=15

 # Maximum number of locals for function / method body
-max-locals=20
+max-locals=25


Is it possible to move some of the logic to a sub-function rather than increasing this limit?

Oh I forgot to mention in the PR description, this rule is broken cannot be concatenated with other rule disabling so i had to change this instead.

emilykl · 2024-06-26T22:12:54Z

dash/_callback.py

@@ -272,8 +279,8 @@ def insert_callback(
    return callback_id


-# pylint: disable=R0912, R0915
-def register_callback(  # pylint: disable=R0914
+# pylint: disable=too-many-branches,too-many-statements


Same question as above -- can this function be lightened a bit by moving some logic to sub-functions?

@T4rk1n any thoughts on ^ ? Not to be a stickler and it's not a blocker for merging this PR but would love to see this function simplified if possible rather than disabling the linter

I didn't disable anything in this PR, just changed to the rulename instead of number. The settings for these rules is kinda low imo,

too-many-branches=15 is kinda high, but this function is wrapped multiple times in decorators which is why it's triggered here.

too-many-statements=50 is kinda low for a project this size and keep getting triggered everywhere, we also have functions within functions which actually separate the code inside but the linter isn't too smart here and the top function accumulates all the statements.

variable numbers also get from the fact this is multiple functions wrapped.

Oh I see now, thanks for the explanation. 👍

emilykl · 2024-06-26T22:16:28Z

dash/_callback.py

@@ -462,7 +473,15 @@ def add_context(*args, **kwargs):
                if output_value is callback_manager.UNDEFINED:
                    return to_json(response)
            else:
-                output_value = _invoke_callback(func, *func_args, **func_kwargs)
+                try:


emilykl · 2024-06-26T22:21:44Z

dash/_callback.py

+                try:
+                    output_value = _invoke_callback(func, *func_args, **func_kwargs)
+                except Exception as err:  # pylint: disable=broad-exception-caught
+                    if on_error:


Why not do something like on_error_func = on_error or app_on_error further up in the code, to avoid this extra elif

emilykl · 2024-06-26T22:32:17Z

@T4rk1n This rocks!!!

I'm going to play around with this feature tomorrow and try to break it.

Couple of questions in the meantime:

Does the global error handler also need to have the same number of return values as callback outputs? How does that work with apps with callbacks with different numbers of outputs?
Regarding the API: Did you consider adding something like an error_output argument, which lets you specify an output that gets used only in case of error? Would that be possible?

T4rk1n · 2024-06-27T13:43:21Z

@T4rk1n This rocks!!!

I'm going to play around with this feature tomorrow and try to break it.

Couple of questions in the meantime:

* Does the global error handler also need to have the same number of return values as callback outputs? How does that work with apps with callbacks with different numbers of outputs?

* Regarding the API: Did you consider adding something like an `error_output` argument, which lets you specify an output that gets used only in case of error? Would that be possible?

The global error handler needs the same outputs, to know the outputs the callback_context can be used. I just noticed that ctx.output_list has deprecation notice if using grouping so need to use ctx.output_grouping in the case of returning a dictionary? (@LiamConnors the docs for this is missing)

Did you consider adding something like an error_output argument,

set_props can handle this.

BSd3v · 2024-06-27T15:19:10Z

@T4rk1n,

If the on_error needs the same outputs as the target callback, wouldnt you encounter an error if these were misconfigured and fall silently when in production?

If this is also the case, then this is not really a global error handler as it can fail unexpectedly and is no different than placing error handling inside of the callback in the first place?

I think the on_error needs to return the same outputs as the renderer is expecting in order to allow for the callback to finish the updates on a component and close the running, and is not necessarily a restriction of the on_error. Wouldnt it make sense to do something like a prevent update for the original callback props?

T4rk1n · 2024-06-27T19:05:47Z

If the on_error needs the same outputs as the target callback, wouldnt you encounter an error if these were misconfigured and fall silently when in production?

The handler return values goes thru the same validation as the original callback function, the exception is raised.

Wouldnt it make sense to do something like a prevent update for the original callback props?

Can still do raise PreventUpdate or return [no_update for _ in ctx.outputs_list] in the handler to prevent the outputs from updating if not required. I think leaving this the same return value for the error handler allow for more possibilities than if we automatically prevent update. cc @ndrezn

emilykl · 2024-06-27T19:48:59Z

Can still do raise PreventUpdate or return [no_update for _ in ctx.outputs_list] in the handler to prevent the outputs from updating if not required. I think leaving this the same return value for the error handler allow for more possibilities than if we automatically prevent update. cc @ndrezn

I like the idea of automatically adding a PreventUpdate or [no_update for _ in ctx.outputs_list] if the error handler doesn't have a return value -- would that be feasible @T4rk1n ?

BSd3v · 2024-06-28T19:43:44Z

Another thing, is if someone does raise PreventUpdate in a legit callback return, this flags as an error and would trigger the on_error, is this correct?

emilykl · 2024-07-01T22:51:30Z

Another thing, is if someone does raise PreventUpdate in a legit callback return, this flags as an error and would trigger the on_error, is this correct?

Just tried this out and it looks like currently it does (cc @T4rk1n). I suppose the fix would be to modify the try/except so that it catches every type of error except PreventUpdate.

T4rk1n · 2024-07-02T12:56:28Z

would that be feasible @T4rk1n ?

Yes, I think that would be less confusing, the outputs would still be able to be changed via set_props.

gvwilson · 2024-07-08T15:37:10Z

@emilykl can you please give this one final look and approve the merge?

emilykl · 2024-07-08T21:17:48Z

dash/_callback.py

+                    if error_handler:
+                        error_handler(err)
+                        if not multi:
+                            output_value = NoUpdate()
+                        else:
+                            output_value = [NoUpdate for _ in output_spec]


Suggested change

if error_handler:

error_handler(err)

if not multi:

output_value = NoUpdate()

else:

output_value = [NoUpdate for _ in output_spec]

if error_handler:

output_value = error_handler(err)

if not multi:

output_value = output_value or NoUpdate()

else:

output_value = output_value or [NoUpdate() for _ in output_spec]

@T4rk1n I feel that we should still allow the dev to return outputs from the error handler -- we should supply the NoUpdate() outputs only if the error handler doesn't return anything. (I realize the dev could still update the outputs via set_props(), but I don't see any downside to also allowing them to output the "normal" way.) Happy to discuss further if you disagree.

For this wouldn't the dev just handle the error inside of the callback itself, as no two outputs are going to be the same?

Also, I thought that this would return a regular output? The no_update seems to be only supplied if there was no returned value? -- wait, I read your suggestion @emilykl and thought it was the code in question and not your suggestion.

Doing it like this could lead to situation where you wanted to output something that evaluate to false but it would get a no_update instead. see #2906 (comment), a components which has no children props will always be false.

Best we could do is if output_value is not None, but that brings back the difference between undefined vs none, which python lacks an undefined type and None could be a valid prop.

What about this:

if error_handler: output_value = error_handler(err) if output_value is not None: # Code to execute if a value is returned '''output no change''' else: # Code to execute if no value is returned output_value = [NoUpdate() for _ in output_spec] if multi else NoUpdate()

Then have it documented that this will pass NoUpdate in error if you are really trying to pass a None as a single output. If this is the case, then the dev should instead place: [None]

For things that cant be solved by this, the error should be handled inside of the callback itself before it bubbles up to the error handler. One example I just tested was trying to reset a dropdown value.

For this wouldn't the dev just handle the error inside of the callback itself, as no two outputs are going to be the same?

For the global error handler, yes, this is true -- I can't imagine a situation where you'd want to return specific outputs from the global error handler. But for a callback-specific error handler the dev might want to return specific outputs.

python lacks an undefined type and None could be a valid prop.

Ugh, this is a good point. Thanks Python.

I think it would be OK to check if output_value is not None, and then document that in the specific edge case where you do actually want to return None, you have to use set_props().

callback-specific error handler

unless they are reusing, they could still handle inside of the callback, but our syntax is still easier I guess, haha. Rather than them having to wrap the whole function in a try except clause.

Changed to NoOutput only if output_value is None, the output could be set to [None] if the dev want to set the output prop to null. cc @LiamConnors

the output could be set to [None] if the dev want to set the output prop to null.

FWIW, does this actually work? Typically I have gotten errors when wrapping a single output in a list.

Yes, it works, but only would work if the output definition in the callback is [Output(id, prop)]

emilykl · 2024-07-08T21:20:07Z

dash/_callback.py

+                try:
+                    output_value = _invoke_callback(func, *func_args, **func_kwargs)
+                except PreventUpdate as err:
+                    raise err


emilykl · 2024-07-08T21:31:04Z

dash/_callback.py

+                        error_handler(exc)
+                        output_value = NoUpdate()


see https://github.com/plotly/dash/pull/2903/files#r1669336763

Suggested change

error_handler(exc)

output_value = NoUpdate()

output_value = error_handler(exc)

output_value = output_value or NoUpdate()

emilykl · 2024-07-08T22:04:21Z

@T4rk1n See my comments regarding outputs from the error handler. Otherwise LGTM.

BSd3v · 2024-07-09T01:01:40Z

Just going to throw this here:

This PR, once merged should close this PR: #2829

And also close this issue: #2348

…rror

emilykl

@T4rk1n Could you maybe update the test cases so there's one test case with an error handler function which returns callback outputs?

In any case, looks good! 🚀

Add callback on_error handler

02de64d

T4rk1n requested review from alexcjohnson and ndrezn as code owners June 26, 2024 19:48

T4rk1n requested a review from emilykl June 26, 2024 19:52

Fix celery on_error

0a19ac5

emilykl reviewed Jun 26, 2024

View reviewed changes

T4rk1n added 3 commits July 2, 2024 12:47

raise back PreventUpdate

40130bd

on error without returning outputs, fix celery on_error.

30d6f11

Merge branch 'dev' into feat/on-error

a485b4e

emilykl reviewed Jul 8, 2024

View reviewed changes

T4rk1n added 2 commits July 10, 2024 14:20

Take the output value from the error handler only if not None.

3f00325

Merge branch 'feat/on-error' of github.com:plotly/dash into feat/on-e…

977b809

…rror

emilykl approved these changes Jul 10, 2024

View reviewed changes

Test output is handled in error handler.

db28caf

T4rk1n merged commit 351a81f into dev Jul 11, 2024
3 checks passed

T4rk1n deleted the feat/on-error branch July 11, 2024 14:23

BSd3v mentioned this pull request Jul 11, 2024

Add support for custom error handling in Dash #2829

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add callback on_error handler #2903

Add callback on_error handler #2903

T4rk1n commented Jun 26, 2024 •

edited by ndrezn

Loading

emilykl Jun 26, 2024 •

edited

Loading

emilykl Jun 26, 2024

T4rk1n Jun 27, 2024

emilykl Jun 26, 2024

emilykl Jul 8, 2024

T4rk1n Jul 10, 2024 •

edited

Loading

emilykl Jul 10, 2024

emilykl Jun 26, 2024

emilykl Jun 26, 2024

emilykl commented Jun 26, 2024

T4rk1n commented Jun 27, 2024 •

edited

Loading

BSd3v commented Jun 27, 2024

T4rk1n commented Jun 27, 2024

emilykl commented Jun 27, 2024 •

edited

Loading

BSd3v commented Jun 28, 2024 •

edited by emilykl

Loading

emilykl commented Jul 1, 2024

T4rk1n commented Jul 2, 2024

gvwilson commented Jul 8, 2024

emilykl Jul 8, 2024

BSd3v Jul 9, 2024 •

edited

Loading

T4rk1n Jul 9, 2024 •

edited

Loading

BSd3v Jul 9, 2024 •

edited

Loading

emilykl Jul 9, 2024 •

edited

Loading

BSd3v Jul 9, 2024

T4rk1n Jul 10, 2024

emilykl Jul 10, 2024

BSd3v Jul 10, 2024

emilykl Jul 8, 2024

emilykl Jul 8, 2024

emilykl commented Jul 8, 2024

BSd3v commented Jul 9, 2024

emilykl left a comment

Add callback on_error handler #2903

Add callback on_error handler #2903

Conversation

T4rk1n commented Jun 26, 2024 • edited by ndrezn Loading

emilykl Jun 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

T4rk1n Jul 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emilykl commented Jun 26, 2024

T4rk1n commented Jun 27, 2024 • edited Loading

BSd3v commented Jun 27, 2024

T4rk1n commented Jun 27, 2024

emilykl commented Jun 27, 2024 • edited Loading

BSd3v commented Jun 28, 2024 • edited by emilykl Loading

emilykl commented Jul 1, 2024

T4rk1n commented Jul 2, 2024

gvwilson commented Jul 8, 2024

Choose a reason for hiding this comment

BSd3v Jul 9, 2024 • edited Loading

Choose a reason for hiding this comment

T4rk1n Jul 9, 2024 • edited Loading

Choose a reason for hiding this comment

BSd3v Jul 9, 2024 • edited Loading

Choose a reason for hiding this comment

emilykl Jul 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emilykl commented Jul 8, 2024

BSd3v commented Jul 9, 2024

emilykl left a comment

Choose a reason for hiding this comment

T4rk1n commented Jun 26, 2024 •

edited by ndrezn

Loading

emilykl Jun 26, 2024 •

edited

Loading

T4rk1n Jul 10, 2024 •

edited

Loading

T4rk1n commented Jun 27, 2024 •

edited

Loading

emilykl commented Jun 27, 2024 •

edited

Loading

BSd3v commented Jun 28, 2024 •

edited by emilykl

Loading

BSd3v Jul 9, 2024 •

edited

Loading

T4rk1n Jul 9, 2024 •

edited

Loading

BSd3v Jul 9, 2024 •

edited

Loading

emilykl Jul 9, 2024 •

edited

Loading