Fix halfopen websocket #18934
Fix halfopen websocket #18934
Conversation
|
Is this the right place to implement this change? |
src/state/connection-mixin.ts
Outdated
| @@ -275,6 +276,23 @@ export const connectionMixin = <T extends Constructor<HassBaseEl>>( | |||
| subscribeFrontendUserData(conn, "core", (userData) => | |||
| this._updateHass({ userData }) | |||
| ); | |||
|
|
|||
| const pingBackend = async () => { | |||
There was a problem hiding this comment.
Should i create a seperate function for this? And can i rely on connect is just called once, otherwise i might end up with muliple pings being scheduled at the same time.
src/state/connection-mixin.ts
Outdated
| this.hass?.connection.reconnect(true); | ||
| }); | ||
| } | ||
| setTimeout(pingBackend, 5000); |
There was a problem hiding this comment.
Should the interval be higher? In this case the function can probably just be scheduled with setInterval
There was a problem hiding this comment.
To know if the interval should be higher we really need to test or know when the issue occurs...
There was a problem hiding this comment.
My own experience with the bug is that a few minutes are enough to trigger
This should probably be done in https://github.com/home-assistant/home-assistant-js-websocket |
That already got rejected... |
|
Have you understood the circumstances that let this happen? I have seen it occasionally but not commonly. |
From my understanding the cause for this are bugs in the implementation of web sockets in browers. In the issue if have linked, i was able to reproduce this on an iPad, but people have also reported the same problem on other platforms. So there are probably multiple circumstances that can cause this. |
|
@bramkragten i can see you assigned the issue to yourself. Are there still some information/context you are missing that i can help with? |
leonardmgh
force-pushed
the
fix-halfopen-websocket
branch
from
f5bff44
to
d1f9ae4
Compare
WalkthroughWalkthroughThe changes in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant WebSocket
User->>App: Initialize Application
App->>WebSocket: Establish Connection
WebSocket-->>App: Connection Established
loop Every Interval
App->>WebSocket: Send Ping
WebSocket-->>App: Pong Response
end
Note over App,WebSocket: If no Pong Response
App->>WebSocket: Attempt Reconnect
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
leonardmgh
force-pushed
the
fix-halfopen-websocket
branch
from
d1f9ae4
to
a18d142
Compare
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Breaking change
Proposed change
Implement ping from frontend to backend to detect a broken websocket connection that doesn't trigger a disconnect event. Currently there is no mechanism to detect this and the frontend doesn't indicate that the shown values can be trusted.
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed:
Summary by CodeRabbit