diff --git a/src/common/connectionManager.ts b/src/common/connectionManager.ts index 231837686..edb0b56ea 100644 --- a/src/common/connectionManager.ts +++ b/src/common/connectionManager.ts @@ -64,11 +64,11 @@ export type AnyConnectionState = | ConnectionStateErrored; export interface ConnectionManagerEvents { - "connection-requested": [AnyConnectionState]; - "connection-succeeded": [ConnectionStateConnected]; - "connection-timed-out": [ConnectionStateErrored]; - "connection-closed": [ConnectionStateDisconnected]; - "connection-errored": [ConnectionStateErrored]; + "connection-request": [AnyConnectionState]; + "connection-success": [ConnectionStateConnected]; + "connection-time-out": [ConnectionStateErrored]; + "connection-close": [ConnectionStateDisconnected]; + "connection-error": [ConnectionStateErrored]; } export class ConnectionManager extends EventEmitter { @@ -101,7 +101,7 @@ export class ConnectionManager extends EventEmitter { } async connect(settings: ConnectionSettings): Promise { - this.emit("connection-requested", this.state); + this.emit("connection-request", this.state); if (this.state.tag === "connected" || this.state.tag === "connecting") { await this.disconnect(); @@ -109,6 +109,7 @@ export class ConnectionManager extends EventEmitter { let serviceProvider: NodeDriverServiceProvider; let connectionInfo: ConnectionInfo; + let connectionStringAuthType: ConnectionStringAuthType = "scram"; try { settings = { ...settings }; @@ -137,6 +138,11 @@ export class ConnectionManager extends EventEmitter { connectionInfo.driverOptions.proxy ??= { useEnvironmentVariableProxies: true }; connectionInfo.driverOptions.applyProxyToOIDC ??= true; + connectionStringAuthType = ConnectionManager.inferConnectionTypeFromSettings( + this.userConfig, + connectionInfo + ); + serviceProvider = await NodeDriverServiceProvider.connect( connectionInfo.connectionString, { @@ -149,9 +155,10 @@ export class ConnectionManager extends EventEmitter { ); } catch (error: unknown) { const errorReason = error instanceof Error ? error.message : `${error as string}`; - this.changeState("connection-errored", { + this.changeState("connection-error", { tag: "errored", errorReason, + connectionStringAuthType, connectedAtlasCluster: settings.atlas, }); throw new MongoDBError(ErrorCodes.MisconfiguredConnectionString, errorReason); @@ -162,7 +169,7 @@ export class ConnectionManager extends EventEmitter { if (connectionType.startsWith("oidc")) { void this.pingAndForget(serviceProvider); - return this.changeState("connection-requested", { + return this.changeState("connection-request", { tag: "connecting", connectedAtlasCluster: settings.atlas, serviceProvider, @@ -173,7 +180,7 @@ export class ConnectionManager extends EventEmitter { await serviceProvider?.runCommand?.("admin", { hello: 1 }); - return this.changeState("connection-succeeded", { + return this.changeState("connection-success", { tag: "connected", connectedAtlasCluster: settings.atlas, serviceProvider, @@ -181,9 +188,10 @@ export class ConnectionManager extends EventEmitter { }); } catch (error: unknown) { const errorReason = error instanceof Error ? error.message : `${error as string}`; - this.changeState("connection-errored", { + this.changeState("connection-error", { tag: "errored", errorReason, + connectionStringAuthType, connectedAtlasCluster: settings.atlas, }); throw new MongoDBError(ErrorCodes.NotConnectedToMongoDB, errorReason); @@ -199,7 +207,7 @@ export class ConnectionManager extends EventEmitter { try { await this.state.serviceProvider?.close(true); } finally { - this.changeState("connection-closed", { + this.changeState("connection-close", { tag: "disconnected", }); } @@ -231,7 +239,7 @@ export class ConnectionManager extends EventEmitter { private onOidcAuthSucceeded(): void { if (this.state.tag === "connecting" && this.state.connectionStringAuthType?.startsWith("oidc")) { - this.changeState("connection-succeeded", { ...this.state, tag: "connected" }); + this.changeState("connection-success", { ...this.state, tag: "connected" }); } this.logger.info({ @@ -243,7 +251,7 @@ export class ConnectionManager extends EventEmitter { private onOidcNotifyDeviceFlow(flowInfo: { verificationUrl: string; userCode: string }): void { if (this.state.tag === "connecting" && this.state.connectionStringAuthType?.startsWith("oidc")) { - this.changeState("connection-requested", { + this.changeState("connection-request", { ...this.state, tag: "connecting", connectionStringAuthType: "oidc-device-flow", @@ -317,7 +325,7 @@ export class ConnectionManager extends EventEmitter { message: String(error), }); } finally { - this.changeState("connection-errored", { tag: "errored", errorReason: String(error) }); + this.changeState("connection-error", { tag: "errored", errorReason: String(error) }); } } } diff --git a/src/common/session.ts b/src/common/session.ts index 5080c05a8..87113894c 100644 --- a/src/common/session.ts +++ b/src/common/session.ts @@ -10,6 +10,7 @@ import type { ConnectionManager, ConnectionSettings, ConnectionStateConnected, + ConnectionStateErrored, } from "./connectionManager.js"; import type { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; import { ErrorCodes, MongoDBError } from "./errors.js"; @@ -28,7 +29,7 @@ export type SessionEvents = { connect: []; close: []; disconnect: []; - "connection-error": [string]; + "connection-error": [ConnectionStateErrored]; }; export class Session extends EventEmitter { @@ -66,10 +67,10 @@ export class Session extends EventEmitter { this.apiClient = new ApiClient({ baseUrl: apiBaseUrl, credentials }, logger); this.exportsManager = exportsManager; this.connectionManager = connectionManager; - this.connectionManager.on("connection-succeeded", () => this.emit("connect")); - this.connectionManager.on("connection-timed-out", (error) => this.emit("connection-error", error.errorReason)); - this.connectionManager.on("connection-closed", () => this.emit("disconnect")); - this.connectionManager.on("connection-errored", (error) => this.emit("connection-error", error.errorReason)); + this.connectionManager.on("connection-success", () => this.emit("connect")); + this.connectionManager.on("connection-time-out", (error) => this.emit("connection-error", error)); + this.connectionManager.on("connection-close", () => this.emit("disconnect")); + this.connectionManager.on("connection-error", (error) => this.emit("connection-error", error)); } setMcpClient(mcpClient: Implementation | undefined): void { @@ -136,13 +137,7 @@ export class Session extends EventEmitter { } async connectToMongoDB(settings: ConnectionSettings): Promise { - try { - await this.connectionManager.connect({ ...settings }); - } catch (error: unknown) { - const message = error instanceof Error ? error.message : (error as string); - this.emit("connection-error", message); - throw error; - } + await this.connectionManager.connect({ ...settings }); } get isConnectedToMongoDB(): boolean { diff --git a/src/resources/common/debug.ts b/src/resources/common/debug.ts index a12436f04..ad1f383df 100644 --- a/src/resources/common/debug.ts +++ b/src/resources/common/debug.ts @@ -1,13 +1,13 @@ import { ReactiveResource } from "../resource.js"; import type { Telemetry } from "../../telemetry/telemetry.js"; import type { Session, UserConfig } from "../../lib.js"; +import type { AtlasClusterConnectionInfo, ConnectionStateErrored } from "../../common/connectionManager.js"; type ConnectionStateDebuggingInformation = { readonly tag: "connected" | "connecting" | "disconnected" | "errored"; readonly connectionStringAuthType?: "scram" | "ldap" | "kerberos" | "oidc-auth-flow" | "oidc-device-flow" | "x.509"; - readonly oidcLoginUrl?: string; - readonly oidcUserCode?: string; readonly errorReason?: string; + readonly connectedAtlasCluster?: AtlasClusterConnectionInfo; }; export class DebugResource extends ReactiveResource< @@ -35,15 +35,21 @@ export class DebugResource extends ReactiveResource< } reduce( eventName: "connect" | "disconnect" | "close" | "connection-error", - event: string | undefined + event: ConnectionStateErrored | undefined ): ConnectionStateDebuggingInformation { - void event; - switch (eventName) { case "connect": return { tag: "connected" }; - case "connection-error": - return { tag: "errored", errorReason: event }; + case "connection-error": { + return { + tag: "errored", + connectionStringAuthType: event?.connectionStringAuthType, + connectedAtlasCluster: event?.connectedAtlasCluster, + errorReason: + event?.errorReason ?? + "Could not find a reason. This might be a bug in the MCP Server. Please open an issue in https://github.com/mongodb-js/mongodb-mcp-server.", + }; + } case "disconnect": case "close": return { tag: "disconnected" }; @@ -59,6 +65,13 @@ export class DebugResource extends ReactiveResource< break; case "errored": result += `The user is not connected to a MongoDB cluster because of an error.\n`; + if (this.current.connectedAtlasCluster) { + result += `Attempted connecting to Atlas Cluster "${this.current.connectedAtlasCluster.clusterName}" in project with id "${this.current.connectedAtlasCluster.projectId}".\n`; + } + + if (this.current.connectionStringAuthType !== undefined) { + result += `The inferred authentication mechanism is "${this.current.connectionStringAuthType}".\n`; + } result += `${this.current.errorReason}`; break; case "connecting": diff --git a/tests/integration/common/connectionManager.oidc.test.ts b/tests/integration/common/connectionManager.oidc.test.ts index e3a406eb6..fe65e63d6 100644 --- a/tests/integration/common/connectionManager.oidc.test.ts +++ b/tests/integration/common/connectionManager.oidc.test.ts @@ -128,7 +128,7 @@ describe.skipIf(process.platform !== "linux")("ConnectionManager OIDC Tests", as await connectionManager.disconnect(); // for testing, force disconnecting AND setting the connection to closed to reset the // state of the connection manager - connectionManager.changeState("connection-closed", { tag: "disconnected" }); + connectionManager.changeState("connection-close", { tag: "disconnected" }); await integration.connectMcpClient(); }, DEFAULT_TIMEOUT); diff --git a/tests/integration/common/connectionManager.test.ts b/tests/integration/common/connectionManager.test.ts index 40386fcde..5a8cb6dae 100644 --- a/tests/integration/common/connectionManager.test.ts +++ b/tests/integration/common/connectionManager.test.ts @@ -19,27 +19,27 @@ describeWithMongoDB("Connection Manager", (integration) => { await connectionManager().disconnect(); // for testing, force disconnecting AND setting the connection to closed to reset the // state of the connection manager - connectionManager().changeState("connection-closed", { tag: "disconnected" }); + connectionManager().changeState("connection-close", { tag: "disconnected" }); }); describe("when successfully connected", () => { type ConnectionManagerSpies = { - "connection-requested": (event: ConnectionManagerEvents["connection-requested"][0]) => void; - "connection-succeeded": (event: ConnectionManagerEvents["connection-succeeded"][0]) => void; - "connection-timed-out": (event: ConnectionManagerEvents["connection-timed-out"][0]) => void; - "connection-closed": (event: ConnectionManagerEvents["connection-closed"][0]) => void; - "connection-errored": (event: ConnectionManagerEvents["connection-errored"][0]) => void; + "connection-request": (event: ConnectionManagerEvents["connection-request"][0]) => void; + "connection-success": (event: ConnectionManagerEvents["connection-success"][0]) => void; + "connection-time-out": (event: ConnectionManagerEvents["connection-time-out"][0]) => void; + "connection-close": (event: ConnectionManagerEvents["connection-close"][0]) => void; + "connection-error": (event: ConnectionManagerEvents["connection-error"][0]) => void; }; let connectionManagerSpies: ConnectionManagerSpies; beforeEach(async () => { connectionManagerSpies = { - "connection-requested": vi.fn(), - "connection-succeeded": vi.fn(), - "connection-timed-out": vi.fn(), - "connection-closed": vi.fn(), - "connection-errored": vi.fn(), + "connection-request": vi.fn(), + "connection-success": vi.fn(), + "connection-time-out": vi.fn(), + "connection-close": vi.fn(), + "connection-error": vi.fn(), }; for (const [event, spy] of Object.entries(connectionManagerSpies)) { @@ -62,11 +62,11 @@ describeWithMongoDB("Connection Manager", (integration) => { }); it("should notify that the connection was requested", () => { - expect(connectionManagerSpies["connection-requested"]).toHaveBeenCalledOnce(); + expect(connectionManagerSpies["connection-request"]).toHaveBeenCalledOnce(); }); it("should notify that the connection was successful", () => { - expect(connectionManagerSpies["connection-succeeded"]).toHaveBeenCalledOnce(); + expect(connectionManagerSpies["connection-success"]).toHaveBeenCalledOnce(); }); describe("when disconnects", () => { @@ -75,7 +75,7 @@ describeWithMongoDB("Connection Manager", (integration) => { }); it("should notify that it was disconnected before connecting", () => { - expect(connectionManagerSpies["connection-closed"]).toHaveBeenCalled(); + expect(connectionManagerSpies["connection-close"]).toHaveBeenCalled(); }); it("should be marked explicitly as disconnected", () => { @@ -91,11 +91,11 @@ describeWithMongoDB("Connection Manager", (integration) => { }); it("should notify that it was disconnected before connecting", () => { - expect(connectionManagerSpies["connection-closed"]).toHaveBeenCalled(); + expect(connectionManagerSpies["connection-close"]).toHaveBeenCalled(); }); it("should notify that it was connected again", () => { - expect(connectionManagerSpies["connection-succeeded"]).toHaveBeenCalled(); + expect(connectionManagerSpies["connection-success"]).toHaveBeenCalled(); }); it("should be marked explicitly as connected", () => { @@ -115,11 +115,53 @@ describeWithMongoDB("Connection Manager", (integration) => { }); it("should notify that it was disconnected before connecting", () => { - expect(connectionManagerSpies["connection-closed"]).toHaveBeenCalled(); + expect(connectionManagerSpies["connection-close"]).toHaveBeenCalled(); }); it("should notify that it failed connecting", () => { - expect(connectionManagerSpies["connection-errored"]).toHaveBeenCalled(); + expect(connectionManagerSpies["connection-error"]).toHaveBeenCalledWith({ + tag: "errored", + connectedAtlasCluster: undefined, + connectionStringAuthType: "scram", + errorReason: "Unable to parse localhost:xxxxx with URL", + }); + }); + + it("should be marked explicitly as connected", () => { + expect(connectionManager().currentConnectionState.tag).toEqual("errored"); + }); + }); + + describe("when fails to connect to a new atlas cluster", () => { + const atlas = { + username: "", + projectId: "", + clusterName: "My Atlas Cluster", + expiryDate: new Date(), + }; + + beforeEach(async () => { + try { + await connectionManager().connect({ + connectionString: "mongodb://localhost:xxxxx", + atlas, + }); + } catch (_error: unknown) { + void _error; + } + }); + + it("should notify that it was disconnected before connecting", () => { + expect(connectionManagerSpies["connection-close"]).toHaveBeenCalled(); + }); + + it("should notify that it failed connecting", () => { + expect(connectionManagerSpies["connection-error"]).toHaveBeenCalledWith({ + tag: "errored", + connectedAtlasCluster: atlas, + connectionStringAuthType: "scram", + errorReason: "Unable to parse localhost:xxxxx with URL", + }); }); it("should be marked explicitly as connected", () => { diff --git a/tests/unit/resources/common/debug.test.ts b/tests/unit/resources/common/debug.test.ts index 0292a726f..52f1dd826 100644 --- a/tests/unit/resources/common/debug.test.ts +++ b/tests/unit/resources/common/debug.test.ts @@ -47,10 +47,51 @@ describe("debug resource", () => { }); it("should be disconnected and contain an error when an error event occurred", () => { - debugResource.reduceApply("connection-error", "Error message from the server"); + debugResource.reduceApply("connection-error", { + tag: "errored", + errorReason: "Error message from the server", + }); + + const output = debugResource.toOutput(); + + expect(output).toContain(`The user is not connected to a MongoDB cluster because of an error.`); + expect(output).toContain(`Error message from the server`); + }); + + it("should show the inferred authentication type", () => { + debugResource.reduceApply("connection-error", { + tag: "errored", + connectionStringAuthType: "scram", + errorReason: "Error message from the server", + }); + + const output = debugResource.toOutput(); + + expect(output).toContain(`The user is not connected to a MongoDB cluster because of an error.`); + expect(output).toContain(`The inferred authentication mechanism is "scram".`); + expect(output).toContain(`Error message from the server`); + }); + + it("should show the atlas cluster information when provided", () => { + debugResource.reduceApply("connection-error", { + tag: "errored", + connectionStringAuthType: "scram", + errorReason: "Error message from the server", + connectedAtlasCluster: { + clusterName: "My Test Cluster", + projectId: "COFFEEFABADA", + username: "", + expiryDate: new Date(), + }, + }); + const output = debugResource.toOutput(); expect(output).toContain(`The user is not connected to a MongoDB cluster because of an error.`); + expect(output).toContain( + `Attempted connecting to Atlas Cluster "My Test Cluster" in project with id "COFFEEFABADA".` + ); + expect(output).toContain(`The inferred authentication mechanism is "scram".`); expect(output).toContain(`Error message from the server`); }); });