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

[BasicUI] Live update is fully unstable with OH 4.1.1 and Firefox 121.0.1 #1757

Open
1 of 4 tasks
lolodomo opened this issue Mar 5, 2023 · 22 comments · Fixed by openhab/openhab-distro#1476
Open
1 of 4 tasks
Labels
basic ui Basic UI bug Something isn't working

Comments

@lolodomo
Copy link
Contributor

lolodomo commented Mar 5, 2023

  • Basic UI
  • HABPanel
  • HABot
  • CometVisu

The problem

It looks like BasicUI is no more updating the states in its current page.

Additional information

I don't think this is related to the other major recent changes (Karaf version, Jetty version, ...) because it was still working as expected for me yesterday in the morning.
I suddenly encountered the problem yesterday afternoon.

It might be something I broke in one of my changes yesterday. I am sorry for that and I will try to find where is the problem.

@lolodomo lolodomo added bug Something isn't working basic ui Basic UI labels Mar 5, 2023
@lolodomo
Copy link
Contributor Author

lolodomo commented Mar 5, 2023

I am starting to have a serious doubt that it is due to my changes because I have undone all my changed in smarthome.js and the problem persists.
I am going to reinstall the snapshot 3340 (before all my UI changes).

@lolodomo
Copy link
Contributor Author

lolodomo commented Mar 5, 2023

With snapshot 3340 or snapshot 3340 + updated jupp bundle (2.7.0), the live update is apparently working.
This confirms that the problem is in changes of yesterday.
In addition of my own changes in BasicUI, I also see this change: openhab/openhab-distro#1460
Is there any chance that this change to impact communication between OH server and BaiscUI ?
@wborn @J-N-K ?

@J-N-K
Copy link
Member

J-N-K commented Mar 5, 2023

You can easily check, shutdown, edit the jetty.xml and start again. If it works, it‘s related, if it doesn’t, then it‘s another issue.

If it does work, then please try to add an exclude similar to the existing include for /rest/events and see if that improves the situation.

@lolodomo
Copy link
Contributor Author

lolodomo commented Mar 5, 2023

Interesting, if I install the last BasicUI version in snapshot 3340, the live update is still working.

@J-N-K
Copy link
Member

J-N-K commented Mar 5, 2023

So if you install BasicUI from 3340 in a recent snapshot it works, if you use the BasicUI coming with the distribution it does not work? Then it's most likely not related to the GZIP change.

@lolodomo
Copy link
Contributor Author

lolodomo commented Mar 5, 2023

If I remove your change in jetty.xml with a fresh snapshot 3344, it looks like live update is back.

@lolodomo lolodomo changed the title [BasicUI] Live update is broken ! [BasicUI] Live update is broken in snapshot 3344 ! Mar 5, 2023
@lolodomo
Copy link
Contributor Author

lolodomo commented Mar 5, 2023

Closed by openhab/openhab-distro#1476

@lolodomo lolodomo closed this as completed Mar 5, 2023
@lolodomo
Copy link
Contributor Author

I reopen this issue because it happens again in 4.1.1 !
@J-N-K : was there another change ? Or was your fix not fixing the root cause of the problem ?

@lolodomo lolodomo reopened this Jan 20, 2024
@J-N-K
Copy link
Member

J-N-K commented Jan 20, 2024

With http or https connections? Safari doesn't support SSE on http connections since a long time, maybe other browser catch up there.

@lolodomo
Copy link
Contributor Author

No sure I reopened the correct issue, the current problem is that Baisc UI is regularly loosing its SSE connection.
I try to find if I have created another issue because I am certain I already encountered this problem during the last months.

@lolodomo
Copy link
Contributor Author

lolodomo commented Jan 20, 2024

With http or https connections? Safari doesn't support SSE on http connections since a long time, maybe other browser catch up there.

HTTP on Windows 10. The problem is that the SSE connection is disconnected after a certain time, maybe one minute or something like that. I see the banner in UI telling me that the UI is disconnected.

@J-N-K
Copy link
Member

J-N-K commented Jan 20, 2024

The something else changed, the Jetty configuration did not.

@lolodomo
Copy link
Contributor Author

My jetty.xml contains the lines you added in your fix.

This bug is really critical, the UI is no more refreshed until you change page because the SSE connection is closed.

@lolodomo
Copy link
Contributor Author

lolodomo commented Jan 20, 2024

The banner "disconnected" appears at bottom of UI and disappears after few seconds but at this time I believe the SSE connection is closed because item updates are not updating the page.
I see no particular logs in server. Any idea what DEBUG logs I should enable ?

Edit to myself: org.openhab.core.io.rest.sitemap

@lolodomo
Copy link
Contributor Author

lolodomo commented Jan 20, 2024

I do not understand the root cause, that is why the SSE connection is now regularly broken.

But when the UI (Baisc UI) detects the closed connection, it opens a new SSE connection. But this new connection is apparently monitoring the main page and not the currently opened (sub) page. It explains why the page content is not refreshed.

11:31:54.114 [DEBUG] [st.sitemap.SitemapSubscriptionService] - Created new subscription with id d85258ef-2b72-452a-9f0c-60d4c263e7be (2 active subscriptions for a max of 50)
11:31:54.119 [DEBUG] [rest.sitemap.internal.SitemapResource] - Client from IP 192.168.xxx.xxx requested new subscription => got id d85258ef-2b72-452a-9f0c-60d4c263e7be.
11:31:54.157 [DEBUG] [st.sitemap.SitemapSubscriptionService] - Subscription d85258ef-2b72-452a-9f0c-60d4c263e7be changed to page maison of sitemap maison (2 active subscriptions}
11:31:54.161 [DEBUG] [rest.sitemap.internal.SitemapResource] - Client from IP 192.168.xxx.xxx requested sitemap event stream for subscription d85258ef-2b72-452a-9f0c-60d4c263e7be.
11:31:55.442 [DEBUG] [st.sitemap.SitemapSubscriptionService] - Subscription d85258ef-2b72-452a-9f0c-60d4c263e7be changed to page 0001 of sitemap maison (2 active subscriptions}

comment: the SSE connection is broken

11:32:45.723 [DEBUG] [st.sitemap.SitemapSubscriptionService] - Created new subscription with id a29fdd0b-7363-4397-9fa8-d86bc4a9d197 (3 active subscriptions for a max of 50)
11:32:45.729 [DEBUG] [rest.sitemap.internal.SitemapResource] - Client from IP 192.168.xxx.xxx requested new subscription => got id a29fdd0b-7363-4397-9fa8-d86bc4a9d197.
11:32:45.762 [DEBUG] [st.sitemap.SitemapSubscriptionService] - Subscription d85258ef-2b72-452a-9f0c-60d4c263e7be changed to page 0001 of sitemap maison (3 active subscriptions}
11:32:45.844 [DEBUG] [st.sitemap.SitemapSubscriptionService] - Subscription a29fdd0b-7363-4397-9fa8-d86bc4a9d197 changed to page maison of sitemap maison (3 active subscriptions}

Comment: new subscription points to the root page and not the sub page

11:32:45.848 [DEBUG] [rest.sitemap.internal.SitemapResource] - Client from IP 192.168.xxx.xxx requested sitemap event stream for subscription a29fdd0b-7363-4397-9fa8-d86bc4a9d197.

11:32:57.647 [INFO ] [openhab.event.ItemStateChangedEvent  ] - Item 'Date' changed from 2024-01-20T11:31:57.642+0100 to 2024-01-20T11:32:57.643+0100
11:32:57.658 [DEBUG] [rest.sitemap.internal.SitemapResource] - Sent sitemap event for widget 0000 to subscription a29fdd0b-7363-4397-9fa8-d86bc4a9d197.
11:32:58.519 [DEBUG] [rest.sitemap.internal.SitemapResource] - Sent sitemap event for widget 0004 to subscription a29fdd0b-7363-4397-9fa8-d86bc4a9d197.
11:32:58.527 [DEBUG] [rest.sitemap.internal.SitemapResource] - Sent sitemap event for widget 0005 to subscription a29fdd0b-7363-4397-9fa8-d86bc4a9d197.

Comment: SSE events are received for widgets in the main page

So there are 2 problems:

  1. the SSE connection is closed regurlarly for an unexplained reason
  2. Basic UI detects a closed SSE connection and reopens a new one but on the wrong page

I have no idea how to fix the first issue but I can certainly try to fix issue 2.

@wborn
Copy link
Member

wborn commented Jan 20, 2024

Did you also check if it sometimes exceeds the max 6 connections per domain limit of your browser?

@lolodomo
Copy link
Contributor Author

Did you also check if it sometimes exceeds the max 6 connections per domain limit of your browser?

How can I check that ?

That is true that I have MainUI + BasisUI opened in browser. I hope this is not MainUI opening too many SSE connections.
But I tried to close MainUI just after opening BasiucUI and I encounter the same problem.

@lolodomo lolodomo changed the title [BasicUI] Live update is broken in snapshot 3344 ! [BasicUI] Live update is broken in snapshot 3344 and in final 4.1.1 ! Jan 20, 2024
@lolodomo
Copy link
Contributor Author

Regarding the reconnection issue, there is clearly a call by BaiscUI to the REST API sitemap/events/<newSubscriptionID> with the wrong page (the main page). All this is done in the JavaScript part (smarthome.js) but the code is so complex that it is hard to understand where is the problem. I also can deduce from server logs that BaiscUI just before this REST call reloads the current page but still with the old subscription. So the old subscription is updated (linked page) rather than the new subscription. That is probably the reason of this reconnection bug.

@lolodomo
Copy link
Contributor Author

lolodomo commented Jan 20, 2024

Regarding the closed SSE connections, it seems that it does not occur when I use Chrome, the SSE connection is then stable. So it may be a new problem specific to a recent version of Firefox.

Edit: the SSE connection is also stable with Microsoft Edge.

After a quick WEB search, it may be that Firefox expects a proper SSE close at the right moment.

@lolodomo
Copy link
Contributor Author

lolodomo commented Jan 20, 2024

To summarize, the SSE connection is now fully unstable when using recent version of Firefox while stable with Chrome/Edge.
This problem with Firefox makes me discover a bug in the reconnection mechanism implemented in Basic UI, the reconnected SSE is monitoring events for the main page instead of the currently displayed page leading to no live update of the current page.

@lolodomo lolodomo changed the title [BasicUI] Live update is broken in snapshot 3344 and in final 4.1.1 ! [BasicUI] Live update is fully unstable with OH 4.1.1 and Firefox 121.0.1 Jan 20, 2024
lolodomo added a commit to lolodomo/openhab-webui that referenced this issue Jan 20, 2024
After an error in the SSE connection, the event source is now reconnected to the right sitemap page.

Fix one of both problems explained in openhab#1757

Signed-off-by: Laurent Garnier <[email protected]>
@lolodomo
Copy link
Contributor Author

After all these years, I am still discovering how is working certain parts of Basic UI !
The SSE reconnection is now fixed and works whatever is your current page when the SSE connection breaks..

kaikreuzer pushed a commit that referenced this issue Jan 20, 2024
After an error in the SSE connection, the event source is now
reconnected to the right sitemap page.

Fix one of both problems explained in #1757

Signed-off-by: Laurent Garnier <[email protected]>
@lolodomo
Copy link
Contributor Author

lolodomo commented Jan 21, 2024

Regarding the handling of the EventSource, when an error is detected, the EventSource is already closed bu our code before a new one is opened.
One improvement would be to close the EventSource when leaving the page.
But this will not fix the current problem with Firefox: why Firefox detects an error on the EventSource ? I am asking myself if we need to send an ALIVE event more frequently (currently every 2 minutes), just an idea.

lolodomo added a commit that referenced this issue Jan 21, 2024
After an error in the SSE connection, the event source is now
reconnected to the right sitemap page.

Fix one of both problems explained in #1757

Signed-off-by: Laurent Garnier <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
basic ui Basic UI bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants