-
Notifications
You must be signed in to change notification settings - Fork 96
chore: Reactive resource support for debugging connectivity MCP-80 #413
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.
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
Changes from 6 commits
84aa8ba
ce6d2e9
26e1407
c042cff
76ddf15
c5b696b
235ffcf
7dc6746
550a284
e50d706
d0790b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,10 +13,14 @@ export interface SessionOptions { | |
apiClientSecret?: string; | ||
} | ||
|
||
export class Session extends EventEmitter<{ | ||
export type SessionEvents = { | ||
connected: []; | ||
close: []; | ||
disconnect: []; | ||
}> { | ||
"connection-error": [string]; | ||
}; | ||
|
||
export class Session extends EventEmitter<SessionEvents> { | ||
sessionId?: string; | ||
serviceProvider?: NodeDriverServiceProvider; | ||
apiClient: ApiClient; | ||
|
@@ -102,19 +106,30 @@ export class Session extends EventEmitter<{ | |
connectionString, | ||
defaultAppName: `${packageInfo.mcpServerName} ${packageInfo.version}`, | ||
}); | ||
this.serviceProvider = await NodeDriverServiceProvider.connect(connectionString, { | ||
productDocsLink: "https://github.com/mongodb-js/mongodb-mcp-server/", | ||
productName: "MongoDB MCP", | ||
readConcern: { | ||
level: connectOptions.readConcern, | ||
}, | ||
readPreference: connectOptions.readPreference, | ||
writeConcern: { | ||
w: connectOptions.writeConcern, | ||
}, | ||
timeoutMS: connectOptions.timeoutMS, | ||
proxy: { useEnvironmentVariableProxies: true }, | ||
applyProxyToOIDC: true, | ||
}); | ||
|
||
try { | ||
this.serviceProvider = await NodeDriverServiceProvider.connect(connectionString, { | ||
productDocsLink: "https://github.com/mongodb-js/mongodb-mcp-server/", | ||
productName: "MongoDB MCP", | ||
readConcern: { | ||
level: connectOptions.readConcern, | ||
}, | ||
readPreference: connectOptions.readPreference, | ||
writeConcern: { | ||
w: connectOptions.writeConcern, | ||
}, | ||
timeoutMS: connectOptions.timeoutMS, | ||
proxy: { useEnvironmentVariableProxies: true }, | ||
applyProxyToOIDC: true, | ||
}); | ||
|
||
await this.serviceProvider?.runCommand?.("admin", { hello: 1 }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NodeDriverServiceProvider.connect, despite it's name, does not in fact connect. This behaviour is likely inherited from the driver. By forcing a ping (by using hello) we force a connection and validate that it's in fact correct. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we have a ping on atlas cluster connect tool where we added a ping, should we drop it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see that it's used for answering the client with the connection status. I think we can keep it for now and then when we remove this logic to the new ConnectionManager (Tech Design) we can refactor and use the same code for both connections. How do you feel about this strategy? |
||
} catch (error: unknown) { | ||
const message = error instanceof Error ? error.message : `${error as string}`; | ||
this.emit("connection-error", message); | ||
throw error; | ||
} | ||
|
||
this.emit("connected"); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
import { ReactiveResource } from "../resource.js"; | ||
import { config } from "../../common/config.js"; | ||
import type { UserConfig } from "../../common/config.js"; | ||
|
||
export class ConfigResource extends ReactiveResource( | ||
{ | ||
name: "config", | ||
uri: "config://config", | ||
config: { | ||
description: | ||
"Server configuration, supplied by the user either as environment variables or as startup arguments", | ||
}, | ||
}, | ||
{ | ||
initial: { ...config }, | ||
events: [], | ||
} | ||
) { | ||
reduce(eventName: undefined, event: undefined): UserConfig { | ||
void eventName; | ||
void event; | ||
Comment on lines
+19
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could possibly be avoided by having reduce without any params? |
||
|
||
return this.current; | ||
} | ||
|
||
toOutput(): string { | ||
const result = { | ||
telemetry: this.current.telemetry, | ||
logPath: this.current.logPath, | ||
connectionString: this.current.connectionString | ||
? "set; access to MongoDB tools are currently available to use" | ||
: "not set; before using any MongoDB tool, you need to configure a connection string, alternatively you can setup MongoDB Atlas access, more info at 'https://github.com/mongodb-js/mongodb-mcp-server'.", | ||
connectOptions: this.current.connectOptions, | ||
atlas: | ||
this.current.apiClientId && this.current.apiClientSecret | ||
? "set; MongoDB Atlas tools are currently available to use" | ||
: "not set; MongoDB Atlas tools are currently unavailable, to have access to MongoDB Atlas tools like creating clusters or connecting to clusters make sure to setup credentials, more info at 'https://github.com/mongodb-js/mongodb-mcp-server'.", | ||
}; | ||
|
||
return JSON.stringify(result); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
import { ReactiveResource } from "../resource.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; | ||
}; | ||
|
||
export class DebugResource extends ReactiveResource( | ||
{ | ||
name: "debug", | ||
uri: "config://debug", | ||
kmruiz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
config: { | ||
description: "Debugging information for connectivity issues.", | ||
}, | ||
}, | ||
{ | ||
initial: { tag: "disconnected" } as ConnectionStateDebuggingInformation, | ||
events: ["connected", "disconnect", "close", "connection-error"], | ||
} | ||
) { | ||
reduce( | ||
eventName: "connected" | "disconnect" | "close" | "connection-error", | ||
event: string | undefined | ||
): ConnectionStateDebuggingInformation { | ||
void event; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this necessary anymore, now that you're using |
||
|
||
switch (eventName) { | ||
case "connected": | ||
return { tag: "connected" }; | ||
case "connection-error": | ||
return { tag: "errored", errorReason: event }; | ||
case "disconnect": | ||
case "close": | ||
return { tag: "disconnected" }; | ||
} | ||
} | ||
|
||
toOutput(): string { | ||
let result = ""; | ||
|
||
switch (this.current.tag) { | ||
case "connected": | ||
result += "The user is connected to the MongoDB cluster."; | ||
break; | ||
case "errored": | ||
result += `The user is not connected to a MongoDB cluster because of an error.\n`; | ||
result += `<error>${this.current.errorReason}</error>`; | ||
break; | ||
case "connecting": | ||
case "disconnected": | ||
result += "The user is not connected to a MongoDB cluster."; | ||
break; | ||
} | ||
|
||
return result; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
import { Server } from "../server.js"; | ||
import { Session } from "../common/session.js"; | ||
import { UserConfig } from "../common/config.js"; | ||
import { Telemetry } from "../telemetry/telemetry.js"; | ||
import type { SessionEvents } from "../common/session.js"; | ||
import { ReadResourceCallback, RegisteredResource, ResourceMetadata } from "@modelcontextprotocol/sdk/server/mcp.js"; | ||
|
||
type PayloadOf<K extends keyof SessionEvents> = SessionEvents[K][0]; | ||
|
||
type ResourceConfiguration = { name: string; uri: string; config: ResourceMetadata }; | ||
|
||
export function ReactiveResource<V, KE extends readonly (keyof SessionEvents)[]>( | ||
kmruiz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ name, uri, config: resourceConfig }: ResourceConfiguration, | ||
{ | ||
initial, | ||
events, | ||
}: { | ||
initial: V; | ||
events: KE; | ||
} | ||
) { | ||
type E = KE[number]; | ||
|
||
abstract class NewReactiveResource { | ||
private registeredResource?: RegisteredResource; | ||
protected readonly session: Session; | ||
protected readonly config: UserConfig; | ||
protected current: V; | ||
|
||
constructor( | ||
protected readonly server: Server, | ||
protected readonly telemetry: Telemetry, | ||
current?: V | ||
) { | ||
this.current = current ?? initial; | ||
this.session = server.session; | ||
this.config = server.userConfig; | ||
|
||
for (const event of events) { | ||
this.session.on(event, (...args: SessionEvents[typeof event]) => { | ||
this.reduceApply(event, (args as unknown[])[0] as PayloadOf<typeof event>); | ||
this.triggerUpdate(); | ||
}); | ||
} | ||
} | ||
|
||
public register(): void { | ||
this.registeredResource = this.server.mcpServer.registerResource( | ||
name, | ||
uri, | ||
resourceConfig, | ||
this.resourceCallback | ||
); | ||
} | ||
|
||
private resourceCallback: ReadResourceCallback = (uri) => ({ | ||
contents: [ | ||
{ | ||
text: this.toOutput(), | ||
mimeType: "application/json", | ||
uri: uri.href, | ||
}, | ||
], | ||
}); | ||
|
||
private triggerUpdate() { | ||
this.registeredResource?.update({}); | ||
this.server.mcpServer.sendResourceListChanged(); | ||
kmruiz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
reduceApply(eventName: E, ...event: PayloadOf<E>[]): void { | ||
this.current = this.reduce(eventName, ...event); | ||
} | ||
|
||
protected abstract reduce(eventName: E, ...event: PayloadOf<E>[]): V; | ||
abstract toOutput(): string; | ||
} | ||
|
||
return NewReactiveResource; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
import { ConfigResource } from "./common/config.js"; | ||
import { DebugResource } from "./common/debug.js"; | ||
|
||
export const Resources = [ConfigResource, DebugResource] as const; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
import { beforeEach, describe, expect, it } from "vitest"; | ||
import { DebugResource } from "../../../../src/resources/common/debug.js"; | ||
import { Session } from "../../../../src/common/session.js"; | ||
import { Server } from "../../../../src/server.js"; | ||
import { Telemetry } from "../../../../src/telemetry/telemetry.js"; | ||
import { config } from "../../../../src/common/config.js"; | ||
|
||
describe("debug resource", () => { | ||
// eslint-disable-next-line | ||
const session = new Session({} as any); | ||
// eslint-disable-next-line | ||
const server = new Server({ session } as any); | ||
const telemetry = Telemetry.create(session, { ...config, telemetry: "disabled" }); | ||
|
||
let debugResource: DebugResource = new DebugResource(server, telemetry); | ||
|
||
beforeEach(() => { | ||
debugResource = new DebugResource(server, telemetry); | ||
}); | ||
|
||
it("should be connected when a connected event happens", () => { | ||
debugResource.reduceApply("connected", undefined); | ||
const output = debugResource.toOutput(); | ||
|
||
expect(output).toContain(`The user is connected to the MongoDB cluster.`); | ||
}); | ||
|
||
it("should be disconnected when a disconnect event happens", () => { | ||
debugResource.reduceApply("disconnect", undefined); | ||
const output = debugResource.toOutput(); | ||
|
||
expect(output).toContain(`The user is not connected to a MongoDB cluster.`); | ||
}); | ||
|
||
it("should be disconnected when a close event happens", () => { | ||
debugResource.reduceApply("close", undefined); | ||
const output = debugResource.toOutput(); | ||
|
||
expect(output).toContain(`The user is not connected to a MongoDB cluster.`); | ||
}); | ||
|
||
it("should be disconnected and contain an error when an error event occurred", () => { | ||
debugResource.reduceApply("connection-error", "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>Error message from the server</error>`); | ||
}); | ||
}); |
Uh oh!
There was an error while loading. Please reload this page.