Fix running= with an ALL wildcard crashing when no components match

CHANGELOG.md

@@ -40,6 +40,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 - [#4027](https://github.com/plotly/dash/pull/4027) Support `async def` hook routes (`dash.hooks.route`) on all backends, stop hook routes from leaking the app context to the caller, and let hook routes and MCP read the request on the `fastapi` backend.
 - [#4053](https://github.com/plotly/dash/pull/4053) Fix callbacks being registered twice under a reloader (e.g. `uvicorn --reload`) when `Dash()` is given an explicit `name` or is built by an app factory called from the main module. Fixes [#3912](https://github.com/plotly/dash/issues/3912).
 - [#3980](https://github.com/plotly/dash/pull/3980) Fix the three `before_request` hooks (`Dash._setup_server` and the pages `router_sync` / `router_async`) publishing their "already done" guard flag before the setup work behind it had run. Under a multi-threaded WSGI worker such as `gunicorn -k gthread` (or under an ASGI worker for the async router), a second request arriving mid-setup could observe the flag already set, skip setup, then read `registered_paths` / `callback_map` / the pages router callback while they were still being registered - causing the first burst of component bundle requests after a restart to 500 with `Error loading dependency. "<lib>" is not a registered library`, or the pages router to hit `DuplicateCallback` when two workers raced past the guard. Each hook body now runs under a lock (`threading.Lock` for the two sync hooks, an `asyncio.Lock` bound to the running loop for the async router) and only publishes the flag after all work completes. Fixes [#3971](https://github.com/plotly/dash/issues/3971).
+- [#3957](https://github.com/plotly/dash/pull/3957) Fix `running=` with a pattern-matching `ALL`/`ALLSMALLER` id crashing the renderer when none of the matching components are on the current page. Fixes [#3297](https://github.com/plotly/dash/issues/3297).
 - [#3944](https://github.com/plotly/dash/pull/3944) Fix `dash.testing` runner backend detection for wrapped FastAPI/Quart servers so threaded Flask-only options are not passed to ASGI runners.
 - [#3955](https://github.com/plotly/dash/pull/3955) Unpin `selenium` in the testing requirements (was capped at `<=4.2.0` from 2022) and require `>=4.11.0`, so it can drive current stable Chrome via Selenium Manager and stop the widespread CI flakiness.
 - [#3881](https://github.com/plotly/dash/pull/3881) Speed up the renderer layout crawl (`crawlLayout` and its callers) by replacing curried-ramda `path`/`pathOr` lookups with direct property access on the hot path: ~16% faster `Patch().append()` into a large container, with no behavior change.

dash/dash-renderer/src/actions/callbacks.ts

@@ -444,16 +444,25 @@ function sideUpdate(outputs: SideUpdateOutput, cb: ICallbackPayload) {
                 let componentId = id,
                     propName,
                     replacedIds = [];
+                let isPatternMatching = false;
 
                 if (id.startsWith('{')) {
                     [componentId, propName] = parsePMCId(id);
                     replacedIds = replacePMC(componentId, cb, i, getState);
+                    isPatternMatching = true;
                 } else if (id.includes('.')) {
                     [componentId, propName] = id.split('.');
                 }
 
                 const props = propName ? {[propName]: value} : value;
 
+                if (isPatternMatching && replacedIds.length === 0) {
+                    // A wildcard that matches nothing currently rendered.
+                    // There is no component to update, and `componentId` still
+                    // holds the unresolved pattern, so it must not be used.
+                    return acc;
+                }
+
                 if (replacedIds.length === 0) {
                     acc.push([componentId, props]);
                 } else if (replacedIds.length === 1) {

dash/dash-renderer/src/actions/patternMatching.ts

@@ -32,7 +32,13 @@ export function parsePMCId(id: string): [any, string | undefined] {
 export function getAllPMCIds(id: any, state: any, triggerKey: string) {
     const keysOfIds = keys(id);
     const idKey = keysOfIds.join(',');
-    return state.paths.objs[idKey]
+    // No component with this id shape is currently rendered, so nothing
+    // matches the wildcard.
+    const registered = state.paths.objs[idKey];
+    if (!registered) {
+        return [];
+    }
+    return registered
         .map((obj: any) =>
             keysOfIds.reduce((acc, key, i) => {
                 acc[key] = obj.values[i];
@@ -62,9 +68,14 @@ export function replacePMC(
     getState: any
 ): any[] {
     let extras: any = [];
+    // Whether an ALL/ALLSMALLER key was expanded. `extras` alone cannot tell
+    // us this, since a wildcard that matches no rendered component expands to
+    // an empty list -- in which case there is genuinely nothing to update, and
+    // `replaced` only holds the non-wildcard keys, so it is not a usable id.
+    let expanded = false;
     const replaced: any = {};
     toPairs(id).forEach(([key, value]) => {
-        if (extras.length) {
+        if (expanded) {
             // All done.
             return;
         }
@@ -75,16 +86,18 @@ export function replacePMC(
                 replaced[key] = triggerValue;
             } else if (value.includes('ALL')) {
                 extras = getAllPMCIds(id, getState(), key);
+                expanded = true;
             } else if (value.includes('ALLSMALLER')) {
                 extras = getAllPMCIds(id, getState(), key).filter(
                     (obj: any) => obj[key] < triggerValue
                 );
+                expanded = true;
             }
         } else {
             replaced[key] = value;
         }
     });
-    if (extras.length) {
+    if (expanded) {
         return extras;
     }
     return [replaced];

dash/dash-renderer/tests/patternMatching.test.js

@@ -0,0 +1,121 @@
+import {expect} from 'chai';
+import {beforeEach, describe, it} from 'mocha';
+import {getAllPMCIds, replacePMC} from '../src/actions/patternMatching';
+
+// Minimal stand-in for the pieces of the redux state that the pattern
+// matching helpers read.
+function makeState(objs) {
+    return {paths: {strs: {}, objs: objs || {}}};
+}
+
+// A wildcard entry as produced by crawling the layout: `values` is the list
+// of id values ordered by the (sorted) id keys.
+function entries(keyStr, valueLists) {
+    return {
+        [keyStr]: valueLists.map((values, i) => ({
+            values,
+            path: ['props', 'children', i]
+        }))
+    };
+}
+
+const cb = {parsedChangedPropsIds: [{id: 'home1', type: 'loading'}]};
+
+describe('getAllPMCIds', () => {
+    it('returns the matching ids when the wildcard key is registered', () => {
+        const state = makeState(
+            entries('id,type', [
+                ['home1', 'loading'],
+                ['home2', 'loading']
+            ])
+        );
+        const result = getAllPMCIds(
+            {id: ['ALL'], type: 'loading'},
+            state,
+            'id'
+        );
+        expect(result).to.deep.equal([
+            {id: 'home1', type: 'loading'},
+            {id: 'home2', type: 'loading'}
+        ]);
+    });
+
+    it('returns an empty list when no component uses that id shape', () => {
+        // This is the state on a page that renders none of the wildcard
+        // components: `paths.objs['id,type']` was never populated.
+        const result = getAllPMCIds(
+            {id: ['ALL'], type: 'loading'},
+            makeState({}),
+            'id'
+        );
+        expect(result).to.deep.equal([]);
+    });
+});
+
+describe('replacePMC', () => {
+    let getState;
+
+    beforeEach(() => {
+        getState = () =>
+            makeState(
+                entries('id,type', [
+                    ['home1', 'loading'],
+                    ['home2', 'loading']
+                ])
+            );
+    });
+
+    it('expands ALL to every matching component', () => {
+        const result = replacePMC(
+            {id: ['ALL'], type: 'loading'},
+            cb,
+            0,
+            getState
+        );
+        expect(result).to.deep.equal([
+            {id: 'home1', type: 'loading'},
+            {id: 'home2', type: 'loading'}
+        ]);
+    });
+
+    it('resolves MATCH against the triggering id', () => {
+        const result = replacePMC(
+            {id: ['MATCH'], type: 'loading'},
+            cb,
+            0,
+            getState
+        );
+        expect(result).to.deep.equal([{id: 'home1', type: 'loading'}]);
+    });
+
+    it('leaves a fully concrete id untouched', () => {
+        const result = replacePMC(
+            {id: 'home1', type: 'loading'},
+            cb,
+            0,
+            getState
+        );
+        expect(result).to.deep.equal([{id: 'home1', type: 'loading'}]);
+    });
+
+    it('yields no ids when ALL matches nothing on the current page', () => {
+        // Regression test for #3297: navigating to a page that has none of
+        // the wildcard components used to throw
+        // `state.paths.objs[idKey] is undefined`, and later returned a
+        // partial id missing the wildcard key.
+        const empty = () => makeState({});
+        const result = replacePMC({id: ['ALL'], type: 'loading'}, cb, 0, empty);
+        expect(result).to.deep.equal([]);
+    });
+
+    it('yields no ids when ALLSMALLER matches nothing on the current page', () => {
+        const empty = () => makeState({});
+        const result = replacePMC(
+            {id: ['ALLSMALLER'], type: 'loading'},
+            cb,
+            0,
+            empty
+        );
+        expect(result).to.deep.equal([]);
+    });
+});

tests/integration/callbacks/test_wildcards.py

@@ -631,6 +631,44 @@ def on_click(_) -> str:
         )
 
 
+def test_cbwc015_running_all_absent_page(dash_duo):
+    app = Dash(__name__)
+    app.layout = html.Div(
+        [
+            dcc.Location(id="url"),
+            dcc.Link("Other", href="/other", id="link"),
+            html.Button("Go", id="btn"),
+            html.Div(id="out"),
+            html.Div(id="page"),
+        ]
+    )
+
+    @app.callback(Output("page", "children"), Input("url", "pathname"))
+    def render_page(pathname):
+        if pathname == "/other":
+            return html.Div("other page", id="other")
+        return [html.Div(id={"type": "loading", "index": i}) for i in range(2)]
+
+    @app.callback(
+        Output("out", "children"),
+        Input("btn", "n_clicks"),
+        running=[(Output({"type": "loading", "index": ALL}, "children"), "on", "off")],
+        prevent_initial_call=True,
+    )
+    def on_click(n_clicks):
+        return f"clicked {n_clicks}"
+
+    dash_duo.start_server(app)
+    dash_duo.wait_for_element('[id*="loading"]')
+
+    dash_duo.find_element("#link").click()
+    dash_duo.wait_for_text_to_equal("#other", "other page")
+
+    dash_duo.find_element("#btn").click()
+    dash_duo.wait_for_text_to_equal("#out", "clicked 1")
+    assert dash_duo.get_logs() == []
+
+
 def test_cbwc009_match_input_fixed_output(dash_duo):
     # Issue #2462: allow MATCH in Input with a fixed-id Output.
     app = Dash(__name__)