diff --git a/CHANGELOG.md b/CHANGELOG.md index 7103f5644c..0ce746a9ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ This project adheres to [Semantic Versioning](https://semver.org/). - [#3881](https://github.com/plotly/dash/pull/3881) Fix components rendered as props (eg. `dcc.Dropdown` option labels, `dcc.Tab` labels) crashing or failing to update when the host subtree was replaced by a callback; out-of-tree `ExternalWrapper` components now re-insert themselves and update in place. - [#3929](https://github.com/plotly/dash/issues/3929) Fix components that set their own initial state on mount (eg. `dash-bootstrap-components` `Tabs`) not applying it on first render, because descendant layout hashes were reset on the first fresh render (regression introduced in 4.2.0 by [#3570](https://github.com/plotly/dash/pull/3570)). - [#3948](https://github.com/plotly/dash/issues/3948) Fix page getting progressively slower as callbacks append children +- [#4012](https://github.com/plotly/dash/pull/4012) Fix `InvalidCallbackReturnValue` raised when a no-output background callback's error handler returns `None` (or the job is cancelled), by treating `None` returns from error handlers and the cancelled-job branch the same way — as `NoUpdate()` only when there are outputs, `None` otherwise. Fixes [#3628](https://github.com/plotly/dash/issues/3628). - [#4009](https://github.com/plotly/dash/issues/4009) Disable autocomplete in DatePickerSingle and DatePickerRange inputs. - [#3002](https://github.com/plotly/dash/issues/3002) Fix page loads failing with a 500 error on Windows when Dash is imported from an extended-length (`\\?\`) path, as in JupyterLab Desktop. Package resource paths are now joined with the OS separator instead of `/`. - [#4011](https://github.com/plotly/dash/issues/4011) Fix extra callback registrations when an app runs from the parent directory: `python app/app.py`, where the script shares its folder's name, registered its callbacks twice, and `python app/main.py` imported a sibling `app/app.py` as the parent package and registered its callbacks too (regression introduced in 4.4.1 by [#3883](https://github.com/plotly/dash/pull/3883)). diff --git a/dash/_callback.py b/dash/_callback.py index 91624c04eb..b71215f1f3 100644 --- a/dash/_callback.py +++ b/dash/_callback.py @@ -682,6 +682,7 @@ def _update_background_callback( kwargs, background, multi, + output_spec, cache_key=None, job_id=None, ): @@ -710,6 +711,7 @@ def _update_background_callback( error_handler, callback_ctx, multi, + output_spec, cache_key=cache_key, job_id=job_id, ) @@ -722,6 +724,7 @@ def _handle_rest_background_callback( error_handler, callback_ctx, multi, + output_spec, has_update=False, cache_key=None, job_id=None, @@ -730,7 +733,10 @@ def _handle_rest_background_callback( job_running = callback_manager.job_running(job_id) if job_id is not None else False if not job_running and output_value is callback_manager.UNDEFINED: # Job canceled -> no output to close the loop. - output_value = NoUpdate() + if output_spec: + output_value = NoUpdate() + else: + output_value = None elif isinstance(output_value, dict) and "background_callback_error" in output_value: error = output_value.get("background_callback_error", {}) @@ -740,7 +746,7 @@ def _handle_rest_background_callback( if error_handler: output_value = error_handler(exc) - if output_value is None: + if output_value is None and output_spec: output_value = NoUpdate() # set_props from the error handler uses the original ctx # instead of manager.get_updated_props since it runs in the @@ -974,7 +980,13 @@ def add_context(*args, **kwargs): ) output_value, has_update, skip = _update_background_callback( - error_handler, callback_ctx, response, kwargs, background, multi + error_handler, + callback_ctx, + response, + kwargs, + background, + multi, + output_spec, ) if skip: return output_value @@ -1046,7 +1058,13 @@ async def async_add_context(*args, **kwargs): callback_ctx, ) output_value, has_update, skip = _update_background_callback( - error_handler, callback_ctx, response, kwargs, background, multi + error_handler, + callback_ctx, + response, + kwargs, + background, + multi, + output_spec, ) if skip: return output_value diff --git a/dash/mcp/tasks/tasks.py b/dash/mcp/tasks/tasks.py index 5f0ac3faf2..f200ce3abf 100644 --- a/dash/mcp/tasks/tasks.py +++ b/dash/mcp/tasks/tasks.py @@ -12,6 +12,7 @@ from dash._callback import _update_background_callback, _prepare_response from dash._utils import AttributeDict, split_callback_id from dash.development.base_component import ComponentRegistry +from dash.exceptions import PreventUpdate from dash.mcp.primitives.tools.results import format_callback_response from dash.mcp.types import MCPError from dash.types import CallbackExecutionResponse @@ -146,7 +147,7 @@ def get_task_result(task_id: str) -> Any: background = cb_info.get("background") has_output = not cb_info.get("no_output") multi = adapter.output_id.startswith("..") - output = split_callback_id(adapter.output_id) + output = split_callback_id(adapter.output_id) if has_output else [] # Build the body to get output_spec, same as as_callback_body body = adapter.as_callback_body({}) @@ -162,6 +163,7 @@ def get_task_result(task_id: str) -> Any: kwargs={"background_callback_manager": manager}, background=background, multi=multi, + output_spec=output_spec, cache_key=cache_key, job_id=job_id, ) @@ -172,21 +174,24 @@ def get_task_result(task_id: str) -> Any: "Task result not ready. Poll tasks/get until status is 'completed'." ) - _prepare_response( - output_value, - output_spec, - multi, - response, - callback_ctx, - app, - set(ComponentRegistry.registry), - background, - has_update, - has_output, - output, - adapter.output_id, - cb_info.get("allow_dynamic_callbacks"), - ) + try: + _prepare_response( + output_value, + output_spec, + multi, + response, + callback_ctx, + app, + set(ComponentRegistry.registry), + background, + has_update, + has_output, + output, + adapter.output_id, + cb_info.get("allow_dynamic_callbacks"), + ) + except PreventUpdate: + return format_callback_response(response, adapter) return format_callback_response(response, adapter) diff --git a/tests/background_callback/app_bg_on_error.py b/tests/background_callback/app_bg_on_error.py index 724531f092..3945695e53 100644 --- a/tests/background_callback/app_bg_on_error.py +++ b/tests/background_callback/app_bg_on_error.py @@ -20,6 +20,8 @@ def global_error_handler(err): html.Div(id="cb-output"), html.Button("global on_error", id="start-global-onerror"), html.Div(id="global-output"), + html.Button("no-output callback on_error", id="start-no-output-cb-onerror"), + html.Div(id="no-output-cb-onerror-output"), ] @@ -48,5 +50,19 @@ def on_click_global(_): raise Exception("global error") +def no_output_callback_on_error(err): + set_props("no-output-cb-onerror-output", {"children": f"callback: {err}"}) + + +@app.callback( + Input("start-no-output-cb-onerror", "n_clicks"), + prevent_initial_call=True, + background=True, + on_error=no_output_callback_on_error, +) +def on_click_no_output(_): + raise Exception("no output callback error") + + if __name__ == "__main__": app.run(debug=True) diff --git a/tests/background_callback/test_basic_long_callback019.py b/tests/background_callback/test_basic_long_callback019.py new file mode 100644 index 0000000000..c90731a71b --- /dev/null +++ b/tests/background_callback/test_basic_long_callback019.py @@ -0,0 +1,12 @@ +from tests.background_callback.utils import setup_background_callback_app + + +def test_lcbc019_background_callback_on_error_without_output(dash_duo, manager): + with setup_background_callback_app(manager, "app_bg_on_error") as app: + dash_duo.start_server(app) + + dash_duo.find_element("#start-no-output-cb-onerror").click() + dash_duo.wait_for_contains_text( + "#no-output-cb-onerror-output", + "callback: An error occurred inside a background callback: no output callback error", + ) diff --git a/tests/integration/mcp/test_mcp_background_tasks.py b/tests/integration/mcp/test_mcp_background_tasks.py index 3bbfcfebef..c3967dae6f 100644 --- a/tests/integration/mcp/test_mcp_background_tasks.py +++ b/tests/integration/mcp/test_mcp_background_tasks.py @@ -17,6 +17,8 @@ from dash import Dash, Input, Output, html, _callback_signing from dash.background_callback.managers.diskcache_manager import DiskcacheManager +from tests.integration.mcp.conftest import _mcp_method + MCP_PATH = "_mcp" @@ -89,6 +91,41 @@ def _post(client, method, params=None, request_id=1): # --------------------------------------------------------------------------- +def test_mcpbg_cancel_no_output_task_result(dash_duo): + manager = DiskcacheManager(diskcache.Cache()) + app = Dash(__name__) + app.layout = html.Div(id="trigger") + + @app.callback( + Input("trigger", "children"), + background=True, + manager=manager, + prevent_initial_call=True, + ) + def no_output_callback(value): + time.sleep(0.5) + + dash_duo.start_server(app) + created = _mcp_method( + dash_duo.server_url, + "tools/call", + { + "name": "no_output_callback", + "arguments": {"value": "hello"}, + "task": {"ttl": 60000}, + }, + ) + task_id = created["result"]["task"]["taskId"] + job_id, _ = _unwrap_handles(app, task_id) + cancelled = _mcp_method(dash_duo.server_url, "tasks/cancel", {"taskId": task_id}) + assert cancelled["result"]["status"] == "cancelled" + assert not manager.job_running(job_id) + + result = _mcp_method(dash_duo.server_url, "tasks/result", {"taskId": task_id}) + assert result["result"]["structuredContent"] == {"multi": True} + assert dash_duo.get_logs() == [] + + def test_mcpbg012_trigger_poll_and_retrieve(): app = _make_background_app() client = app.server.test_client() diff --git a/tests/unit/mcp/tools/test_mcp_background_callbacks.py b/tests/unit/mcp/tools/test_mcp_background_callbacks.py index 8a9b96bbb8..aee3dcf3d5 100644 --- a/tests/unit/mcp/tools/test_mcp_background_callbacks.py +++ b/tests/unit/mcp/tools/test_mcp_background_callbacks.py @@ -94,7 +94,9 @@ def _trigger_task(app): def _wait_for_completion(app, task_id, timeout=3): """Block until the callback manager reports the job is no longer running.""" job_id, _ = _unwrap_handles(app, task_id) - manager = app.callback_map["output.children"]["manager"] + tool_name = task_id.split(":", 1)[0] + adapter = app.mcp_callback_map.find_by_tool_name(tool_name) + manager = app.callback_map[adapter.output_id]["manager"] deadline = time.time() + timeout while time.time() < deadline: if not manager.job_running(job_id): @@ -298,3 +300,64 @@ def test_mcpbg011_task_id_encodes_tool_name_job_id_cache_key(): assert job_id.isdigit() assert len(cache_key) == 64 # SHA256 hex assert created_epoch.isdigit() + + +def _make_no_output_background_app(): + cache = diskcache.Cache() + manager = DiskcacheManager(cache) + + app = Dash(__name__) + app.layout = html.Div( + [ + html.Div(id="trigger-no-output"), + html.Div(id="side-effect-target"), + ] + ) + + @app.callback( + Input("trigger-no-output", "children"), + background=True, + manager=manager, + ) + def no_output_slow_callback(value): + time.sleep(0.5) + + return app + + +def test_mcpbg012_no_output_task_result_does_not_raise(): + app = _make_no_output_background_app() + create_result = _mcp( + app, + "tools/call", + { + "name": "no_output_slow_callback", + "arguments": {"value": "hello"}, + "task": {"ttl": 60000}, + }, + ) + task_id = create_result["result"]["task"]["taskId"] + _wait_for_completion(app, task_id) + + result = _mcp(app, "tasks/result", {"taskId": task_id}) + assert result["result"]["structuredContent"] == {"multi": True} + + +def test_mcpbg013_cancel_no_output_task_does_not_raise(): + app = _make_no_output_background_app() + create_result = _mcp( + app, + "tools/call", + { + "name": "no_output_slow_callback", + "arguments": {"value": "hello"}, + "task": {"ttl": 60000}, + }, + ) + task_id = create_result["result"]["task"]["taskId"] + + cancel_result = _mcp(app, "tasks/cancel", {"taskId": task_id}) + assert "error" not in cancel_result + + result = _mcp(app, "tasks/result", {"taskId": task_id}) + assert result["result"]["structuredContent"] == {"multi": True}