From c408d9d513756a0dc1ac4d1d01b97f4866fb7bfc Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 11 Apr 2025 23:30:59 +0300 Subject: [PATCH 1/2] fix: show login screen when token expires during workspace polling - in fact we will now jump to the login screen for any error other than socket timeout because of an OS wake-up - this patch also contains a re-work of the REST API exception. Coder backend sends very detailed messages with the reason for the http calls to be rejected. We now un-marshall those responses and fill the exception system with better details. --- CHANGELOG.md | 11 +- .../com/coder/toolbox/CoderRemoteProvider.kt | 8 ++ .../com/coder/toolbox/sdk/CoderRestClient.kt | 80 ++++++++++++-- .../toolbox/sdk/ex/APIResponseException.kt | 102 ++++++++++++++---- .../toolbox/sdk/v2/models/ApiErrorResponse.kt | 10 ++ 5 files changed, 178 insertions(+), 33 deletions(-) create mode 100644 src/main/kotlin/com/coder/toolbox/sdk/v2/models/ApiErrorResponse.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 37b962a..5c3ec3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,16 +2,21 @@ ## Unreleased +### Fixed + +- login screen is shown instead of an empty list of workspaces when token expired + ## 0.1.4 - 2025-04-11 ### Fixed -- SSH connection to a Workspace is no longer established only once -- authorization wizard automatically goes to a previous screen when an error is encountered during connection to Coder deployment +- SSH connection to a Workspace is no longer established only once +- authorization wizard automatically goes to a previous screen when an error is encountered during connection to Coder + deployment ### Changed -- action buttons on the token input step were swapped to achieve better keyboard navigation +- action buttons on the token input step were swapped to achieve better keyboard navigation - URI `project_path` query parameter was renamed to `folder` ## 0.1.3 - 2025-04-09 diff --git a/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt b/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt index 10b39cc..85f9e33 100644 --- a/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt +++ b/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt @@ -2,6 +2,7 @@ package com.coder.toolbox import com.coder.toolbox.cli.CoderCLIManager import com.coder.toolbox.sdk.CoderRestClient +import com.coder.toolbox.sdk.ex.APIResponseException import com.coder.toolbox.sdk.v2.models.WorkspaceStatus import com.coder.toolbox.util.CoderProtocolHandler import com.coder.toolbox.util.DialogUi @@ -145,10 +146,17 @@ class CoderRemoteProvider( logout() break } + } catch (ex: APIResponseException) { + context.logger.error(ex, "error in contacting ${client.url} while polling the available workspaces") + pollError = ex + logout() + goToEnvironmentsPage() + break } catch (ex: Exception) { context.logger.error(ex, "workspace polling error encountered") pollError = ex logout() + goToEnvironmentsPage() break } diff --git a/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt b/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt index 2f87e41..feb2de4 100644 --- a/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt +++ b/src/main/kotlin/com/coder/toolbox/sdk/CoderRestClient.kt @@ -7,6 +7,7 @@ import com.coder.toolbox.sdk.convertors.OSConverter import com.coder.toolbox.sdk.convertors.UUIDConverter import com.coder.toolbox.sdk.ex.APIResponseException import com.coder.toolbox.sdk.v2.CoderV2RestFacade +import com.coder.toolbox.sdk.v2.models.ApiErrorResponse import com.coder.toolbox.sdk.v2.models.BuildInfo import com.coder.toolbox.sdk.v2.models.CreateWorkspaceBuildRequest import com.coder.toolbox.sdk.v2.models.Template @@ -24,6 +25,7 @@ import com.coder.toolbox.util.getOS import com.squareup.moshi.Moshi import okhttp3.Credentials import okhttp3.OkHttpClient +import retrofit2.Response import retrofit2.Retrofit import retrofit2.converter.moshi.MoshiConverterFactory import java.net.HttpURLConnection @@ -55,6 +57,7 @@ open class CoderRestClient( private val pluginVersion: String = "development", ) { private val settings = context.settingsStore.readOnly() + private lateinit var moshi: Moshi private lateinit var httpClient: OkHttpClient private lateinit var retroRestClient: CoderV2RestFacade @@ -66,7 +69,7 @@ open class CoderRestClient( } fun setupSession() { - val moshi = + moshi = Moshi.Builder() .add(ArchConverter()) .add(InstantConverter()) @@ -152,7 +155,7 @@ open class CoderRestClient( suspend fun me(): User { val userResponse = retroRestClient.me() if (!userResponse.isSuccessful) { - throw APIResponseException("authenticate", url, userResponse) + throw APIResponseException("authenticate", url, userResponse.code(), userResponse.parseErrorBody(moshi)) } return userResponse.body()!! @@ -165,7 +168,12 @@ open class CoderRestClient( suspend fun workspaces(): List { val workspacesResponse = retroRestClient.workspaces("owner:me") if (!workspacesResponse.isSuccessful) { - throw APIResponseException("retrieve workspaces", url, workspacesResponse) + throw APIResponseException( + "retrieve workspaces", + url, + workspacesResponse.code(), + workspacesResponse.parseErrorBody(moshi) + ) } return workspacesResponse.body()!!.workspaces @@ -178,7 +186,12 @@ open class CoderRestClient( suspend fun workspace(workspaceID: UUID): Workspace { val workspacesResponse = retroRestClient.workspace(workspaceID) if (!workspacesResponse.isSuccessful) { - throw APIResponseException("retrieve workspace", url, workspacesResponse) + throw APIResponseException( + "retrieve workspace", + url, + workspacesResponse.code(), + workspacesResponse.parseErrorBody(moshi) + ) } return workspacesResponse.body()!! @@ -209,7 +222,12 @@ open class CoderRestClient( val resourcesResponse = retroRestClient.templateVersionResources(workspace.latestBuild.templateVersionID) if (!resourcesResponse.isSuccessful) { - throw APIResponseException("retrieve resources for ${workspace.name}", url, resourcesResponse) + throw APIResponseException( + "retrieve resources for ${workspace.name}", + url, + resourcesResponse.code(), + resourcesResponse.parseErrorBody(moshi) + ) } return resourcesResponse.body()!! } @@ -217,7 +235,12 @@ open class CoderRestClient( suspend fun buildInfo(): BuildInfo { val buildInfoResponse = retroRestClient.buildInfo() if (!buildInfoResponse.isSuccessful) { - throw APIResponseException("retrieve build information", url, buildInfoResponse) + throw APIResponseException( + "retrieve build information", + url, + buildInfoResponse.code(), + buildInfoResponse.parseErrorBody(moshi) + ) } return buildInfoResponse.body()!! } @@ -228,7 +251,12 @@ open class CoderRestClient( private suspend fun template(templateID: UUID): Template { val templateResponse = retroRestClient.template(templateID) if (!templateResponse.isSuccessful) { - throw APIResponseException("retrieve template with ID $templateID", url, templateResponse) + throw APIResponseException( + "retrieve template with ID $templateID", + url, + templateResponse.code(), + templateResponse.parseErrorBody(moshi) + ) } return templateResponse.body()!! } @@ -240,7 +268,12 @@ open class CoderRestClient( val buildRequest = CreateWorkspaceBuildRequest(null, WorkspaceTransition.START) val buildResponse = retroRestClient.createWorkspaceBuild(workspace.id, buildRequest) if (buildResponse.code() != HttpURLConnection.HTTP_CREATED) { - throw APIResponseException("start workspace ${workspace.name}", url, buildResponse) + throw APIResponseException( + "start workspace ${workspace.name}", + url, + buildResponse.code(), + buildResponse.parseErrorBody(moshi) + ) } return buildResponse.body()!! } @@ -251,7 +284,12 @@ open class CoderRestClient( val buildRequest = CreateWorkspaceBuildRequest(null, WorkspaceTransition.STOP) val buildResponse = retroRestClient.createWorkspaceBuild(workspace.id, buildRequest) if (buildResponse.code() != HttpURLConnection.HTTP_CREATED) { - throw APIResponseException("stop workspace ${workspace.name}", url, buildResponse) + throw APIResponseException( + "stop workspace ${workspace.name}", + url, + buildResponse.code(), + buildResponse.parseErrorBody(moshi) + ) } return buildResponse.body()!! } @@ -263,7 +301,12 @@ open class CoderRestClient( val buildRequest = CreateWorkspaceBuildRequest(null, WorkspaceTransition.DELETE, false) val buildResponse = retroRestClient.createWorkspaceBuild(workspace.id, buildRequest) if (buildResponse.code() != HttpURLConnection.HTTP_CREATED) { - throw APIResponseException("delete workspace ${workspace.name}", url, buildResponse) + throw APIResponseException( + "delete workspace ${workspace.name}", + url, + buildResponse.code(), + buildResponse.parseErrorBody(moshi) + ) } } @@ -283,7 +326,12 @@ open class CoderRestClient( CreateWorkspaceBuildRequest(template.activeVersionID, WorkspaceTransition.START) val buildResponse = retroRestClient.createWorkspaceBuild(workspace.id, buildRequest) if (buildResponse.code() != HttpURLConnection.HTTP_CREATED) { - throw APIResponseException("update workspace ${workspace.name}", url, buildResponse) + throw APIResponseException( + "update workspace ${workspace.name}", + url, + buildResponse.code(), + buildResponse.parseErrorBody(moshi) + ) } return buildResponse.body()!! } @@ -296,3 +344,13 @@ open class CoderRestClient( } } } + +private fun Response<*>.parseErrorBody(moshi: Moshi): ApiErrorResponse? { + val errorBody = this.errorBody() ?: return null + return try { + val adapter = moshi.adapter(ApiErrorResponse::class.java) + adapter.fromJson(errorBody.string()) + } catch (e: Exception) { + null + } +} \ No newline at end of file diff --git a/src/main/kotlin/com/coder/toolbox/sdk/ex/APIResponseException.kt b/src/main/kotlin/com/coder/toolbox/sdk/ex/APIResponseException.kt index 2540ca8..9f78198 100644 --- a/src/main/kotlin/com/coder/toolbox/sdk/ex/APIResponseException.kt +++ b/src/main/kotlin/com/coder/toolbox/sdk/ex/APIResponseException.kt @@ -1,26 +1,90 @@ package com.coder.toolbox.sdk.ex +import com.coder.toolbox.sdk.v2.models.ApiErrorResponse import java.io.IOException import java.net.HttpURLConnection import java.net.URL -class APIResponseException(action: String, url: URL, res: retrofit2.Response<*>) : - IOException( - "Unable to $action: url=$url, code=${res.code()}, details=${ - when (res.code()) { - HttpURLConnection.HTTP_NOT_FOUND -> "The requested resource could not be found" - else -> res.errorBody()?.charStream()?.use { - val text = it.readText() - // Be careful with the length because if you try to show a - // notification in Toolbox that is too large it crashes the - // application. - if (text.length > 500) { - "${text.substring(0, 500)}…" - } else { - text - } - } ?: "no details provided" - }}", - ) { - val isUnauthorized = res.code() == HttpURLConnection.HTTP_UNAUTHORIZED +class APIResponseException(action: String, url: URL, code: Int, errorResponse: ApiErrorResponse?) : + IOException(formatToPretty(action, url, code, errorResponse)) { + + + val isUnauthorized = HttpURLConnection.HTTP_UNAUTHORIZED == code + + companion object { + private fun formatToPretty( + action: String, + url: URL, + code: Int, + errorResponse: ApiErrorResponse?, + ): String { + return if (errorResponse == null) { + "Unable to $action: url=$url, code=$code, details=${HttpErrorStatusMapper.getMessage(code)}" + } else { + var msg = "Unable to $action: url=$url, code=$code, message=${errorResponse.message}" + if (errorResponse.detail?.isNotEmpty() == true) { + msg += ", reason=${errorResponse.detail}" + } + + // Be careful with the length because if you try to show a + // notification in Toolbox that is too large it crashes the + // application. + if (msg.length > 500) { + msg = "${msg.substring(0, 500)}…" + } + msg + } + } + } +} + +private object HttpErrorStatusMapper { + private val errorStatusMap = mapOf( + // 4xx: Client Errors + 400 to "Bad Request", + 401 to "Unauthorized", + 402 to "Payment Required", + 403 to "Forbidden", + 404 to "Not Found", + 405 to "Method Not Allowed", + 406 to "Not Acceptable", + 407 to "Proxy Authentication Required", + 408 to "Request Timeout", + 409 to "Conflict", + 410 to "Gone", + 411 to "Length Required", + 412 to "Precondition Failed", + 413 to "Payload Too Large", + 414 to "URI Too Long", + 415 to "Unsupported Media Type", + 416 to "Range Not Satisfiable", + 417 to "Expectation Failed", + 418 to "I'm a teapot", + 421 to "Misdirected Request", + 422 to "Unprocessable Entity", + 423 to "Locked", + 424 to "Failed Dependency", + 425 to "Too Early", + 426 to "Upgrade Required", + 428 to "Precondition Required", + 429 to "Too Many Requests", + 431 to "Request Header Fields Too Large", + 451 to "Unavailable For Legal Reasons", + + // 5xx: Server Errors + 500 to "Internal Server Error", + 501 to "Not Implemented", + 502 to "Bad Gateway", + 503 to "Service Unavailable", + 504 to "Gateway Timeout", + 505 to "HTTP Version Not Supported", + 506 to "Variant Also Negotiates", + 507 to "Insufficient Storage", + 508 to "Loop Detected", + 510 to "Not Extended", + 511 to "Network Authentication Required" + ) + + fun getMessage(code: Int): String = + errorStatusMap[code] ?: "Unknown Error Status" } diff --git a/src/main/kotlin/com/coder/toolbox/sdk/v2/models/ApiErrorResponse.kt b/src/main/kotlin/com/coder/toolbox/sdk/v2/models/ApiErrorResponse.kt new file mode 100644 index 0000000..167e020 --- /dev/null +++ b/src/main/kotlin/com/coder/toolbox/sdk/v2/models/ApiErrorResponse.kt @@ -0,0 +1,10 @@ +package com.coder.toolbox.sdk.v2.models + +import com.squareup.moshi.Json +import com.squareup.moshi.JsonClass + +@JsonClass(generateAdapter = true) +data class ApiErrorResponse( + @Json(name = "message") val message: String, + @Json(name = "detail") val detail: String?, +) From da30c16be62123d9aedc605daca457cdd46433e7 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Sat, 12 Apr 2025 01:14:17 +0300 Subject: [PATCH 2/2] fix: exception collector was never reset - forcing the main screen to always show the same exception - this patch refactors the code to accumulate the errors in a buffer and show them when auth screen is visible - in addition, the error reporting uses the Snackbar api which provides greater control, and the possibility to stack errors one on top of each other. This approach simplifies the code on our side even more because the ui page no longer needs to accumulate the errors in a buffer and process them when a notifier is injected by toolbox. --- .../com/coder/toolbox/CoderRemoteProvider.kt | 22 +++++++------- .../com/coder/toolbox/views/CoderPage.kt | 29 ++++++------------- 2 files changed, 20 insertions(+), 31 deletions(-) diff --git a/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt b/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt index 85f9e33..292e276 100644 --- a/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt +++ b/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt @@ -61,7 +61,7 @@ class CoderRemoteProvider( // If we have an error in the polling we store it here before going back to // sign-in page, so we can display it there. This is mainly because there // does not seem to be a mechanism to show errors on the environment list. - private var pollError: Exception? = null + private var errorBuffer = mutableListOf() // On the first load, automatically log in if we can. private var firstRun = true @@ -142,19 +142,19 @@ class CoderRemoteProvider( client.setupSession() } else { context.logger.error(ex, "workspace polling error encountered") - pollError = ex + errorBuffer.add(ex) logout() break } } catch (ex: APIResponseException) { context.logger.error(ex, "error in contacting ${client.url} while polling the available workspaces") - pollError = ex + errorBuffer.add(ex) logout() goToEnvironmentsPage() break } catch (ex: Exception) { context.logger.error(ex, "workspace polling error encountered") - pollError = ex + errorBuffer.add(ex) logout() goToEnvironmentsPage() break @@ -308,7 +308,6 @@ class CoderRemoteProvider( if (client == null) { // When coming back to the application, authenticate immediately. val autologin = shouldDoAutoLogin() - var autologinEx: Exception? = null context.secrets.lastToken.let { lastToken -> context.secrets.lastDeploymentURL.let { lastDeploymentURL -> if (autologin && lastDeploymentURL.isNotBlank() && (lastToken.isNotBlank() || !settings.requireTokenAuth)) { @@ -316,7 +315,7 @@ class CoderRemoteProvider( AuthWizardState.goToStep(WizardStep.LOGIN) return AuthWizardPage(context, true, ::onConnect) } catch (ex: Exception) { - autologinEx = ex + errorBuffer.add(ex) } } } @@ -325,11 +324,12 @@ class CoderRemoteProvider( // Login flow. val authWizard = AuthWizardPage(context, false, ::onConnect) - // We might have tried and failed to automatically log in. - autologinEx?.let { authWizard.notify("Error logging in", it) } // We might have navigated here due to a polling error. - pollError?.let { authWizard.notify("Error fetching workspaces", it) } - + errorBuffer.forEach { + authWizard.notify("Error encountered", it) + } + // and now reset the errors, otherwise we show it every time on the screen + errorBuffer.clear() return authWizard } return null @@ -344,7 +344,7 @@ class CoderRemoteProvider( // Currently we always remember, but this could be made an option. context.secrets.rememberMe = true this.client = client - pollError = null + errorBuffer.clear() pollJob?.cancel() pollJob = poll(client, cli) goToEnvironmentsPage() diff --git a/src/main/kotlin/com/coder/toolbox/views/CoderPage.kt b/src/main/kotlin/com/coder/toolbox/views/CoderPage.kt index eb2f252..77fb1c2 100644 --- a/src/main/kotlin/com/coder/toolbox/views/CoderPage.kt +++ b/src/main/kotlin/com/coder/toolbox/views/CoderPage.kt @@ -6,6 +6,8 @@ import com.jetbrains.toolbox.api.core.ui.icons.SvgIcon.IconType import com.jetbrains.toolbox.api.localization.LocalizableString import com.jetbrains.toolbox.api.ui.actions.RunnableActionDescription import com.jetbrains.toolbox.api.ui.components.UiPage +import kotlinx.coroutines.launch +import java.util.UUID /** * Base page that handles the icon, displaying error notifications, and @@ -23,12 +25,6 @@ abstract class CoderPage( showIcon: Boolean = true, ) : UiPage(title) { - /** Toolbox uses this to show notifications on the page. */ - private var notifier: ((Throwable) -> Unit)? = null - - /** Stores errors until the notifier is attached. */ - private var errorBuffer: MutableList = mutableListOf() - /** * Return the icon, if showing one. * @@ -48,20 +44,13 @@ abstract class CoderPage( */ fun notify(logPrefix: String, ex: Throwable) { context.logger.error(ex, logPrefix) - // It is possible the error listener is not attached yet. - notifier?.let { it(ex) } ?: errorBuffer.add(ex) - } - - /** - * Immediately notify any pending errors and store for later errors. - */ - override fun setActionErrorNotifier(notifier: ((Throwable) -> Unit)?) { - this.notifier = notifier - notifier?.let { - errorBuffer.forEach { - notifier(it) - } - errorBuffer.clear() + context.cs.launch { + context.ui.showSnackbar( + UUID.randomUUID().toString(), + context.i18n.pnotr(logPrefix), + context.i18n.pnotr(ex.message ?: ""), + context.i18n.ptrl("Dismiss") + ) } } }