Skip to content

Commit a06b443

Browse files
kmruizCopilot
kmruiz
and
Copilot
authored
chore: add more information when there is a failure connecting MCP-92 (#475)
Co-authored-by: Copilot <[email protected]>
1 parent cd5cf37 commit a06b443

File tree

6 files changed

+152
-53
lines changed

6 files changed

+152
-53
lines changed

‎src/common/connectionManager.ts

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,11 @@ export type AnyConnectionState =
6464
| ConnectionStateErrored;
6565

6666
export interface ConnectionManagerEvents {
67-
"connection-requested": [AnyConnectionState];
68-
"connection-succeeded": [ConnectionStateConnected];
69-
"connection-timed-out": [ConnectionStateErrored];
70-
"connection-closed": [ConnectionStateDisconnected];
71-
"connection-errored": [ConnectionStateErrored];
67+
"connection-request": [AnyConnectionState];
68+
"connection-success": [ConnectionStateConnected];
69+
"connection-time-out": [ConnectionStateErrored];
70+
"connection-close": [ConnectionStateDisconnected];
71+
"connection-error": [ConnectionStateErrored];
7272
}
7373

7474
export class ConnectionManager extends EventEmitter<ConnectionManagerEvents> {
@@ -101,14 +101,15 @@ export class ConnectionManager extends EventEmitter<ConnectionManagerEvents> {
101101
}
102102

103103
async connect(settings: ConnectionSettings): Promise<AnyConnectionState> {
104-
this.emit("connection-requested", this.state);
104+
this.emit("connection-request", this.state);
105105

106106
if (this.state.tag === "connected" || this.state.tag === "connecting") {
107107
await this.disconnect();
108108
}
109109

110110
let serviceProvider: NodeDriverServiceProvider;
111111
let connectionInfo: ConnectionInfo;
112+
let connectionStringAuthType: ConnectionStringAuthType = "scram";
112113

113114
try {
114115
settings = { ...settings };
@@ -137,6 +138,11 @@ export class ConnectionManager extends EventEmitter<ConnectionManagerEvents> {
137138
connectionInfo.driverOptions.proxy ??= { useEnvironmentVariableProxies: true };
138139
connectionInfo.driverOptions.applyProxyToOIDC ??= true;
139140

141+
connectionStringAuthType = ConnectionManager.inferConnectionTypeFromSettings(
142+
this.userConfig,
143+
connectionInfo
144+
);
145+
140146
serviceProvider = await NodeDriverServiceProvider.connect(
141147
connectionInfo.connectionString,
142148
{
@@ -149,9 +155,10 @@ export class ConnectionManager extends EventEmitter<ConnectionManagerEvents> {
149155
);
150156
} catch (error: unknown) {
151157
const errorReason = error instanceof Error ? error.message : `${error as string}`;
152-
this.changeState("connection-errored", {
158+
this.changeState("connection-error", {
153159
tag: "errored",
154160
errorReason,
161+
connectionStringAuthType,
155162
connectedAtlasCluster: settings.atlas,
156163
});
157164
throw new MongoDBError(ErrorCodes.MisconfiguredConnectionString, errorReason);
@@ -162,7 +169,7 @@ export class ConnectionManager extends EventEmitter<ConnectionManagerEvents> {
162169
if (connectionType.startsWith("oidc")) {
163170
void this.pingAndForget(serviceProvider);
164171

165-
return this.changeState("connection-requested", {
172+
return this.changeState("connection-request", {
166173
tag: "connecting",
167174
connectedAtlasCluster: settings.atlas,
168175
serviceProvider,
@@ -173,17 +180,18 @@ export class ConnectionManager extends EventEmitter<ConnectionManagerEvents> {
173180

174181
await serviceProvider?.runCommand?.("admin", { hello: 1 });
175182

176-
return this.changeState("connection-succeeded", {
183+
return this.changeState("connection-success", {
177184
tag: "connected",
178185
connectedAtlasCluster: settings.atlas,
179186
serviceProvider,
180187
connectionStringAuthType: connectionType,
181188
});
182189
} catch (error: unknown) {
183190
const errorReason = error instanceof Error ? error.message : `${error as string}`;
184-
this.changeState("connection-errored", {
191+
this.changeState("connection-error", {
185192
tag: "errored",
186193
errorReason,
194+
connectionStringAuthType,
187195
connectedAtlasCluster: settings.atlas,
188196
});
189197
throw new MongoDBError(ErrorCodes.NotConnectedToMongoDB, errorReason);
@@ -199,7 +207,7 @@ export class ConnectionManager extends EventEmitter<ConnectionManagerEvents> {
199207
try {
200208
await this.state.serviceProvider?.close(true);
201209
} finally {
202-
this.changeState("connection-closed", {
210+
this.changeState("connection-close", {
203211
tag: "disconnected",
204212
});
205213
}
@@ -231,7 +239,7 @@ export class ConnectionManager extends EventEmitter<ConnectionManagerEvents> {
231239

232240
private onOidcAuthSucceeded(): void {
233241
if (this.state.tag === "connecting" && this.state.connectionStringAuthType?.startsWith("oidc")) {
234-
this.changeState("connection-succeeded", { ...this.state, tag: "connected" });
242+
this.changeState("connection-success", { ...this.state, tag: "connected" });
235243
}
236244

237245
this.logger.info({
@@ -243,7 +251,7 @@ export class ConnectionManager extends EventEmitter<ConnectionManagerEvents> {
243251

244252
private onOidcNotifyDeviceFlow(flowInfo: { verificationUrl: string; userCode: string }): void {
245253
if (this.state.tag === "connecting" && this.state.connectionStringAuthType?.startsWith("oidc")) {
246-
this.changeState("connection-requested", {
254+
this.changeState("connection-request", {
247255
...this.state,
248256
tag: "connecting",
249257
connectionStringAuthType: "oidc-device-flow",
@@ -317,7 +325,7 @@ export class ConnectionManager extends EventEmitter<ConnectionManagerEvents> {
317325
message: String(error),
318326
});
319327
} finally {
320-
this.changeState("connection-errored", { tag: "errored", errorReason: String(error) });
328+
this.changeState("connection-error", { tag: "errored", errorReason: String(error) });
321329
}
322330
}
323331
}

‎src/common/session.ts

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import type {
1010
ConnectionManager,
1111
ConnectionSettings,
1212
ConnectionStateConnected,
13+
ConnectionStateErrored,
1314
} from "./connectionManager.js";
1415
import type { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver";
1516
import { ErrorCodes, MongoDBError } from "./errors.js";
@@ -28,7 +29,7 @@ export type SessionEvents = {
2829
connect: [];
2930
close: [];
3031
disconnect: [];
31-
"connection-error": [string];
32+
"connection-error": [ConnectionStateErrored];
3233
};
3334

3435
export class Session extends EventEmitter<SessionEvents> {
@@ -66,10 +67,10 @@ export class Session extends EventEmitter<SessionEvents> {
6667
this.apiClient = new ApiClient({ baseUrl: apiBaseUrl, credentials }, logger);
6768
this.exportsManager = exportsManager;
6869
this.connectionManager = connectionManager;
69-
this.connectionManager.on("connection-succeeded", () => this.emit("connect"));
70-
this.connectionManager.on("connection-timed-out", (error) => this.emit("connection-error", error.errorReason));
71-
this.connectionManager.on("connection-closed", () => this.emit("disconnect"));
72-
this.connectionManager.on("connection-errored", (error) => this.emit("connection-error", error.errorReason));
70+
this.connectionManager.on("connection-success", () => this.emit("connect"));
71+
this.connectionManager.on("connection-time-out", (error) => this.emit("connection-error", error));
72+
this.connectionManager.on("connection-close", () => this.emit("disconnect"));
73+
this.connectionManager.on("connection-error", (error) => this.emit("connection-error", error));
7374
}
7475

7576
setMcpClient(mcpClient: Implementation | undefined): void {
@@ -136,13 +137,7 @@ export class Session extends EventEmitter<SessionEvents> {
136137
}
137138

138139
async connectToMongoDB(settings: ConnectionSettings): Promise<void> {
139-
try {
140-
await this.connectionManager.connect({ ...settings });
141-
} catch (error: unknown) {
142-
const message = error instanceof Error ? error.message : (error as string);
143-
this.emit("connection-error", message);
144-
throw error;
145-
}
140+
await this.connectionManager.connect({ ...settings });
146141
}
147142

148143
get isConnectedToMongoDB(): boolean {

‎src/resources/common/debug.ts

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import { ReactiveResource } from "../resource.js";
22
import type { Telemetry } from "../../telemetry/telemetry.js";
33
import type { Session, UserConfig } from "../../lib.js";
4+
import type { AtlasClusterConnectionInfo, ConnectionStateErrored } from "../../common/connectionManager.js";
45

56
type ConnectionStateDebuggingInformation = {
67
readonly tag: "connected" | "connecting" | "disconnected" | "errored";
78
readonly connectionStringAuthType?: "scram" | "ldap" | "kerberos" | "oidc-auth-flow" | "oidc-device-flow" | "x.509";
8-
readonly oidcLoginUrl?: string;
9-
readonly oidcUserCode?: string;
109
readonly errorReason?: string;
10+
readonly connectedAtlasCluster?: AtlasClusterConnectionInfo;
1111
};
1212

1313
export class DebugResource extends ReactiveResource<
@@ -35,15 +35,21 @@ export class DebugResource extends ReactiveResource<
3535
}
3636
reduce(
3737
eventName: "connect" | "disconnect" | "close" | "connection-error",
38-
event: string | undefined
38+
event: ConnectionStateErrored | undefined
3939
): ConnectionStateDebuggingInformation {
40-
void event;
41-
4240
switch (eventName) {
4341
case "connect":
4442
return { tag: "connected" };
45-
case "connection-error":
46-
return { tag: "errored", errorReason: event };
43+
case "connection-error": {
44+
return {
45+
tag: "errored",
46+
connectionStringAuthType: event?.connectionStringAuthType,
47+
connectedAtlasCluster: event?.connectedAtlasCluster,
48+
errorReason:
49+
event?.errorReason ??
50+
"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.",
51+
};
52+
}
4753
case "disconnect":
4854
case "close":
4955
return { tag: "disconnected" };
@@ -59,6 +65,13 @@ export class DebugResource extends ReactiveResource<
5965
break;
6066
case "errored":
6167
result += `The user is not connected to a MongoDB cluster because of an error.\n`;
68+
if (this.current.connectedAtlasCluster) {
69+
result += `Attempted connecting to Atlas Cluster "${this.current.connectedAtlasCluster.clusterName}" in project with id "${this.current.connectedAtlasCluster.projectId}".\n`;
70+
}
71+
72+
if (this.current.connectionStringAuthType !== undefined) {
73+
result += `The inferred authentication mechanism is "${this.current.connectionStringAuthType}".\n`;
74+
}
6275
result += `<error>${this.current.errorReason}</error>`;
6376
break;
6477
case "connecting":

‎tests/integration/common/connectionManager.oidc.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ describe.skipIf(process.platform !== "linux")("ConnectionManager OIDC Tests", as
128128
await connectionManager.disconnect();
129129
// for testing, force disconnecting AND setting the connection to closed to reset the
130130
// state of the connection manager
131-
connectionManager.changeState("connection-closed", { tag: "disconnected" });
131+
connectionManager.changeState("connection-close", { tag: "disconnected" });
132132

133133
await integration.connectMcpClient();
134134
}, DEFAULT_TIMEOUT);

‎tests/integration/common/connectionManager.test.ts

Lines changed: 60 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,27 +19,27 @@ describeWithMongoDB("Connection Manager", (integration) => {
1919
await connectionManager().disconnect();
2020
// for testing, force disconnecting AND setting the connection to closed to reset the
2121
// state of the connection manager
22-
connectionManager().changeState("connection-closed", { tag: "disconnected" });
22+
connectionManager().changeState("connection-close", { tag: "disconnected" });
2323
});
2424

2525
describe("when successfully connected", () => {
2626
type ConnectionManagerSpies = {
27-
"connection-requested": (event: ConnectionManagerEvents["connection-requested"][0]) => void;
28-
"connection-succeeded": (event: ConnectionManagerEvents["connection-succeeded"][0]) => void;
29-
"connection-timed-out": (event: ConnectionManagerEvents["connection-timed-out"][0]) => void;
30-
"connection-closed": (event: ConnectionManagerEvents["connection-closed"][0]) => void;
31-
"connection-errored": (event: ConnectionManagerEvents["connection-errored"][0]) => void;
27+
"connection-request": (event: ConnectionManagerEvents["connection-request"][0]) => void;
28+
"connection-success": (event: ConnectionManagerEvents["connection-success"][0]) => void;
29+
"connection-time-out": (event: ConnectionManagerEvents["connection-time-out"][0]) => void;
30+
"connection-close": (event: ConnectionManagerEvents["connection-close"][0]) => void;
31+
"connection-error": (event: ConnectionManagerEvents["connection-error"][0]) => void;
3232
};
3333

3434
let connectionManagerSpies: ConnectionManagerSpies;
3535

3636
beforeEach(async () => {
3737
connectionManagerSpies = {
38-
"connection-requested": vi.fn(),
39-
"connection-succeeded": vi.fn(),
40-
"connection-timed-out": vi.fn(),
41-
"connection-closed": vi.fn(),
42-
"connection-errored": vi.fn(),
38+
"connection-request": vi.fn(),
39+
"connection-success": vi.fn(),
40+
"connection-time-out": vi.fn(),
41+
"connection-close": vi.fn(),
42+
"connection-error": vi.fn(),
4343
};
4444

4545
for (const [event, spy] of Object.entries(connectionManagerSpies)) {
@@ -62,11 +62,11 @@ describeWithMongoDB("Connection Manager", (integration) => {
6262
});
6363

6464
it("should notify that the connection was requested", () => {
65-
expect(connectionManagerSpies["connection-requested"]).toHaveBeenCalledOnce();
65+
expect(connectionManagerSpies["connection-request"]).toHaveBeenCalledOnce();
6666
});
6767

6868
it("should notify that the connection was successful", () => {
69-
expect(connectionManagerSpies["connection-succeeded"]).toHaveBeenCalledOnce();
69+
expect(connectionManagerSpies["connection-success"]).toHaveBeenCalledOnce();
7070
});
7171

7272
describe("when disconnects", () => {
@@ -75,7 +75,7 @@ describeWithMongoDB("Connection Manager", (integration) => {
7575
});
7676

7777
it("should notify that it was disconnected before connecting", () => {
78-
expect(connectionManagerSpies["connection-closed"]).toHaveBeenCalled();
78+
expect(connectionManagerSpies["connection-close"]).toHaveBeenCalled();
7979
});
8080

8181
it("should be marked explicitly as disconnected", () => {
@@ -91,11 +91,11 @@ describeWithMongoDB("Connection Manager", (integration) => {
9191
});
9292

9393
it("should notify that it was disconnected before connecting", () => {
94-
expect(connectionManagerSpies["connection-closed"]).toHaveBeenCalled();
94+
expect(connectionManagerSpies["connection-close"]).toHaveBeenCalled();
9595
});
9696

9797
it("should notify that it was connected again", () => {
98-
expect(connectionManagerSpies["connection-succeeded"]).toHaveBeenCalled();
98+
expect(connectionManagerSpies["connection-success"]).toHaveBeenCalled();
9999
});
100100

101101
it("should be marked explicitly as connected", () => {
@@ -115,11 +115,53 @@ describeWithMongoDB("Connection Manager", (integration) => {
115115
});
116116

117117
it("should notify that it was disconnected before connecting", () => {
118-
expect(connectionManagerSpies["connection-closed"]).toHaveBeenCalled();
118+
expect(connectionManagerSpies["connection-close"]).toHaveBeenCalled();
119119
});
120120

121121
it("should notify that it failed connecting", () => {
122-
expect(connectionManagerSpies["connection-errored"]).toHaveBeenCalled();
122+
expect(connectionManagerSpies["connection-error"]).toHaveBeenCalledWith({
123+
tag: "errored",
124+
connectedAtlasCluster: undefined,
125+
connectionStringAuthType: "scram",
126+
errorReason: "Unable to parse localhost:xxxxx with URL",
127+
});
128+
});
129+
130+
it("should be marked explicitly as connected", () => {
131+
expect(connectionManager().currentConnectionState.tag).toEqual("errored");
132+
});
133+
});
134+
135+
describe("when fails to connect to a new atlas cluster", () => {
136+
const atlas = {
137+
username: "",
138+
projectId: "",
139+
clusterName: "My Atlas Cluster",
140+
expiryDate: new Date(),
141+
};
142+
143+
beforeEach(async () => {
144+
try {
145+
await connectionManager().connect({
146+
connectionString: "mongodb://localhost:xxxxx",
147+
atlas,
148+
});
149+
} catch (_error: unknown) {
150+
void _error;
151+
}
152+
});
153+
154+
it("should notify that it was disconnected before connecting", () => {
155+
expect(connectionManagerSpies["connection-close"]).toHaveBeenCalled();
156+
});
157+
158+
it("should notify that it failed connecting", () => {
159+
expect(connectionManagerSpies["connection-error"]).toHaveBeenCalledWith({
160+
tag: "errored",
161+
connectedAtlasCluster: atlas,
162+
connectionStringAuthType: "scram",
163+
errorReason: "Unable to parse localhost:xxxxx with URL",
164+
});
123165
});
124166

125167
it("should be marked explicitly as connected", () => {

0 commit comments

Comments
 (0)