Skip to content

[JetBrains] improve handling of client connection termination #20288

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Oct 11, 2024
Merged

[JetBrains] improve handling of client connection termination #20288

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d941f69
[JetBrains] improve handling of client connection termination
mustard-mh Oct 10, 2024
6c1488e
Hold connection provider until workspace stopped
mustard-mh Oct 11, 2024
8a3047a
use lifetime launch
mustard-mh Oct 11, 2024
6cf5a2f
delay 5 seconds
mustard-mh Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 49 additions & 3 deletions ...ns/gateway-plugin/src/main/kotlin/io/gitpod/jetbrains/gateway/GitpodConnectionProvider.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,13 @@ import com.jetbrains.rd.util.ConcurrentHashMap
import com.jetbrains.rd.util.URI
import com.jetbrains.rd.util.lifetime.Lifetime
import com.jetbrains.rd.util.lifetime.LifetimeDefinition
import com.jetbrains.rd.util.threading.coroutines.launch
import io.gitpod.gitpodprotocol.api.entities.WorkspaceInstance
import io.gitpod.jetbrains.gateway.common.GitpodConnectionHandleFactory
import io.gitpod.jetbrains.icons.GitpodIcons
import kotlinx.coroutines.*
import kotlinx.coroutines.future.await
import java.awt.Component
import java.net.URL
import java.net.http.HttpClient
import java.net.http.HttpRequest
Expand All @@ -71,6 +73,17 @@ class GitpodConnectionProvider : GatewayConnectionProvider {
private val jacksonMapper = jacksonObjectMapper()
.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)


private fun showTimedOutDialogDialog(workspaceId: String, detail: String?) {
val title = "Workspace Timed Out"
val message = "Your workspace $workspaceId has timed out${if (detail.isNullOrBlank()) "" else " : $detail"}."
val okButton = Messages.getOkButton()
val options = arrayOf(okButton)
val defaultIndex = 0
val icon = Messages.getInformationIcon()
Messages.showDialog(message, title, options, defaultIndex, icon)
}

override suspend fun connect(
parameters: Map<String, String>,
requestor: ConnectionRequestor
Expand Down Expand Up @@ -188,6 +201,7 @@ class GitpodConnectionProvider : GatewayConnectionProvider {
var thinClientJob: Job? = null

var lastUpdate: WorkspaceInstance? = null
var canceledByGitpod = false
try {
for (update in updates) {
try {
Expand Down Expand Up @@ -255,8 +269,8 @@ class GitpodConnectionProvider : GatewayConnectionProvider {
statusMessage.text = ""
}
}

if (update.status.phase == "stopping" || update.status.phase == "stopped") {
canceledByGitpod = true
thinClientJob?.cancel()
thinClient?.close()
}
Expand Down Expand Up @@ -295,9 +309,41 @@ class GitpodConnectionProvider : GatewayConnectionProvider {
SshHostTunnelConnector(credentials),
URI(joinLinkResp.joinLink)
)
var triggeredClientClosed = false
clientHandle.clientClosed.advise(connectionLifetime) {
application.invokeLater {
connectionLifetime.terminate()
// Been canceled by user
if (!canceledByGitpod) {
connectionLifetime.launch {
// Delay for 5 seconds to see if thinClient could be terminated in time
// Then we don't see error dialog from Gateway
delay(5000)
Copy link
Member

@akosyakov akosyakov Oct 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do we know 5secs is enough, is there a triggere to which we can reliable bind instead?

Copy link
Contributor Author

@mustard-mh mustard-mh Oct 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think thinClient will block for 5 more seconds (unless UI freeze maybe?). From java Doc, there's no other public listeners available, I found clientProcessTerminated but it's private

image

application.invokeLater {
connectionLifetime.terminate()
}
}
return@advise
}
if (triggeredClientClosed) {
return@advise
}
triggeredClientClosed = true
// Wait until workspace is stopped
suspend fun waitUntilStopped(): Boolean {
while (lastUpdate.status.phase != "stopped") {
delay(1000)
}
return true
}
// Check if it's timed out, if so, show timed out dialog
connectionLifetime.launch {
val isInStoppedPhase = waitUntilStopped()
val isTimedOut = isInStoppedPhase && phaseMessage.text == "Timed Out"
application.invokeLater {
if (isTimedOut) {
showTimedOutDialogDialog(connectParams.resolvedWorkspaceId, lastUpdate.status.conditions.timeout)
}
connectionLifetime.terminate()
}
}
}
clientHandle.onClientPresenceChanged.advise(connectionLifetime) {
Expand Down
Loading