feat: suggest using the config connection string on connection failure

Author: nirinchev

src/tools/mongodb/metadata/connect.ts

@@ -4,6 +4,7 @@ import { DbOperationType, MongoDBToolBase } from "../mongodbTool.js";
 import { ToolArgs } from "../../tool.js";
 import { ErrorCodes, MongoDBError } from "../../../errors.js";
 import config from "../../../config.js";
+import { MongoError as DriverError } from "mongodb";
 
 export class ConnectTool extends MongoDBToolBase {
     protected name = "connect";
@@ -57,10 +58,32 @@ export class ConnectTool extends MongoDBToolBase {
             throw new MongoDBError(ErrorCodes.InvalidParams, "Invalid connection options");
         }
 
-        await this.connectToMongoDB(connectionString);
+        try {
+            await this.connectToMongoDB(connectionString);
+            return {
+                content: [{ type: "text", text: `Successfully connected to ${connectionString}.` }],
+            };
+        } catch (error) {
+            // Sometimes the model will supply an incorrect connection string. If the user has configured
+            // a different one as environment variable or a cli argument, suggest using that one instead.
+            if (
+                config.connectionString &&
+                error instanceof DriverError &&
+                config.connectionString !== connectionString
+            ) {
+                return {
+                    content: [
+                        {
+                            type: "text",
+                            text:
+                                `Failed to connect to MongoDB at '${connectionString}' due to error: '${error.message}.` +
+                                `Your config lists a different connection string: '${config.connectionString}' - do you want to try connecting to it instead?`,
+                        },
+                    ],
+                };
+            }
 
-        return {
-            content: [{ type: "text", text: `Successfully connected to ${connectionString}.` }],
-        };
+            throw error;
+        }
     }
 }

tests/integration/tools/mongodb/metadata/connect.test.ts

@@ -77,6 +77,9 @@ describe("Connect tool", () => {
                 });
                 const content = getResponseContent(response.content);
                 expect(content).toContain("Error running connect");
+
+                // Should not suggest using the config connection string (because we don't have one)
+                expect(content).not.toContain("Your config lists a different connection string");
             });
         });
     });
@@ -103,5 +106,34 @@ describe("Connect tool", () => {
             expect(content).toContain("Successfully connected");
             expect(content).toContain(newConnectionString);
         });
+
+        describe("when the arugment connection string is invalid", () => {
+            it("suggests the config connection string if set", async () => {
+                const response = await client.callTool({
+                    name: "connect",
+                    arguments: { connectionStringOrClusterName: "mongodb://localhost:12345" },
+                });
+                const content = getResponseContent(response.content);
+                expect(content).toContain("Failed to connect to MongoDB at 'mongodb://localhost:12345'");
+                expect(content).toContain(
+                    `Your config lists a different connection string: '${config.connectionString}' - do you want to try connecting to it instead?`
+                );
+            });
+
+            it("returns error message if the config connection string matches the argument", async () => {
+                config.connectionString = "mongodb://localhost:12345";
+                const response = await client.callTool({
+                    name: "connect",
+                    arguments: { connectionStringOrClusterName: "mongodb://localhost:12345" },
+                });
+
+                const content = getResponseContent(response.content);
+
+                // Should be handled by default error handler and not suggest the config connection string
+                // because it matches the argument connection string
+                expect(content).toContain("Error running connect");
+                expect(content).not.toContain("Your config lists a different connection string");
+            });
+        });
     });
 });