feat: Add web terminal with reconnecting TTYs #1186
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1186 +/- ##
==========================================
+ Coverage 65.65% 65.80% +0.14%
==========================================
Files 269 272 +3
Lines 17366 17872 +506
Branches 164 192 +28
==========================================
+ Hits 11402 11760 +358
- Misses 4772 4882 +110
- Partials 1192 1230 +38
Continue to review full report at Codecov.
|
47e0e98 to
6e1ac65
Compare
This adds a web terminal that can reconnect to resume sessions! No more disconnects, and no more bad bufferring!
| "/api": { | ||
| target: "http://localhost:3000", | ||
| ws: true, | ||
| }, |
There was a problem hiding this comment.
It's much fancier now! ✨
cf9d2a3 to
8cc14c1
Compare
|
Have you considered instead of capturing scroll back just reconnecting the tty to the new websocket and forcing the shell or ncurses app to refresh by sending a resize signal? Would be way simpler, and we can show the reconnection similar to how iterm or vscode does it |
|
This actually is how VS Code does it! |
|
@deansheather do you feel like this is complex right now? We do just store a reconnection token, much like VS Code. |
|
@kylecarbs my suggestion is to not use a scroll back buffer at all, but when they reconnect the client would keep the existing history it has locally and just write a "Restored terminal session" message to the terminal, then force a screen refresh for interactive apps (like the shell and vim etc.) using sigwinch. We would not need to worry about scrollback buffers which I think are weird and complex anyways |
|
How would that work if data occurred during disconnect? Or if they refresh the window? The current case handles both seamlessly, and could even allow terminal sharing just with a link. |
|
@kylecarbs my suggestion is how vscode does it I think. Obviously having a scrollback is nicer, but I think it adds to much complexity and is prone to weird terminal bugs rather than a simpler and more robust solution. Terminal sharing is still possible but when new clients connect they won't have scrollback which I think is fine. |
|
VS Code has a buffer like the current implementation, but maybe they also do what you mention. What bugs do you foresee? Have you tried it out? |
|
We can add scrollback later if customers request it, but for most cases users just don't want to do lose their shell history or running processes or vim session etc and the output (as long as we do the refresh thing) isn't a huge issue IMO |
0fd3e02 to
b1f3b0e
Compare
daf4ece to
8212287
Compare
There was a problem hiding this comment.
Niiiiice!!
One thing I thought would be nice is an automatic reconnection or a longer timeout because I will have a disconnect and not notice for a while but by then it is too late to reconnect (no need to do that in this PR though IMO).
Do you get disconnected a lot or is it just me? That disconnect overlay keeps coming up for me (one to two minutes). I doubt it is something in this PR, just curious if this is normal. only happens when using the provided tunneled URL; might be something to look at eventually though
| return render( | ||
| <Routes> | ||
| <Route path="/:username/:workspace/terminal" element={<TerminalPage renderer="dom" />} /> | ||
| </Routes>, |
There was a problem hiding this comment.
Ooh, curious why you put Routes in here, it might help me with some pain points I've had
There was a problem hiding this comment.
It said it was required for me to use <Route /> :(
|
@presleyp I appreciate the thorough review! |
bd6dc51 to
7d9c1b8
Compare
|
I'm going to merge, but @deansheather please feel free to follow up if my message doesn't resolve your concerns! It appears to be a solid web-terminal as-is, but if we can make it even better I'm of course happy to do so! 😄🪄 |
* feat: Add web terminal with reconnecting TTYs This adds a web terminal that can reconnect to resume sessions! No more disconnects, and no more bad bufferring! * Add xstate service * Add the webpage for accessing a web terminal * Add terminal page tests * Use Ticker instead of Timer * Active Windows mode on Windows
This adds a web terminal that can reconnect to resume sessions!
No more disconnects, and no more bad buffering!
It uses a circular buffer under the hood to constantly buffer ~64KB
of terminal scrollback data.
Todo:
Allow agents in "connecting" state to "Wait for connection...", just like the CLI.This doesn't seem necessary for merge. Customers will get an error that the workspace is offline anyways, which is a pretty obvious indicator.There isn't currently a direct link from the dashboard, but you can try it out by:
make bin && go run -tags embed ./cmd/coder server --devhttps://localhost:3000/<username>/<workspace>/terminal.reconnecting-terminal-2022-04-29_09.06.33.mp4