Still seeing `KeyError: 'XXX.....'` after 2.24.0 bug fix

[alydersen]

First Check

• I added a very descriptive title here.
• This is not a Q&A. I am sure something is wrong with NiceGUI or its documentation.
• I used the GitHub search to find a similar issue and came up empty.

Example Code

Description

In #5090 the issue about KeyError was reported. After applying the latest version 2.24.0 I'm still seeing these issues in the log:

'd7cbb5c1-e428-4050-a20f-94c4a0f8b49b'
Traceback (most recent call last):
  File ".../.venv/lib/python3.13/site-packages/nicegui/background_tasks.py", line 77, in _handle_task_result
    task.result()
    ~~~~~~~~~~~^^
  File ".../.venv/lib/python3.13/site-packages/nicegui/client.py", line 292, in delete_content
    self.delete()
    ~~~~~~~~~~~^^
  File ".../.venv/lib/python3.13/site-packages/nicegui/client.py", line 356, in delete
    del Client.instances[self.id]
        ~~~~~~~~~~~~~~~~^^^^^^^^^
KeyError: 'd7cbb5c1-e428-4050-a20f-94c4a0f8b49b'

NiceGUI Version

2.24.0

Python Version

3.13.3

Browser

Other

Operating System

macOS

Additional Context

No response

[evnchn]

Related to #5090

[rodja]

Oh, while investigating #5115 and prerender behaviour of Chrome, I discovered that I can reproduce the KeyError with this code:

from nicegui import ui


def root():
    ui.add_head_html('''<script type="speculationrules">
        {
            "prerender": [
                {"source":"list","urls":["/test"],"eagerness":"eager","tag":"t1"}
            ]
        }
        </script>''')
    ui.link('test', '/test')


@ui.page('/test')
async def test():
    await ui.context.client.connected()
    print('connected')


ui.run(root)

@alydersen are you using await ui.context.client.connected()?

[alydersen]

await ui.context.client.connected()

@rodja I'm not specifically using that, but I do have a lot of async functions and ui.page()

[evnchn]

Trying to backport the MRE to 2.x since the issue is with regards to 2.24.0:

from nicegui import ui


@ui.page('/')
def main_page():
    ui.add_head_html('''<script type="speculationrules">
        {
            "prerender": [
                {"source":"list","urls":["/test"],"eagerness":"eager","tag":"t1"}
            ]
        }
        </script>''')
    ui.link('test', '/test')


@ui.page('/test')
async def test():
    await ui.context.client.connected()
    print('connected')


ui.run()

No results... No such error... @rodja Can you try as well? Given the asyncio I suspect it may be race-condition-like (or depends on CPU processing speed of certain threads)

We do have TimeoutError: No connection after 3.0 seconds though, so the hint is that maybe this has to do with graceful handling of clients who are created in vain due to no browser JS websocket connection.

[evnchn]

I think I got why the error occurs now. It is a tricky one.

First off, indeed in #5090, Client.delete() called in 2 instances:

• delete_content: no changes since 7 months ago
• prune_instances: changed in 2.23.1, then changed again in 2.24.0

Note, however, that as early as #1826 we already have a similar issue, so the recent changes of prune_instances into app.timer in #5074 and the fix thereafter in #5119, can be argued that it is unrelated to the core of the issue.

Then we have no choice but to look at delete_content. When is it called? It is called inside handle_disconnect. Hmm...

Big question: What happens when, in the time gap between the enumeration of stale_clients and the actual deletion of clients in for client in stale_clients: client.delete() that a client is disconnected? Then we call .delete() twice!

Then, there are two outcomes:

1. Try and use asyncio lock to ensure only one thing, handling client disconnect OR purging of stale client can occur, never both.
2. Use if self.id in Client.instances: for a Client.delete() which can be called twice, as suggested in Lots of KeyError after Redis connection close fix #5090 (comment) and KeyError when removing a disconnected client #1826 (comment)
3. Implement Client.ensure_deleted(), if you want to keep Client.delete() to have the same behaviour (though I not sure why that's helpful...)

[falkoschindler]

I know I'm late to the party, but:

• As far as I can tell, we still don't have a reliable reproduction. On my machine @rodja's example from Still seeing KeyError: 'XXX.....' after 2.24.0 bug fix #5119 (comment) causes timeout errors in page.py when run on the 3.0 branch, but no key error.
• I think @evnchn's assumption that a client is disconnected "in the time gap between the enumeration of stale_clients and the actual deletion of clients in for client in stale_clients: client.delete()" is false. The list stale_clients is produced and consumed synchronously, so no other async task can do anything in between.
• PR Let Client.delete() do not cause KeyError if called multiple times #5128 basically silences the KeyError without resolving or explaining the underlying problem. I mean, if we are sure that the exception is expected and needs to be silenced, fine. But the PR itself doesn't proof its validity.
• There's a potential risk in silently catching the exception (or conditionally skipping the del statement) without understanding the root cause: Imagine delete isn't called twice, but recursively for whatever reason. Then breaking the loop by raising an exception might be better than being caught in an endless loop calling delete again and again.

Therefore I think we need to re-open this issue, potentially revert PR #5128 and continue investigating this bug. 😕