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

Warn if ui.open is used before client is connected #1309

Merged
merged 2 commits into from
Aug 2, 2023

Conversation

rodja
Copy link
Member

@rodja rodja commented Aug 2, 2023

This PR informs the user of a subtle problem which was observed by @Anindya088 in #1289 (comment)

@rodja rodja added the enhancement New feature or request label Aug 2, 2023
@rodja rodja added this to the 1.3.7 milestone Aug 2, 2023
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.

Ok, I just changed the code to use the global logger (which we don't use very consistently by the way).
I wonder if we should also recommend await client.connected() in the error message.

@rodja
Copy link
Member Author

rodja commented Aug 2, 2023

I wonder if we should also recommend await client.connected() in the error message.

Well... it does not really make sense to await the connection in my opinion. Because if you know that you want to open another page before the page is served you can use a redirect. There may be a short time frame between serving the page and the established web-connection where it would make a difference... but that can be ignored in my opinion.

@falkoschindler falkoschindler merged commit b1b30ca into main Aug 2, 2023
5 checks passed
@falkoschindler falkoschindler deleted the open_before_connected branch August 2, 2023 15:20
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.

2 participants