Skip to content

feat: suggest the config connection string on connect failure #75

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 1 commit into from
Apr 16, 2025
Merged

feat: suggest the config connection string on connect failure #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
31 changes: 27 additions & 4 deletions src/tools/mongodb/metadata/connect.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: alternatively you can override handleError here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately, it's a bit more complicated since I wanted to have the extra check comparing the connection string we tried connecting to to the connection string in the config. handleError right now doesn't supply the original args used to invoke the tool and even if it did, in the case of connecting by cluster name, we wouldn't know what the resolved connection string is.

// 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;
}
}
}
32 changes: 32 additions & 0 deletions tests/integration/tools/mongodb/metadata/connect.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,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");
});
});
});
Expand All @@ -83,5 +86,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");
});
});
});
});
Loading