dash icon indicating copy to clipboard operation
dash copied to clipboard

Add callback on_error handler

Open T4rk1n opened this issue 1 year ago • 6 comments
trafficstars

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 indivudual callback: eg: @callback(..., on_error=handler).

The signature of the handler function is: def func(error: Exception) -> Any, the return value must be same kind as the original callback. If multiple outputs, it needs multiple return value, etc.

Example:

def global_callback_error_handler(err):
    return f"global: {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):
    return f"callback: {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")

T4rk1n avatar Jun 26 '24 19:06 T4rk1n

@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?

emilykl avatar Jun 26 '24 22:06 emilykl

@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.

T4rk1n avatar Jun 27 '24 13:06 T4rk1n

@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?

BSd3v avatar Jun 27 '24 15:06 BSd3v

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

T4rk1n avatar Jun 27 '24 19:06 T4rk1n

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 ?

emilykl avatar Jun 27 '24 19:06 emilykl

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?

BSd3v avatar Jun 28 '24 19:06 BSd3v

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.

emilykl avatar Jul 01 '24 22:07 emilykl

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.

T4rk1n avatar Jul 02 '24 12:07 T4rk1n

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

gvwilson avatar Jul 08 '24 15:07 gvwilson

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

emilykl avatar Jul 08 '24 22:07 emilykl

Just going to throw this here:

This PR, once merged should close this PR: https://github.com/plotly/dash/pull/2829

And also close this issue: https://github.com/plotly/dash/issues/2348

BSd3v avatar Jul 09 '24 01:07 BSd3v