-
Notifications
You must be signed in to change notification settings - Fork 268
fix(amazonq): Replacing message bus with direct communication #5949
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.
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
base: main
Are you sure you want to change the base?
Changes from 11 commits
364f6e8
ccc77f2
e91fe0e
1f8e687
b9cea7a
459352d
539c2ca
2291677
aa0dc81
bf608df
dbd94a9
2307343
8499b64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,7 +43,6 @@ import software.aws.toolkits.jetbrains.services.amazonq.lsp.JsonRpcNotification | |
import software.aws.toolkits.jetbrains.services.amazonq.lsp.JsonRpcRequest | ||
import software.aws.toolkits.jetbrains.services.amazonq.lsp.encryption.JwtEncryptionManager | ||
import software.aws.toolkits.jetbrains.services.amazonq.lsp.flareChat.AwsServerCapabilitiesProvider | ||
import software.aws.toolkits.jetbrains.services.amazonq.lsp.flareChat.ChatAsyncResultManager | ||
import software.aws.toolkits.jetbrains.services.amazonq.lsp.flareChat.ChatCommunicationManager | ||
import software.aws.toolkits.jetbrains.services.amazonq.lsp.flareChat.FlareUiMessage | ||
import software.aws.toolkits.jetbrains.services.amazonq.lsp.model.aws.chat.AUTH_FOLLOW_UP_CLICKED | ||
|
@@ -123,7 +122,6 @@ class BrowserConnector( | |
) { | ||
val uiReady = CompletableDeferred<Boolean>() | ||
private val chatCommunicationManager = ChatCommunicationManager.getInstance(project) | ||
private val chatAsyncResultManager = ChatAsyncResultManager.getInstance(project) | ||
|
||
suspend fun connect( | ||
browser: Browser, | ||
|
@@ -240,7 +238,6 @@ class BrowserConnector( | |
|
||
val tabId = requestFromUi.params.tabId | ||
val partialResultToken = chatCommunicationManager.addPartialChatMessage(tabId) | ||
chatCommunicationManager.registerPartialResultToken(partialResultToken) | ||
|
||
var encryptionManager: JwtEncryptionManager? = null | ||
val result = AmazonQLspService.executeAsyncIfRunning(project) { server -> | ||
|
@@ -261,7 +258,6 @@ class BrowserConnector( | |
val tabId = requestFromUi.params.tabId | ||
val quickActionParams = node.params ?: error("empty payload") | ||
val partialResultToken = chatCommunicationManager.addPartialChatMessage(tabId) | ||
chatCommunicationManager.registerPartialResultToken(partialResultToken) | ||
var encryptionManager: JwtEncryptionManager? = null | ||
val result = AmazonQLspService.executeAsyncIfRunning(project) { server -> | ||
encryptionManager = this.encryptionManager | ||
|
@@ -595,30 +591,13 @@ class BrowserConnector( | |
chatCommunicationManager.removeInflightRequestForTab(tabId) | ||
} catch (e: CancellationException) { | ||
LOG.warn { "Cancelled chat generation" } | ||
try { | ||
chatAsyncResultManager.createRequestId(partialResultToken) | ||
chatAsyncResultManager.getResult(partialResultToken) | ||
handleCancellation(tabId, browser) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this ok to delete There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this code is introduced when stop conversation needed to be handled on JB side. Now it is not required. So reverting this change. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we also maintain records for partial results cause they are received separately and we mark them done when the result is completed, are we sure that line 599-600 can be deleted? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to mirror the cancellation logic here how it's handled in Eclipse. In the bug bash today removing this results in the original issue where:
jk jk |
||
} catch (ex: Exception) { | ||
LOG.warn(ex) { "An error occurred while processing cancellation" } | ||
} finally { | ||
chatAsyncResultManager.removeRequestId(partialResultToken) | ||
chatCommunicationManager.removePartialResultLock(partialResultToken) | ||
chatCommunicationManager.removeFinalResultProcessed(partialResultToken) | ||
} | ||
} catch (e: Exception) { | ||
LOG.warn(e) { "Failed to send chat message" } | ||
browser.postChat(chatCommunicationManager.getErrorUiMessage(tabId, e, partialResultToken)) | ||
} | ||
} | ||
} | ||
|
||
private fun handleCancellation(tabId: String, browser: Browser) { | ||
// Send a message to hide the stop button without showing an error | ||
val cancelMessage = chatCommunicationManager.getCancellationUiMessage(tabId) | ||
browser.postChat(cancelMessage) | ||
} | ||
|
||
private fun updateQuickActionsInBrowser(browser: Browser) { | ||
val isFeatureDevAvailable = isFeatureDevAvailable(project) | ||
val isCodeTransformAvailable = isCodeTransformAvailable(project) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ import com.intellij.openapi.project.Project | |
import kotlinx.coroutines.flow.MutableSharedFlow | ||
import kotlinx.coroutines.flow.asSharedFlow | ||
import kotlinx.coroutines.runBlocking | ||
import software.aws.toolkits.jetbrains.services.amazonq.lsp.flareChat.AsyncChatUiListener | ||
import software.aws.toolkits.jetbrains.services.amazonq.lsp.flareChat.ChatCommunicationManager | ||
import software.aws.toolkits.jetbrains.services.amazonq.lsp.flareChat.FlareUiMessage | ||
import software.aws.toolkits.jetbrains.services.amazonq.lsp.model.aws.chat.GENERIC_COMMAND | ||
import software.aws.toolkits.jetbrains.services.amazonq.lsp.model.aws.chat.GenericCommandParams | ||
|
@@ -40,7 +40,8 @@ class ActionRegistrar { | |
val params = SendToPromptParams(selection = codeSelection, triggerType = TriggerType.CONTEXT_MENU) | ||
uiMessage = FlareUiMessage(command = SEND_TO_PROMPT, params = params) | ||
} | ||
AsyncChatUiListener.notifyPartialMessageUpdate(project, uiMessage) | ||
ChatCommunicationManager.getInstance(project).notifyUi(uiMessage) | ||
// AsyncChatUiListener.notifyPartialMessageUpdate(project, uiMessage) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: can be removed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed |
||
} | ||
} | ||
} | ||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should probably just associate the browser with the communication manager
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Browser is not in shared package, and cannot import it. I am able to do it using
Any
, but not sure if it is a right way to do it.