Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve recovery after reconnecting #1762

Merged
merged 6 commits into from
Oct 12, 2023
Merged

Conversation

rodja
Copy link
Member

@rodja rodja commented Oct 7, 2023

This pull request tries to implement #1761.

@rodja rodja added the enhancement New feature or request label Oct 7, 2023
@rodja rodja added this to the 1.3.17 milestone Oct 7, 2023
@falkoschindler falkoschindler marked this pull request as ready for review October 12, 2023 18:06
Copy link
Contributor

@falkoschindler falkoschindler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rodja I just tested the implementation with a simple test app:

from nicegui import ui

ui.input('Hello world!')
ui.button('Click me!', on_click=lambda: ui.notify('Button clicked!'))

ui.run(reconnect_timeout=10.0)

When I type something into the input field, disconnect the connection via developer tools > network > throttling, wait for the connection popup and reconnect, the input is preserved. But the same happens for reconnect_timeout = 0. Shouldn't this trigger a reload?

Copy link
Contributor

@falkoschindler falkoschindler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, my bad: reconnect_timeout has no effect on the auto-index client. 🤦🏻‍♂️

@falkoschindler falkoschindler merged commit eea7d8d into main Oct 12, 2023
6 checks passed
@falkoschindler falkoschindler deleted the recover_from_reconnect branch October 12, 2023 18:14
@falkoschindler
Copy link
Contributor

@rodja I just got aware of a possibly inconvenient behavior when using reconnect_timeout = 3 (the new default in version 1.4):

@ui.page('/test')
async def page(client: Client):
    ui.label('Hello, world!')
    await client.disconnected()
    print('disconnected', flush=True)

The client.disconnected will only return after 3 seconds. Although this is correct and consistent, it might lead to unexpected delays when transitioning to version 1.4.

@rodja
Copy link
Member Author

rodja commented Oct 13, 2023

@falkoschindler yes, that's why I chose to use 0 for 1.3. But allowing clients to reconnect without page reload is super important and I would gladly trade the delayed client.disconnected() for the benefit of reducing state loss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants