From 837abb3d3526efede941cf36f0d6c89f3cac0995 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Fri, 29 Aug 2025 00:55:45 +0300 Subject: [PATCH 1/2] fix: NPE during error reporting The try/catch block raised NPE in the `notify` if another exception was raised after the context containing the URL was reset - so that means an error in the onConnect handler. In addition, some of the reset steps were moved after onConnect to make sure they execute only if onConnect callback is successful. Because of the fault in how the steps were arranged, the original exception was never logged instead a misleading NPE was treated by the coroutine's exception handler. --- CHANGELOG.md | 4 ++++ .../com/coder/toolbox/CoderRemoteProvider.kt | 1 - .../kotlin/com/coder/toolbox/views/ConnectStep.kt | 15 ++++++++++----- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 826e038..8358d75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - improved diagnose support +### Fixed + +- NPE during error reporting + ## 0.6.3 - 2025-08-25 ### Added diff --git a/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt b/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt index cf9e04e..8c6dcd3 100644 --- a/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt +++ b/src/main/kotlin/com/coder/toolbox/CoderRemoteProvider.kt @@ -409,7 +409,6 @@ class CoderRemoteProvider( context.logger.info("Displaying ${client.url} in the UI") pollJob = poll(client, cli) context.logger.info("Workspace poll job created with reference $pollJob") - context.envPageManager.showPluginEnvironmentsPage() } private fun MutableStateFlow>>.showLoadingMessage() { diff --git a/src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt b/src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt index f005b53..a38469f 100644 --- a/src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt +++ b/src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt @@ -73,6 +73,9 @@ class ConnectStep( errorField.textState.update { context.i18n.ptrl("Token is required") } return } + // Capture the host name early for error reporting + val hostName = CoderCliSetupContext.url!!.host + signInJob?.cancel() signInJob = context.cs.launch(CoroutineName("Http and CLI Setup")) { try { @@ -100,21 +103,23 @@ class ConnectStep( yield() cli.login(client.token!!) } - logAndReportProgress("Successfully configured ${CoderCliSetupContext.url!!.host}...") + logAndReportProgress("Successfully configured ${hostName}...") // allows interleaving with the back/cancel action yield() - CoderCliSetupContext.reset() - CoderCliSetupWizardState.goToFirstStep() context.logger.info("Connection setup done, initializing the workspace poller...") onConnect(client, cli) + + CoderCliSetupContext.reset() + CoderCliSetupWizardState.goToFirstStep() + context.envPageManager.showPluginEnvironmentsPage() } catch (ex: CancellationException) { if (ex.message != USER_HIT_THE_BACK_BUTTON) { - notify("Connection to ${CoderCliSetupContext.url!!.host} was configured", ex) + notify("Connection to $hostName was configured", ex) handleNavigation() refreshWizard() } } catch (ex: Exception) { - notify("Failed to configure ${CoderCliSetupContext.url!!.host}", ex) + notify("Failed to configure $hostName", ex) handleNavigation() refreshWizard() } From 7bbc1618a6b44f5d15bdd8a091a6bd3dec2d4751 Mon Sep 17 00:00:00 2001 From: Faur Ioan-Aurel Date: Mon, 1 Sep 2025 23:09:19 +0300 Subject: [PATCH 2/2] chore: refactor the error reporter --- .../toolbox/views/CoderCliSetupWizardPage.kt | 47 ++---------- .../com/coder/toolbox/views/ConnectStep.kt | 14 ++-- .../coder/toolbox/views/DeploymentUrlStep.kt | 9 ++- .../com/coder/toolbox/views/ErrorReporter.kt | 73 +++++++++++++++++++ 4 files changed, 94 insertions(+), 49 deletions(-) create mode 100644 src/main/kotlin/com/coder/toolbox/views/ErrorReporter.kt diff --git a/src/main/kotlin/com/coder/toolbox/views/CoderCliSetupWizardPage.kt b/src/main/kotlin/com/coder/toolbox/views/CoderCliSetupWizardPage.kt index 7ff36bc..bca3606 100644 --- a/src/main/kotlin/com/coder/toolbox/views/CoderCliSetupWizardPage.kt +++ b/src/main/kotlin/com/coder/toolbox/views/CoderCliSetupWizardPage.kt @@ -3,22 +3,19 @@ package com.coder.toolbox.views import com.coder.toolbox.CoderToolboxContext import com.coder.toolbox.cli.CoderCLIManager import com.coder.toolbox.sdk.CoderRestClient -import com.coder.toolbox.sdk.ex.APIResponseException import com.coder.toolbox.views.state.CoderCliSetupWizardState import com.coder.toolbox.views.state.WizardStep import com.jetbrains.toolbox.api.remoteDev.ProviderVisibilityState import com.jetbrains.toolbox.api.ui.actions.RunnableActionDescription import com.jetbrains.toolbox.api.ui.components.UiField -import kotlinx.coroutines.CoroutineName import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.update -import kotlinx.coroutines.launch -import java.util.UUID class CoderCliSetupWizardPage( private val context: CoderToolboxContext, private val settingsPage: CoderSettingsPage, - private val visibilityState: MutableStateFlow, + visibilityState: StateFlow, initialAutoSetup: Boolean = false, jumpToMainPageOnError: Boolean = false, onConnect: suspend ( @@ -31,16 +28,17 @@ class CoderCliSetupWizardPage( context.ui.showUiPage(settingsPage) }) - private val deploymentUrlStep = DeploymentUrlStep(context, this::notify) + private val deploymentUrlStep = DeploymentUrlStep(context, visibilityState) private val tokenStep = TokenStep(context) private val connectStep = ConnectStep( context, shouldAutoLogin = shouldAutoSetup, jumpToMainPageOnError, - this::notify, + visibilityState, this::displaySteps, onConnect ) + private val errorReporter = ErrorReporter.create(context, visibilityState, this.javaClass) /** * Fields for this page, displayed in order. @@ -48,16 +46,10 @@ class CoderCliSetupWizardPage( override val fields: MutableStateFlow> = MutableStateFlow(emptyList()) override val actionButtons: MutableStateFlow> = MutableStateFlow(emptyList()) - private val errorBuffer = mutableListOf() override fun beforeShow() { displaySteps() - if (errorBuffer.isNotEmpty() && visibilityState.value.applicationVisible) { - errorBuffer.forEach { - showError(it) - } - errorBuffer.clear() - } + errorReporter.flush() } private fun displaySteps() { @@ -124,30 +116,5 @@ class CoderCliSetupWizardPage( /** * Show an error as a popup on this page. */ - fun notify(logPrefix: String, ex: Throwable) { - context.logger.error(ex, logPrefix) - if (!visibilityState.value.applicationVisible) { - context.logger.debug("Toolbox is not yet visible, scheduling error to be displayed later") - errorBuffer.add(ex) - return - } - showError(ex) - } - - private fun showError(ex: Throwable) { - val textError = if (ex is APIResponseException) { - if (!ex.reason.isNullOrBlank()) { - ex.reason - } else ex.message - } else ex.message - - context.cs.launch(CoroutineName("Coder Setup Visual Error Reporting")) { - context.ui.showSnackbar( - UUID.randomUUID().toString(), - context.i18n.ptrl("Error encountered while setting up Coder"), - context.i18n.pnotr(textError ?: ""), - context.i18n.ptrl("Dismiss") - ) - } - } + fun notify(message: String, ex: Throwable) = errorReporter.report(message, ex) } diff --git a/src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt b/src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt index a38469f..7798328 100644 --- a/src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt +++ b/src/main/kotlin/com/coder/toolbox/views/ConnectStep.kt @@ -7,6 +7,7 @@ import com.coder.toolbox.plugin.PluginManager import com.coder.toolbox.sdk.CoderRestClient import com.coder.toolbox.views.state.CoderCliSetupContext import com.coder.toolbox.views.state.CoderCliSetupWizardState +import com.jetbrains.toolbox.api.remoteDev.ProviderVisibilityState import com.jetbrains.toolbox.api.ui.components.LabelField import com.jetbrains.toolbox.api.ui.components.RowGroup import com.jetbrains.toolbox.api.ui.components.ValidationErrorField @@ -27,17 +28,15 @@ class ConnectStep( private val context: CoderToolboxContext, private val shouldAutoLogin: StateFlow, private val jumpToMainPageOnError: Boolean, - private val notify: (String, Throwable) -> Unit, + visibilityState: StateFlow, private val refreshWizard: () -> Unit, - private val onConnect: suspend ( - client: CoderRestClient, - cli: CoderCLIManager, - ) -> Unit, + private val onConnect: suspend (client: CoderRestClient, cli: CoderCLIManager) -> Unit, ) : WizardStep { private var signInJob: Job? = null private val statusField = LabelField(context.i18n.pnotr("")) private val errorField = ValidationErrorField(context.i18n.pnotr("")) + private val errorReporter = ErrorReporter.create(context, visibilityState, this.javaClass) override val panel: RowGroup = RowGroup( RowGroup.RowField(statusField), @@ -45,6 +44,7 @@ class ConnectStep( ) override fun onVisible() { + errorReporter.flush() errorField.textState.update { context.i18n.pnotr("") } @@ -114,12 +114,12 @@ class ConnectStep( context.envPageManager.showPluginEnvironmentsPage() } catch (ex: CancellationException) { if (ex.message != USER_HIT_THE_BACK_BUTTON) { - notify("Connection to $hostName was configured", ex) + errorReporter.report("Connection to $hostName was configured", ex) handleNavigation() refreshWizard() } } catch (ex: Exception) { - notify("Failed to configure $hostName", ex) + errorReporter.report("Failed to configure $hostName", ex) handleNavigation() refreshWizard() } diff --git a/src/main/kotlin/com/coder/toolbox/views/DeploymentUrlStep.kt b/src/main/kotlin/com/coder/toolbox/views/DeploymentUrlStep.kt index 34b027c..be3d4d0 100644 --- a/src/main/kotlin/com/coder/toolbox/views/DeploymentUrlStep.kt +++ b/src/main/kotlin/com/coder/toolbox/views/DeploymentUrlStep.kt @@ -6,6 +6,7 @@ import com.coder.toolbox.util.toURL import com.coder.toolbox.util.validateStrictWebUrl import com.coder.toolbox.views.state.CoderCliSetupContext import com.coder.toolbox.views.state.CoderCliSetupWizardState +import com.jetbrains.toolbox.api.remoteDev.ProviderVisibilityState import com.jetbrains.toolbox.api.ui.components.CheckboxField import com.jetbrains.toolbox.api.ui.components.LabelField import com.jetbrains.toolbox.api.ui.components.LabelStyleType @@ -13,6 +14,7 @@ import com.jetbrains.toolbox.api.ui.components.RowGroup import com.jetbrains.toolbox.api.ui.components.TextField import com.jetbrains.toolbox.api.ui.components.TextType import com.jetbrains.toolbox.api.ui.components.ValidationErrorField +import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.update import java.net.MalformedURLException import java.net.URL @@ -25,9 +27,11 @@ import java.net.URL */ class DeploymentUrlStep( private val context: CoderToolboxContext, - private val notify: (String, Throwable) -> Unit + visibilityState: StateFlow, ) : WizardStep { + private val errorReporter = ErrorReporter.create(context, visibilityState, this.javaClass) + private val urlField = TextField(context.i18n.ptrl("Deployment URL"), "", TextType.General) private val emptyLine = LabelField(context.i18n.pnotr(""), LabelStyleType.Normal) @@ -66,6 +70,7 @@ class DeploymentUrlStep( signatureFallbackStrategyField.checkedState.update { context.settingsStore.fallbackOnCoderForSignatures.isAllowed() } + errorReporter.flush() } override fun onNext(): Boolean { @@ -78,7 +83,7 @@ class DeploymentUrlStep( try { CoderCliSetupContext.url = validateRawUrl(url) } catch (e: MalformedURLException) { - notify("URL is invalid", e) + errorReporter.report("URL is invalid", e) return false } if (context.settingsStore.requireTokenAuth) { diff --git a/src/main/kotlin/com/coder/toolbox/views/ErrorReporter.kt b/src/main/kotlin/com/coder/toolbox/views/ErrorReporter.kt new file mode 100644 index 0000000..88ace65 --- /dev/null +++ b/src/main/kotlin/com/coder/toolbox/views/ErrorReporter.kt @@ -0,0 +1,73 @@ +package com.coder.toolbox.views + +import com.coder.toolbox.CoderToolboxContext +import com.coder.toolbox.sdk.ex.APIResponseException +import com.jetbrains.toolbox.api.remoteDev.ProviderVisibilityState +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.launch +import java.util.UUID + +sealed class ErrorReporter { + + /** + * Logs and show errors as popups. + */ + abstract fun report(message: String, ex: Throwable) + + /** + * Processes any buffered errors when the application becomes visible. + */ + abstract fun flush() + + companion object { + fun create( + context: CoderToolboxContext, + visibilityState: StateFlow, + callerClass: Class<*> + ): ErrorReporter = ErrorReporterImpl(context, visibilityState, callerClass) + } +} + +private class ErrorReporterImpl( + private val context: CoderToolboxContext, + private val visibilityState: StateFlow, + private val callerClass: Class<*> +) : ErrorReporter() { + private val errorBuffer = mutableListOf() + + override fun report(message: String, ex: Throwable) { + context.logger.error(ex, "[${callerClass.simpleName}] $message") + if (!visibilityState.value.applicationVisible) { + context.logger.debug("Toolbox is not yet visible, scheduling error to be displayed later") + errorBuffer.add(ex) + return + } + showError(ex) + } + + private fun showError(ex: Throwable) { + val textError = if (ex is APIResponseException) { + if (!ex.reason.isNullOrBlank()) { + ex.reason + } else ex.message + } else ex.message ?: ex.toString() + 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") + ) + } + } + + + override fun flush() { + if (errorBuffer.isNotEmpty() && visibilityState.value.applicationVisible) { + errorBuffer.forEach { + showError(it) + } + errorBuffer.clear() + } + } +} \ No newline at end of file