DevoxxGenieIDEAPlugin icon indicating copy to clipboard operation
DevoxxGenieIDEAPlugin copied to clipboard

[ENHANCE] MCP testing should timeout if command doesn't exist

Open stephanj opened this issue 9 months ago • 2 comments

The "Test Connection & Fetch Tools" should timeout when the command or arguments are invalid. Currently it freezes the IDEA !

Image

stephanj avatar Mar 24 '25 09:03 stephanj

GPT 4o investigation:

The issue with the "Test Connection & Fetch Tools" feature, which causes the application to freeze, likely arises because the testConnection() method is performed on the Event Dispatch Thread (EDT). Activities like establishing network connections and fetching data should not be done on the EDT because they can block the UI, leading to the IDE appearing to be frozen.

To address this, you should offload long-running tasks to a separate thread. You can achieve this by leveraging a SwingWorker, a utility provided by Swing for handling background tasks, or by using a plain Java thread.

Suggested Fix

Here's how you might modify the testConnection() method to run in a background thread using SwingWorker:

import javax.swing.SwingWorker;

private void testConnection() {
    // Get the current transport panel
    MCPServer.TransportType selectedTransport = (MCPServer.TransportType) transportTypeCombo.getSelectedItem();
    TransportPanel panel = transportPanels.get(selectedTransport);

    if (panel == null) {
        return;
    }

    // Validate required fields
    if (!panel.isValid()) {
        ErrorDialogUtil.showErrorDialog(
                getContentPanel(),
                "Validation Error",
                "Please correct the following error before testing the connection:\n" + panel.getErrorMessage(),
                null
        );
        return;
    }

    // Disable button and show connecting status
    testConnectionButton.setEnabled(false);
    testConnectionButton.setText("Connecting...");

    // Use SwingWorker for background task
    new SwingWorker<Void, Void>() {
        @Override
        protected Void doInBackground() {
            try {
                // Create client
                McpClient mcpClient = panel.createClient();

                // Get available tools
                List<ToolSpecification> tools = mcpClient.listTools();

                // Create server builder
                MCPServer.MCPServerBuilder builder = MCPServer.builder()
                        .name(nameField.getText().trim());

                // Apply settings from transport panel
                panel.applySettings(builder);

                // Process tool specifications
                panel.processTools(tools, builder);

                // Build the server
                if (existingServer != null) {
                    // Preserve environment variables from existing server
                    builder.env(new HashMap<>(existingServer.getEnv()));
                }

                // Save the server
                server = builder.build();

                // Close the client
                mcpClient.close();
                
                SwingUtilities.invokeLater(() -> {
                    // Show success message
                    testConnectionButton.setText("Connection Successful! " + tools.size() + " tools found");
                });

            } catch (Exception ex) {
                log.error("Failed to connect to MCP server", ex);

                // Show error message
                SwingUtilities.invokeLater(() -> {
                    ErrorDialogUtil.showErrorDialog(
                            getContentPanel(),
                            "Connection Failed",
                            "Failed to connect to MCP server",
                            ex
                    );

                    testConnectionButton.setText("Connection Failed - Try Again");
                });
            } finally {
                SwingUtilities.invokeLater(() -> {
                    testConnectionButton.setEnabled(true);
                });
            }
            return null;
        }
    }.execute();
}

Explanation

  • SwingWorker: This utility class allows you to perform long-running tasks in the background, keeping the UI responsive.
  • doInBackground(): The method where the time-intensive logic occurs, running in a worker thread, not blocking the EDT.
  • SwingUtilities.invokeLater(): Ensures updates to the GUI components are executed on the EDT, maintaining thread safety in a Swing application.

By implementing the above fix, the IDE should remain responsive while the connection test is executed in the background.

stephanj avatar Apr 04 '25 11:04 stephanj

Claude Sonnet :

Based on the code review, I've identified the root cause of the bug:

  1. In the MCPServerDialog.java, the testConnection() method attempts to create and connect to an MCP server but does not have any explicit timeout handling.

  2. For STDIO transport, it uses StdioMcpTransport from the dev.langchain4j.mcp.client.transport.stdio package, but it doesn't set a timeout for process execution.

  3. When an invalid command or arguments are provided, the underlying process may hang indefinitely, causing the UI to freeze.

Here's a summary of the changes made:

  1. Added timeout settings to both transport implementations:

    • Added a 30-second timeout to StdioMcpTransport to prevent indefinite hanging when executing commands.
    • Standardized the timeout for HttpMcpTransport from 60 seconds to 30 seconds for consistency.
  2. Improved the command existence check:

    • Added a 5-second timeout to the command existence check in testIfCommandExists method to prevent freezing when checking invalid commands.
    • Added proper cleanup for the process if the timeout occurs.
  3. Used SwingWorker for background processing:

    • Modified the testConnection method in MCPServerDialog to use SwingWorker to run the connection test in a background thread.
    • This ensures the UI remains responsive even if the connection test takes time or fails.
    • The UI will be updated properly once the background task completes or times out.

These changes ensure that when an invalid command or arguments are provided, the operation will properly timeout rather than freezing the IDEA IDE. The timeouts are reasonable (30 seconds for connection, 5 seconds for command verification) to provide a good balance between allowing legitimate operations to complete and preventing indefinite hangs.

stephanj avatar Apr 04 '25 11:04 stephanj