Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## Unreleased

### Fixed

- snackbar dismissal no longer cancels the calling coroutine, so error popups during URI handling don't leave the page
stuck in a busy state

### Changed

- skip the Coder TLS alternate hostname when fetching IDE metadata from JetBrains
Expand Down
24 changes: 13 additions & 11 deletions src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt
Original file line number Diff line number Diff line change
Expand Up @@ -360,15 +360,18 @@ class CoderRemoteProvider(
context.logger.info("Handling $uri...")
val newUrl = resolveDeploymentUrl(params)?.toURL() ?: return
val newToken = if (context.settingsStore.requiresMTlsAuth) null else resolveToken(params) ?: return
coderHeaderPage.isBusy.update { true }
if (sameUrl(newUrl, client?.url)) {
if (context.settingsStore.requiresTokenAuth) {
newToken?.let {
refreshSession(newUrl, it)
coderHeaderPage.isBusy.update { true }
try {
if (context.settingsStore.requiresTokenAuth) {
newToken?.let {
refreshSession(newUrl, it)
}
}
linkHandler.handle(params, newUrl, this.client!!, this.cli!!)
} finally {
coderHeaderPage.isBusy.update { false }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nbd at all but I think these changes make the coderHeaderPage.isBusy.update { false } in the parent catch unnecessary

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good point. Thx. Fixed

}
linkHandler.handle(params, newUrl, this.client!!, this.cli!!)
coderHeaderPage.isBusy.update { false }
} else {
// Different URL - we need a new connection. Tear down any
// in-flight wizard, install a fresh one on the router, and let
Expand All @@ -378,10 +381,7 @@ class CoderRemoteProvider(
context, settingsPage, visibilityState,
url = newUrl,
credentials = credentials,
onConnect = onConnect.andThen(deferredLinkHandler(params, newUrl))
.andThen { _, _ ->
coderHeaderPage.isBusy.update { false }
},
onConnect = onConnect.andThen(deferredLinkHandler(params, newUrl)),
onTokenRefreshed = ::onTokenRefreshed,
)
router.navigate(wizard)
Expand All @@ -397,7 +397,6 @@ class CoderRemoteProvider(
"Error encountered while handling Coder URI",
textError ?: ""
)
coderHeaderPage.isBusy.update { false }
} finally {
firstRun = false
}
Expand Down Expand Up @@ -654,13 +653,16 @@ class CoderRemoteProvider(
deploymentUrl: URL,
): SuspendBiConsumer<CoderRestClient, CoderCLIManager> = SuspendBiConsumer { client, cli ->
context.cs.launch(CoroutineName("Deferred Link Handler")) {
coderHeaderPage.isBusy.update { true }
try {
linkHandler.handle(params, deploymentUrl, client, cli)
} catch (ex: Exception) {
context.logAndShowError(
"Error handling deferred link",
ex.message ?: ""
)
} finally {
coderHeaderPage.isBusy.update { false }
}
}
}
Expand Down
68 changes: 40 additions & 28 deletions src/main/kotlin/com/coder/toolbox/CoderToolboxContext.kt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ import com.jetbrains.toolbox.api.remoteDev.connection.ToolboxProxySettings
import com.jetbrains.toolbox.api.remoteDev.states.EnvironmentStateColorPalette
import com.jetbrains.toolbox.api.remoteDev.ui.EnvironmentUiPageManager
import com.jetbrains.toolbox.api.ui.ToolboxUi
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.CoroutineName
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import java.net.URL
import java.util.UUID

Expand Down Expand Up @@ -49,44 +52,53 @@ data class CoderToolboxContext(
?: settingsStore.defaultURL.toURL()
}

suspend fun logAndShowError(title: String, error: String) {
fun logAndShowError(title: String, error: String) {
logger.error(error)
ui.showSnackbar(
UUID.randomUUID().toString(),
i18n.pnotr(title),
i18n.pnotr(error),
i18n.ptrl("OK")
)
showSnackbar(title, error)
}

suspend fun logAndShowError(title: String, error: String, exception: Exception) {
fun logAndShowError(title: String, error: String, exception: Exception) {
logger.error(exception, error)
ui.showSnackbar(
UUID.randomUUID().toString(),
i18n.pnotr(title),
i18n.pnotr(error),
i18n.ptrl("OK")
)
showSnackbar(title, error)
}

suspend fun logAndShowWarning(title: String, warning: String) {
fun logAndShowWarning(title: String, warning: String) {
logger.warn(warning)
ui.showSnackbar(
UUID.randomUUID().toString(),
i18n.pnotr(title),
i18n.pnotr(warning),
i18n.ptrl("OK")
)
showSnackbar(title, warning)
}

suspend fun logAndShowInfo(title: String, info: String) {
fun logAndShowInfo(title: String, info: String) {
logger.info(info)
ui.showSnackbar(
UUID.randomUUID().toString(),
i18n.pnotr(title),
i18n.pnotr(info),
i18n.ptrl("OK")
)
showSnackbar(title, info)
}

/**
* Displays a snackbar on a child of the plugin coroutine scope rather than on the
* caller's coroutine, without waiting for it.
*
* Toolbox keeps the [ToolboxUi.showSnackbar] coroutine suspended for the entire lifetime
* of the snackbar and cancels it (rather than resuming it) when the snackbar goes away.
* Calling it directly on the caller's coroutine would therefore either block the caller
* until the snackbar is gone or, on dismissal, abruptly cancel the caller (e.g. the URI
* handler) - skipping any code that runs after the error is shown, such as resetting the
* busy state. Launching it fire-and-forget on the plugin scope lets the caller continue
* immediately while the snackbar lives independently.
*/
fun showSnackbar(title: String, text: String) {
cs.launch(CoroutineName("snackbar")) {
try {
ui.showSnackbar(
UUID.randomUUID().toString(),
i18n.pnotr(title),
i18n.pnotr(text),
i18n.ptrl("OK")
)
} catch (_: CancellationException) {
// Expected when the snackbar is dismissed or the plugin scope shuts down.
} catch (ex: Exception) {
logger.error(ex, "Failed to display snackbar with title '$title'")
}
}
}

fun popupPluginMainPage() {
Expand Down
12 changes: 1 addition & 11 deletions src/main/kotlin/com/coder/toolbox/views/ErrorReporter.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import com.coder.toolbox.CoderToolboxContext
import com.coder.toolbox.util.prettify
import com.jetbrains.toolbox.api.remoteDev.ProviderVisibilityState
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.launch
import java.util.UUID

sealed class ErrorReporter {

Expand Down Expand Up @@ -47,15 +45,7 @@ private class ErrorReporterImpl(

private fun showError(ex: Throwable) {
val textError = ex.prettify()

context.cs.launch {
context.ui.showSnackbar(
UUID.randomUUID().toString(),
context.i18n.ptrl("Error encountered while setting up Coder"),
context.i18n.pnotr(textError),
context.i18n.ptrl("Dismiss")
)
}
context.showSnackbar("Error encountered while setting up Coder", textError)
}

override fun flush() {
Expand Down
Loading