setup script can communicate an error message to the end user #538
Conversation
src/main/kotlin/com/coder/gateway/CoderRemoteConnectionHandle.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/com/coder/gateway/CoderRemoteConnectionHandle.kt
Outdated
Show resolved
Hide resolved
| ?.let { it.substring(it.indexOf(GATEWAY_SETUP_COMMAND_ERROR) + GATEWAY_SETUP_COMMAND_ERROR.length).trim() } | ||
|
|
||
| if (!errorText.isNullOrBlank()) { | ||
| throw Exception(errorText) |
There was a problem hiding this comment.
in Java&Kotlin it is best to avoid throwing the generic exception, for many reasons - but some of them are that it's too generic, and probably the most important one: upstream code can be forced to catch all checked exceptions
In other words we should be more specific and throw one of java/kotlin checked exceptions or we can create our own custom exception. IOException is probably good enough, or maybe something custom like CmdExecutionException.
There was a problem hiding this comment.
I agree, just tried not to change the code much. So do we want a new exception type for this? What do you want it to be called?
There was a problem hiding this comment.
So I called it CoderSetupCommandException and provided handling when we show the dialog so it's clearly says what it's a setup command problem, not just an abstract exception
| class CoderSetupCommandException : Exception { | ||
|
|
||
| constructor(message: String) : super(message) | ||
| constructor(message: String, cause: Throwable) : super(message, cause) |
There was a problem hiding this comment.
a more simpler approach:
class CoderSetupCommandException(message: String, cause: Throwable? = null) : Exception(message, cause)
| @@ -523,5 +534,26 @@ class CoderRemoteConnectionHandle { | |||
|
|
|||
| companion object { | |||
| val logger = Logger.getInstance(CoderRemoteConnectionHandle::class.java.simpleName) | |||
| @Throws(CoderSetupCommandException::class) | |||
| fun processSetupCommand( | |||
| ignoreSetupFailure: Boolean, | |||
There was a problem hiding this comment.
Not a comment on this PR just a note to self about how we're doing this in general: don't love us drilling down this Boolean flag for ignoring errors.
|
@kirillk in response to your slack comment
No :) |
Setup script can provide an error message to be shown to the end user.
Example:
