feat: auto reconnect the terminal #18796
Conversation
|
@code-asher I'm wondering if we can stop adding error messages to the terminal since we already have the alert. Wdyt? 
|
Makes perfect sense to me! |
There was a problem hiding this comment.
Looks promising! I tried it out and it seems to reconnect but then I was unable to see anything show up in the terminal. Is that the same for you? I can see messages being sent and received on the web socket, so it seems like we are just not printing to the terminal anymore. Maybe we have to rebind something? Or maybe the terminal is being recreated or something.
| @@ -259,9 +266,11 @@ const TerminalPage: FC = () => { | |||
| if (disposed) { | |||
| return; // Unmounted while we waited for the async call. | |||
| } | |||
| websocket = new WebSocket(url); | |||
| websocket = new WebsocketBuilder(url) | |||
| .withBackoff(new ConstantBackoff(1000)) | |||
There was a problem hiding this comment.
I think we should switch this to exponential (ideally with some jitter) soon, if not in this PR. A fixed reconnect interval is a good way to DDoS yourself 😆
|
Ah actually it seems the data is a blob after reconnect instead of a array buffer? Does Also we should probably send the size on reconnect in case the user resized the terminal while offline. |
|
@code-asher fixed the issues you pointed out. I tested using the following steps:
Do you know if this is a good recipe for testing these changes? Let me know if you think I should test this differently. |
There was a problem hiding this comment.
It works! I wish their backoff implementation had jitter, but what can ya do. We can add our own backoff implementation later if necessary.
Put network on offline using the dev console (in network tab)
My experience has been that this setting does not affect existing connections (like web sockets). I tried just now to be sure (in Chromium) but the "trying to connect" message never appears and I can still use the terminal even when "offline".
What I have been doing is killing coderd, waiting for the reconnect message, then starting coderd again.
Changes:
useWithRetrybecause it is not necessary anymoreOther topics:
Closes coder/internal#659