Skip to content

fix(amazonq): cancel chat export doesn't bubble error up to user #5723

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 7 commits into from
May 15, 2025
Merged

fix(amazonq): cancel chat export doesn't bubble error up to user #5723

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1cb3cca
handle error properly
samgst-amazon May 15, 2025
6634ff0
Merge branch 'feature/q-lsp-chat' into samgst/q-lsp-chat-export-cancel
samgst-amazon May 15, 2025
6d0df4e
override breaking build
samgst-amazon May 15, 2025
a2c8672
Update plugins/amazonq/chat/jetbrains-community/src/software/aws/tool…
samgst-amazon May 15, 2025
5ba6cf9
Update plugins/amazonq/chat/jetbrains-community/src/software/aws/tool…
samgst-amazon May 15, 2025
72ef6d9
feedback
samgst-amazon May 15, 2025
5707dcc
detekt
samgst-amazon May 15, 2025
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
14 changes: 11 additions & 3 deletions ...ommunity/src/software/aws/toolkits/jetbrains/services/amazonq/webview/BrowserConnector.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import org.cef.browser.CefBrowser
import org.eclipse.lsp4j.TextDocumentIdentifier
import org.eclipse.lsp4j.jsonrpc.ResponseErrorException
import software.aws.toolkits.core.utils.error
import software.aws.toolkits.core.utils.getLogger
import software.aws.toolkits.core.utils.warn
Expand Down Expand Up @@ -94,6 +95,7 @@ import software.aws.toolkits.jetbrains.settings.MeetQSettings
import software.aws.toolkits.telemetry.MetricResult
import software.aws.toolkits.telemetry.Telemetry
import java.util.concurrent.CompletableFuture
import java.util.concurrent.CompletionException
import java.util.function.Function

class BrowserConnector(
Expand Down Expand Up @@ -382,6 +384,7 @@ class BrowserConnector(

CHAT_TAB_BAR_ACTIONS -> {
handleChat(AmazonQChatServer.tabBarActions, node) { params, invoke ->
println("handling TabBarActions")
invoke()
.whenComplete { actions, error ->
try {
Expand All @@ -396,9 +399,14 @@ class BrowserConnector(
)
)
} catch (e: Exception) {
LOG.error { "Failed to perform chat tab bar action $e" }
params.tabId?.let {
browser.postChat(chatCommunicationManager.getErrorUiMessage(it, e, null))
val cause = if (e is CompletionException) e.cause else e

// dont post error to UI if user cancels export
if (!(cause is ResponseErrorException && cause.responseError.message == "Export cancelled by user")) {
LOG.error { "Failed to perform chat tab bar action $e" }
params.tabId?.let {
browser.postChat(chatCommunicationManager.getErrorUiMessage(it, e, null))
}
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions ...are/aws/toolkits/jetbrains/services/codewhisperer/language/languages/CodeWhispererAbap.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ class CodeWhispererAbap private constructor() : CodeWhispererProgrammingLanguage

override fun toTelemetryType(): CodewhispererLanguage = CodewhispererLanguage.Abap

override fun isCodeCompletionSupported(): Boolean = true

companion object {
const val ID = "abap"

Expand Down
6 changes: 4 additions & 2 deletions ...ity/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/AmazonQLanguageClientImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ import org.eclipse.lsp4j.PublishDiagnosticsParams
import org.eclipse.lsp4j.ShowDocumentParams
import org.eclipse.lsp4j.ShowDocumentResult
import org.eclipse.lsp4j.ShowMessageRequestParams
import org.eclipse.lsp4j.jsonrpc.ResponseErrorException
import org.eclipse.lsp4j.jsonrpc.messages.ResponseError
import org.eclipse.lsp4j.jsonrpc.messages.ResponseErrorCode
import org.slf4j.event.Level
import software.aws.toolkits.core.utils.error
import software.aws.toolkits.core.utils.getLogger
Expand Down Expand Up @@ -230,8 +233,7 @@ class AmazonQLanguageClientImpl(private val project: Project) : AmazonQLanguageC

chosenFile?.let {
ShowSaveFileDialogResult(chosenFile.file.path)
// TODO: Add error state shown in chat ui instead of throwing
} ?: throw Error("Export failed")
} ?: throw ResponseErrorException(ResponseError(ResponseErrorCode.RequestCancelled, "Export cancelled by user", null))
Copy link
Contributor

Choose a reason for hiding this comment

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

lol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we need to throw this here to get the specific RequestCancelled error otherwise the ResponseErrorCode is InternalError which I think is a little too generic and I'm afraid it may unintentionally catch things. Is there any other reason the chosenFile would be null?

Copy link
Contributor

Choose a reason for hiding this comment

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

seems like they either choose a file or they dont

Copy link
Contributor

Choose a reason for hiding this comment

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

The other error condition that I can think of here is if a file is chosen which is not html or md if it exists but with the filtering dont think a user should run into this

},
ApplicationManager.getApplication()::invokeLater
)
Expand Down
Loading