Skip to content
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

Make command handling in Q chat webview async #35

Merged
merged 2 commits into from
Sep 25, 2024
Merged

Make command handling in Q chat webview async #35

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8abd31
Make command handling in Q chat webview async
breedloj Sep 25, 2024
e0ef659
Remove unnecessary nested thread
breedloj Sep 25, 2024
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
58 changes: 29 additions & 29 deletions plugin/src/software/aws/toolkits/eclipse/amazonq/chat/ChatCommunicationManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

package software.aws.toolkits.eclipse.amazonq.chat;

import java.util.concurrent.CompletableFuture;

import org.eclipse.swt.browser.Browser;

import software.aws.toolkits.eclipse.amazonq.chat.models.ChatRequestParams;
Expand All @@ -10,51 +12,49 @@
import software.aws.toolkits.eclipse.amazonq.chat.models.GenericTabParams;
import software.aws.toolkits.eclipse.amazonq.exception.AmazonQPluginException;
import software.aws.toolkits.eclipse.amazonq.util.JsonHandler;
import software.aws.toolkits.eclipse.amazonq.util.PluginLogger;
import software.aws.toolkits.eclipse.amazonq.views.model.Command;

public final class ChatCommunicationManager {

private final JsonHandler jsonHandler;
private final ChatMessageProvider chatMessageProvider;
private final CompletableFuture<ChatMessageProvider> chatMessageProvider;

public ChatCommunicationManager() {
this.jsonHandler = new JsonHandler();
this.chatMessageProvider = new ChatMessageProvider();
this.chatMessageProvider = ChatMessageProvider.createAsync();
}

/*
* Sends a message to the Amazon Q LSP server.
*/
public ChatResult sendMessageToChatServer(final Command command, final Object params) {

switch (command) {
case CHAT_SEND_PROMPT:
ChatRequestParams chatRequestParams = jsonHandler.convertObject(params, ChatRequestParams.class);
ChatResult result = chatMessageProvider.sendChatPrompt(chatRequestParams);
return result;
case CHAT_READY:
chatMessageProvider.sendChatReady();
return null;
case CHAT_TAB_ADD:
GenericTabParams tabParams = jsonHandler.convertObject(params, GenericTabParams.class);
chatMessageProvider.sendTabAdd(tabParams);
return null;
default:
throw new AmazonQPluginException("Unhandled command in ChatCommunicationManager: " + command.toString());
}
public CompletableFuture<ChatResult> sendMessageToChatServer(final Command command, final Object params) {
return chatMessageProvider.thenCompose(chatMessageProvider -> {
try {
switch (command) {
case CHAT_SEND_PROMPT:
ChatRequestParams chatRequestParams = jsonHandler.convertObject(params, ChatRequestParams.class);
return chatMessageProvider.sendChatPrompt(chatRequestParams);
case CHAT_READY:
chatMessageProvider.sendChatReady();
return CompletableFuture.completedFuture(null);
case CHAT_TAB_ADD:
GenericTabParams tabParams = jsonHandler.convertObject(params, GenericTabParams.class);
chatMessageProvider.sendTabAdd(tabParams);
return CompletableFuture.completedFuture(null);
default:
throw new AmazonQPluginException("Unhandled command in ChatCommunicationManager: " + command.toString());
}
} catch (Exception e) {
PluginLogger.error("Error occurred in sendMessageToChatServer", e);
return CompletableFuture.failedFuture(new AmazonQPluginException(e));
}
});
}

/**
* Sends a message to the webview.
*
* See handlers in Flare chat-client:
* https://github.com/aws/language-servers/blob/9226fb4ed10dc54f1719b14a5b1dac1807641f79/chat-client/src/client/chat.ts#L67-L101
*/
public void sendMessageToChatUI(final Browser browser, final ChatUIInboundCommand command) {
String message = this.jsonHandler.serialize(command);
String script = "window.postMessage(" + message + ");";
browser.getDisplay().asyncExec(() -> {
browser.evaluate(script);
});
}
}

}
28 changes: 14 additions & 14 deletions plugin/src/software/aws/toolkits/eclipse/amazonq/chat/ChatMessageProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

package software.aws.toolkits.eclipse.amazonq.chat;

import java.util.concurrent.ExecutionException;
import java.util.concurrent.CompletableFuture;

import software.aws.toolkits.eclipse.amazonq.chat.models.ChatRequestParams;
import software.aws.toolkits.eclipse.amazonq.chat.models.ChatResult;
Expand All @@ -15,25 +15,24 @@

public final class ChatMessageProvider {

private AmazonQLspServer amazonQLspServer;
private final AmazonQLspServer amazonQLspServer;

public ChatMessageProvider() {
try {
amazonQLspServer = LspProvider.getAmazonQServer().get();
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this the root cause of the deadlock between the LSP and Client startup?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this blocking call was preventing download and startup from happening.

} catch (InterruptedException | ExecutionException e) {
PluginLogger.error("Error occurred while retrieving Amazon Q LSP server. Failed to instantiate ChatMessageProvider.", e);
throw new AmazonQPluginException(e);
}
public static CompletableFuture<ChatMessageProvider> createAsync() {
return LspProvider.getAmazonQServer()
.thenApply(ChatMessageProvider::new);
Copy link
Contributor

@dingfeli dingfeli Sep 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This looks to be the root of the change. Not entirely familiar with how JVM handles async but is thenApply thread safe somehow? How does it prevent deadlock in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thenApply runs once the future is resolved. Primary thing here is to avoid a blocking call in the constructor run on the main thread. This pattern gives consumers a way to safely construct an instance in an async flow by making the resolution explicit.

Copy link
Contributor

Choose a reason for hiding this comment

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

ahh I see. That makes sense.

}

private ChatMessageProvider(AmazonQLspServer amazonQLspServer) {
this.amazonQLspServer = amazonQLspServer;
}

public ChatResult sendChatPrompt(final ChatRequestParams chatRequestParams) {
public CompletableFuture<ChatResult> sendChatPrompt(final ChatRequestParams chatRequestParams) {
try {
PluginLogger.info("Sending " + Command.CHAT_SEND_PROMPT + " message to Amazon Q LSP server");
ChatResult chatResult = amazonQLspServer.sendChatPrompt(chatRequestParams).get();
return chatResult;
} catch (InterruptedException | ExecutionException e) {
return amazonQLspServer.sendChatPrompt(chatRequestParams);
} catch (Exception e) {
PluginLogger.error("Error occurred while sending " + Command.CHAT_SEND_PROMPT + " message to Amazon Q LSP server", e);
throw new AmazonQPluginException(e);
return CompletableFuture.failedFuture(new AmazonQPluginException(e));
}
}

Expand All @@ -56,4 +55,5 @@ public void sendTabAdd(final GenericTabParams tabParams) {
throw new AmazonQPluginException(e);
}
}

}
1 change: 0 additions & 1 deletion ...c/software/aws/toolkits/eclipse/amazonq/lsp/manager/fetcher/RemoteManifestLspFetcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ public final class RemoteManifestLspFetcher implements LspFetcher {
private static final int TIMEOUT_SECONDS = 10;
private static final ObjectMapper OBJECT_MAPPER = ObjectMapperFactory.getInstance();


private final String manifestUrl;
private final VersionRange versionRange;
private final boolean integrityChecking;
Expand Down
55 changes: 29 additions & 26 deletions plugin/src/software/aws/toolkits/eclipse/amazonq/views/AmazonQChatViewActionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@

import software.aws.toolkits.eclipse.amazonq.chat.ChatCommunicationManager;
import software.aws.toolkits.eclipse.amazonq.chat.models.ChatRequestParams;
import software.aws.toolkits.eclipse.amazonq.chat.models.ChatResult;
import software.aws.toolkits.eclipse.amazonq.chat.models.ChatUIInboundCommand;
import software.aws.toolkits.eclipse.amazonq.chat.models.ChatUIInboundCommandName;
import software.aws.toolkits.eclipse.amazonq.exception.AmazonQPluginException;
import software.aws.toolkits.eclipse.amazonq.util.JsonHandler;
import software.aws.toolkits.eclipse.amazonq.util.PluginLogger;
import software.aws.toolkits.eclipse.amazonq.util.ThreadingUtils;
import software.aws.toolkits.eclipse.amazonq.views.model.Command;
import software.aws.toolkits.eclipse.amazonq.views.model.ParsedCommand;

Expand All @@ -35,30 +35,33 @@ public final void handleCommand(final ParsedCommand parsedCommand, final Browser

PluginLogger.info(command + " being processed by ActionHandler");

switch (command) {
case CHAT_SEND_PROMPT:
ChatResult chatResult = chatCommunicationManager.sendMessageToChatServer(command, params);

ChatRequestParams chatRequestParams = jsonHandler.convertObject(params, ChatRequestParams.class);
ChatUIInboundCommand chatUIInboundCommand = new ChatUIInboundCommand(
ChatUIInboundCommandName.ChatPrompt.toString(),
chatRequestParams.tabId(),
chatResult,
false
);
chatCommunicationManager.sendMessageToChatUI(browser, chatUIInboundCommand);
break;
case CHAT_READY:
chatCommunicationManager.sendMessageToChatServer(command, params);
break;
case CHAT_TAB_ADD:
chatCommunicationManager.sendMessageToChatServer(command, params);
break;
case TELEMETRY_EVENT:
break;
default:
throw new AmazonQPluginException("Unhandled command in AmazonQChatViewActionHandler: " + command.toString());
}
ThreadingUtils.executeAsyncTask(() -> {
switch (command) {
case CHAT_SEND_PROMPT:
chatCommunicationManager.sendMessageToChatServer(command, params)
.thenAccept(chatResult -> {
ChatRequestParams chatRequestParams = jsonHandler.convertObject(params, ChatRequestParams.class);
ChatUIInboundCommand chatUIInboundCommand = new ChatUIInboundCommand(
ChatUIInboundCommandName.ChatPrompt.toString(),
chatRequestParams.tabId(),
chatResult,
false
);
chatCommunicationManager.sendMessageToChatUI(browser, chatUIInboundCommand);
});
break;
case CHAT_READY:
chatCommunicationManager.sendMessageToChatServer(command, params);
break;
case CHAT_TAB_ADD:
chatCommunicationManager.sendMessageToChatServer(command, params);
break;
case TELEMETRY_EVENT:
break;
default:
throw new AmazonQPluginException("Unhandled command in AmazonQChatViewActionHandler: " + command.toString());
}
});
}

}
}