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
+14 −4
Merged

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

Show file tree
Hide file tree
Changes from all 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
10 changes: 10 additions & 0 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,8 @@
import kotlinx.coroutines.launch
import org.cef.browser.CefBrowser
import org.eclipse.lsp4j.TextDocumentIdentifier
import org.eclipse.lsp4j.jsonrpc.ResponseErrorException
import org.eclipse.lsp4j.jsonrpc.messages.ResponseErrorCode
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 +96,7 @@
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 +385,7 @@

CHAT_TAB_BAR_ACTIONS -> {
handleChat(AmazonQChatServer.tabBarActions, node) { params, invoke ->
println("handling TabBarActions")
invoke()
.whenComplete { actions, error ->
try {
Expand All @@ -396,6 +400,12 @@
)
)
} catch (e: Exception) {
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.code == ResponseErrorCode.RequestCancelled.getValue()) {

Check notice on line 406 in plugins/amazonq/chat/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/webview/BrowserConnector.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Accessor call that can be replaced with property access syntax 

Use of getter method instead of property access syntax

Check notice

Code scanning / QDJVMC

Accessor call that can be replaced with property access syntax Note

Use of getter method instead of property access syntax
return@whenComplete

Check warning on line 407 in plugins/amazonq/chat/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/webview/BrowserConnector.kt

View check run for this annotation

Codecov / codecov/patch

plugins/amazonq/chat/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/webview/BrowserConnector.kt#L407 

Added line #L407 was not covered by tests
}
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.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 @@

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))

Check warning on line 236 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/AmazonQLanguageClientImpl.kt

View check run for this annotation

Codecov / codecov/patch

plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/AmazonQLanguageClientImpl.kt#L236 

Added line #L236 was not covered by tests
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