plotly / dash

Data Apps & Dashboards for Python. No JavaScript Required.
https://plotly.com/dash
MIT License
21.54k stars 2.08k forks source link

Flag 'suppress_callback_exceptions' not working as intended #3054

Open emer-bestseller opened 3 weeks ago

emer-bestseller commented 3 weeks ago

Describe your context

dash                       2.18.1
dash_ag_grid               31.2.0
dash-bootstrap-components  1.5.0
dash-bootstrap-templates   1.0.8
dash-core-components       2.0.0
dash-daq                   0.5.0
dash-extensions            1.0.15
dash-html-components       2.0.0
dash-iconify               0.1.2
dash_mantine_components    0.14.5
dash-table                 5.0.0

Describe the bug

The suppress_callback_exceptions doesn't seem to be working as intended. If I run this MWE,

import dash
import dash_html_components as html
from dash.dependencies import Input, Output

app = dash.Dash(
    __name__,
    prevent_initial_callbacks=True,
    suppress_callback_exceptions=True,
)

app.layout = html.Div(
    [
        html.Button(id="button", children="Button Exists"),
        html.Div(id="exists"),
    ]
)

@app.callback(
    Output("exists", "children"),
    Input("button", "n_clicks"),
    Input("not-exists", "n_clicks"),
)
def on_click(_, __):
    return "Hello world"

if __name__ == "__main__":
    app.run_server()

and click the button, I get a console error,

ReferenceError: A nonexistent object was used in an `Input` of a Dash callback. The id of this object is `not-exists` and the property is `n_clicks`. The string ids in the current layout are: [button, exists]

Expected behavior

As per the documentation, if this flag is set, I would expect the callback simply to not fire (and thus no errors to be raised).

... if you define a callback with only a subset of the specified Inputs present in the current page layout, the callback will simply not fire at all.

https://dash.plotly.com/callback-gotchas

Screenshots

Image

Image

emer-bestseller commented 3 weeks ago

It looks like the error originates here,

https://github.com/plotly/dash/blob/0d9cd2c2a611e1b8cce21d1d46b69234c20bdb11/dash/dash-renderer/src/actions/callbacks.ts#L109-L126

Would it make sense to suppress this error similar to how it's done in updateComponent?

https://github.com/plotly/dash/blob/0d9cd2c2a611e1b8cce21d1d46b69234c20bdb11/dash/dash-renderer/src/actions/callbacks.ts#L341

What do you think, T4rk1n?

CNFeffery commented 3 weeks ago

@emer-bestseller I think the parameter suppress_callback_exceptions is to ignore those errors about ID not found in layout, like dynamic callbacks, not the A nonexistent object was used in an XXX of a Dash callback. errors.

CNFeffery commented 3 weeks ago

By the way, there is a trick to handle this kind of error with ALL pattern-match:

import dash
from dash import html
from dash.dependencies import Input, Output, ALL

app = dash.Dash(
    __name__,
    prevent_initial_callbacks=True,
    suppress_callback_exceptions=True,
)

app.layout = html.Div(
    [
        html.Button(id="button", children="Button Exists"),
        html.Div(id={"type": "exists", "index": "whatever"}),
    ]
)

@app.callback(
    Output("exists", "children"),
    Input("button", "n_clicks"),
    Input({"type": "exists", "index": ALL}, "n_clicks"),
)
def on_click(_, __):
    return "Hello world"

if __name__ == "__main__":
    app.run(debug=True)
emer-bestseller commented 3 weeks ago

Ah, so you're saying that it's in fact intended only to silence errors related to (initial) layout validation? In that case, I guess the code is correct, but that the documentation might need some clarification 👍

Reading through the source code, I also started to think about the "hack" of using an ALL wildcard to circumvent the issue. I guess that could be a workaround. Introducing pattern matching callback does increase code complexity somewhat though (partly in syntax, partly in concept; by adding an ALL wildcard the code suggests that there can be multiple elements while in fact the intent is that there will always be exactly one - or zero, of course), so maybe it would make sense to have (another?) flag to silence the exception mentioned in the issue? (:

I don't see any immediate downside of introducing such a flag / allowing these kind of dynamic layout constructs.