[Assist] Clear the refresh websocket timeout when closing Assist#28610
Merged
[Assist] Clear the refresh websocket timeout when closing Assist#28610
Conversation
ryanclark
changed the title
jakule
approved these changes
Jul 3, 2023
| loadConversations(); | ||
|
|
||
| return () => { | ||
| window.clearTimeout(refreshWebSocketTimeout.current); |
Contributor
There was a problem hiding this comment.
Can you add that this can be removed after https://github.com/gravitational/teleport.e/pull/1609 is implemented?
kimlisa
approved these changes
Jul 3, 2023
public-teleport-github-review-bot
Bot
removed the request for review
from ravicious
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Jul 14, 2023
* [Assist] Clear the refresh websocket timeout when closing Assist (#28610) * Clear the refresh websocket timeout when closing Assist * Missing semicolon to please prettier * Add comment to remove once the new session implementation is done * fix: Decode JSON directly from stream in `waitForCommandOutput` (#28040) * Assist: mark individual nodes as finished (#28477) * Mark nodes as done when command finishes in Assist * Split Close/CloseWithPayload * Expect a close message before the summary in test * Remediate confusing type usage in test `Envelope` is the outer layer used (in protobuf format) for execution / terminal sessions. Meanwhile `outEnvelope` is the inner layer used (in JSON) spefically with assist (execution), when outer envelope is of "raw" type Using `Envelope` where decoding `outEnvelope` in tests previously worked "by accident" due to matching field names * Assert that server ID is set in the close message * Refactor command execution logic and adjust WebSocket handling This commit changes how command execution and WebSocket handling are performed in the code. Instead of manually managing session close signals and command execution notifications via WebSocket, we have wrapped it into more easily manageable form. Changed parts: assist and websocket libraries, test and several components of 'teleport'. Why: This allows cleaner command management and error handling, leading to more reliable and maintainable application. Also made code more readable and easy to understand. Made WebSocket more precise with its handling to prevent cases where stale or incorrect data might interrupt the session. Details: Instead of sending a close signal after all commands have been executed, we now send individual session end updates for each command. Sessions no longer remain active unnecessarily. Message handling in command execution has been refactored for better error propagation. All these changes were also adjusted and reflected in the associated test cases. Also, names and types of various data structures are edited to reflect their actual usage. * Change ServerID to NodeID to improve code consistency This commit replaces occurrences of `ServerID` with `NodeID` across several files. `ServerID` was misleading and causing confusion as it was serving as an identifier for nodes, so to improve comprehension and consistency in the codebase, all instances of `ServerID` have been replaced with `NodeID`. Tests have also been updated to reflect this change. --------- Co-authored-by: Jakub Nyckowski <jakub.nyckowski@goteleport.com> --------- Co-authored-by: Ryan Clark <ryan.clark@goteleport.com> Co-authored-by: Alan Parra <alan.parra@goteleport.com> Co-authored-by: Justinas Stankevičius <justinas@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, after opening Assist & selecting a conversation/creating a conversation, the websocket would still refresh after 8 minutes even if Assist has been closed. This PR clears the timeout when the
AssistContextis unmounted.