-
Notifications
You must be signed in to change notification settings - Fork 104
feat: update connectionString appName param - [MCP-68] #406
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
Merged
Changes from all commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
c5c91e9
feat: update connectionString appName param - [MCP-68]
blva 4cf78c2
Merge remote-tracking branch 'origin/main' into MCP-68
blva 026b91a
add removed test
blva 680e1e1
update tests
blva 92aab61
Merge remote-tracking branch 'origin/main' into MCP-68
blva eca40e1
add timeout test
blva 6cf0ae6
Merge remote-tracking branch 'origin/main' into MCP-68
blva 524d965
fix
blva 72f1ab8
Update src/helpers/deviceId.ts
blva c3f0928
add buffering update back
blva f8de877
squashed commits
blva 8048cf6
Merge remote-tracking branch 'origin/main' into MCP-68
blva 0c620b2
move appName setting to connection manager
blva d148376
chore: rename agentRunner to mcpClient
blva 13a0349
refactor and address some comments
blva bb97041
keep getMachineId under deviceId
blva 5fb0284
chore: lint and test fix
blva a690850
fix typo
blva a2e1091
fix test
blva f964e12
fix: update appName and set it to unknown if not available
blva 6e922e6
fix: fix test
blva 4ba3a16
decouple error handling into it's own method
blva 78d430a
more linting
blva ae1d6d0
reformat
blva 68a462e
reformat and add integration test
blva 720be15
Revert "reformat"
blva 565ca2b
decouple config validation from connection
blva 69609b4
lint
blva bdf1272
Merge remote-tracking branch 'origin/main' into MCP-68
blva 658cdc8
new device id
blva dae4f06
simplify device id
blva 858ce1e
fix linting
blva 0bc1a9f
update test
blva b0972bd
address comment: inject deviceId
blva e570562
chore: update transport to close deviceId last
blva 8e62d2e
Merge branch 'main' into MCP-68
blva 0ed8d23
chore: add deviceId close to integration
blva abf285a
fix check
blva e7727ee
fix check
blva f9c22d2
Merge branch 'main' into MCP-68
blva File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,59 @@ | ||
import { MongoClientOptions } from "mongodb"; | ||
import ConnectionString from "mongodb-connection-string-url"; | ||
|
||
export function setAppNameParamIfMissing({ | ||
export interface AppNameComponents { | ||
appName: string; | ||
deviceId?: Promise<string>; | ||
clientName?: string; | ||
} | ||
|
||
/** | ||
* Sets the appName parameter with the extended format: appName--deviceId--clientName | ||
* Only sets the appName if it's not already present in the connection string | ||
* @param connectionString - The connection string to modify | ||
* @param components - The components to build the appName from | ||
* @returns The modified connection string | ||
*/ | ||
export async function setAppNameParamIfMissing({ | ||
connectionString, | ||
defaultAppName, | ||
components, | ||
}: { | ||
connectionString: string; | ||
defaultAppName?: string; | ||
}): string { | ||
components: AppNameComponents; | ||
}): Promise<string> { | ||
const connectionStringUrl = new ConnectionString(connectionString); | ||
|
||
const searchParams = connectionStringUrl.typedSearchParams<MongoClientOptions>(); | ||
|
||
if (!searchParams.has("appName") && defaultAppName !== undefined) { | ||
searchParams.set("appName", defaultAppName); | ||
// Only set appName if it's not already present | ||
if (searchParams.has("appName")) { | ||
gagik marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return connectionStringUrl.toString(); | ||
} | ||
|
||
const appName = components.appName || "unknown"; | ||
const deviceId = components.deviceId ? await components.deviceId : "unknown"; | ||
const clientName = components.clientName || "unknown"; | ||
|
||
// Build the extended appName format: appName--deviceId--clientName | ||
const extendedAppName = `${appName}--${deviceId}--${clientName}`; | ||
|
||
searchParams.set("appName", extendedAppName); | ||
|
||
return connectionStringUrl.toString(); | ||
} | ||
|
||
/** | ||
* Validates the connection string | ||
* @param connectionString - The connection string to validate | ||
* @param looseValidation - Whether to allow loose validation | ||
* @returns void | ||
* @throws Error if the connection string is invalid | ||
*/ | ||
export function validateConnectionString(connectionString: string, looseValidation: boolean): void { | ||
try { | ||
new ConnectionString(connectionString, { looseValidation }); | ||
} catch (error) { | ||
throw new Error( | ||
`Invalid connection string with error: ${error instanceof Error ? error.message : String(error)}` | ||
); | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
import { getDeviceId } from "@mongodb-js/device-id"; | ||
import nodeMachineId from "node-machine-id"; | ||
import { LogId, LoggerBase } from "../common/logger.js"; | ||
|
||
export const DEVICE_ID_TIMEOUT = 3000; | ||
gagik marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
export class DeviceId { | ||
private deviceId: string | undefined = undefined; | ||
private deviceIdPromise: Promise<string> | undefined = undefined; | ||
private abortController: AbortController | undefined = undefined; | ||
private logger: LoggerBase; | ||
private readonly getMachineId: () => Promise<string>; | ||
private timeout: number; | ||
private static instance: DeviceId | undefined = undefined; | ||
|
||
private constructor(logger: LoggerBase, timeout: number = DEVICE_ID_TIMEOUT) { | ||
this.logger = logger; | ||
this.timeout = timeout; | ||
this.getMachineId = (): Promise<string> => nodeMachineId.machineId(true); | ||
} | ||
|
||
public static create(logger: LoggerBase, timeout?: number): DeviceId { | ||
if (this.instance) { | ||
blva marked this conversation as resolved.
Show resolved
Hide resolved
|
||
throw new Error("DeviceId instance already exists, use get() to retrieve the device ID"); | ||
gagik marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
const instance = new DeviceId(logger, timeout ?? DEVICE_ID_TIMEOUT); | ||
instance.setup(); | ||
|
||
this.instance = instance; | ||
|
||
return instance; | ||
} | ||
|
||
private setup(): void { | ||
this.deviceIdPromise = this.calculateDeviceId(); | ||
} | ||
|
||
/** | ||
* Closes the device ID calculation promise and abort controller. | ||
*/ | ||
public close(): void { | ||
if (this.abortController) { | ||
this.abortController.abort(); | ||
this.abortController = undefined; | ||
} | ||
|
||
this.deviceId = undefined; | ||
this.deviceIdPromise = undefined; | ||
DeviceId.instance = undefined; | ||
} | ||
|
||
/** | ||
* Gets the device ID, waiting for the calculation to complete if necessary. | ||
* @returns Promise that resolves to the device ID string | ||
*/ | ||
public get(): Promise<string> { | ||
if (this.deviceId) { | ||
return Promise.resolve(this.deviceId); | ||
} | ||
|
||
if (this.deviceIdPromise) { | ||
return this.deviceIdPromise; | ||
} | ||
|
||
return this.calculateDeviceId(); | ||
} | ||
|
||
/** | ||
* Internal method that performs the actual device ID calculation. | ||
*/ | ||
private async calculateDeviceId(): Promise<string> { | ||
if (!this.abortController) { | ||
this.abortController = new AbortController(); | ||
} | ||
|
||
this.deviceIdPromise = getDeviceId({ | ||
getMachineId: this.getMachineId, | ||
onError: (reason, error) => { | ||
this.handleDeviceIdError(reason, String(error)); | ||
}, | ||
timeout: this.timeout, | ||
abortSignal: this.abortController.signal, | ||
}); | ||
|
||
return this.deviceIdPromise; | ||
} | ||
|
||
private handleDeviceIdError(reason: string, error: string): void { | ||
this.deviceIdPromise = Promise.resolve("unknown"); | ||
|
||
switch (reason) { | ||
case "resolutionError": | ||
this.logger.debug({ | ||
id: LogId.deviceIdResolutionError, | ||
context: "deviceId", | ||
message: `Resolution error: ${String(error)}`, | ||
}); | ||
break; | ||
case "timeout": | ||
this.logger.debug({ | ||
id: LogId.deviceIdTimeout, | ||
context: "deviceId", | ||
message: "Device ID retrieval timed out", | ||
noRedaction: true, | ||
}); | ||
break; | ||
case "abort": | ||
// No need to log in the case of 'abort' errors | ||
break; | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.