-
Notifications
You must be signed in to change notification settings - Fork 109
fix: minor tweaks to enable vscode integration MCP-134 #467
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
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
5249266
fix: minor tweaks to enable vscode integration
nirinchev 5f1e1b2
add tests
nirinchev 2ab7f42
Better formatted error
nirinchev 8c3c968
lowercase the key
nirinchev f29226f
fix build tests
nirinchev e85b2eb
fix tests again
nirinchev 24097c3
Merge branch 'main' into ni/http-tweaks
nirinchev 3c111b0
fix tests, refactor deviceId
nirinchev 9d2e127
Merge branch 'main' into ni/http-tweaks
nirinchev f5481fd
more tweaks
nirinchev 2b77ed8
fix tests
nirinchev 8b2e7c7
run permissions monitor on ubuntu only
nirinchev 4ddaebb
Merge branch 'main' into ni/http-tweaks
nirinchev 9faea5d
address -> serverAddress
nirinchev 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
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,94 +1,63 @@ | ||||||||||||||||||||||||||
import { getDeviceId } from "@mongodb-js/device-id"; | ||||||||||||||||||||||||||
import nodeMachineId from "node-machine-id"; | ||||||||||||||||||||||||||
import * as nodeMachineId from "node-machine-id"; | ||||||||||||||||||||||||||
import type { LoggerBase } from "../common/logger.js"; | ||||||||||||||||||||||||||
import { LogId } from "../common/logger.js"; | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
export const DEVICE_ID_TIMEOUT = 3000; | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
export class DeviceId { | ||||||||||||||||||||||||||
private deviceId: string | undefined = undefined; | ||||||||||||||||||||||||||
private deviceIdPromise: Promise<string> | undefined = undefined; | ||||||||||||||||||||||||||
private abortController: AbortController | undefined = undefined; | ||||||||||||||||||||||||||
private static readonly UnknownDeviceId = Promise.resolve("unknown"); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
private deviceIdPromise: Promise<string>; | ||||||||||||||||||||||||||
private abortController: AbortController; | ||||||||||||||||||||||||||
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); | ||||||||||||||||||||||||||
this.abortController = new AbortController(); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
this.deviceIdPromise = DeviceId.UnknownDeviceId; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
public static create(logger: LoggerBase, timeout?: number): DeviceId { | ||||||||||||||||||||||||||
if (this.instance) { | ||||||||||||||||||||||||||
throw new Error("DeviceId instance already exists, use get() to retrieve the device ID"); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
private initialize(): void { | ||||||||||||||||||||||||||
this.deviceIdPromise = getDeviceId({ | ||||||||||||||||||||||||||
getMachineId: this.getMachineId, | ||||||||||||||||||||||||||
onError: (reason, error) => { | ||||||||||||||||||||||||||
this.handleDeviceIdError(reason, String(error)); | ||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||
timeout: this.timeout, | ||||||||||||||||||||||||||
abortSignal: this.abortController.signal, | ||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
public static create(logger: LoggerBase, timeout?: number): DeviceId { | ||||||||||||||||||||||||||
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.
Suggested change
personal preference re: timeout as object because it's one of these things which we basically largely have only for test DI purposes |
||||||||||||||||||||||||||
const instance = new DeviceId(logger, timeout ?? DEVICE_ID_TIMEOUT); | ||||||||||||||||||||||||||
instance.setup(); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
this.instance = instance; | ||||||||||||||||||||||||||
instance.initialize(); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
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; | ||||||||||||||||||||||||||
this.abortController.abort(); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||
* 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"); | ||||||||||||||||||||||||||
this.deviceIdPromise = DeviceId.UnknownDeviceId; | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
switch (reason) { | ||||||||||||||||||||||||||
case "resolutionError": | ||||||||||||||||||||||||||
|
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,4 +1,7 @@ | ||
export { Server, type ServerOptions } from "./server.js"; | ||
export { Telemetry } from "./telemetry/telemetry.js"; | ||
export { Session, type SessionOptions } from "./common/session.js"; | ||
export type { UserConfig } from "./common/config.js"; | ||
export { type UserConfig, defaultUserConfig } from "./common/config.js"; | ||
export { StreamableHttpRunner } from "./transports/streamableHttp.js"; | ||
export { LoggerBase } from "./common/logger.js"; | ||
export type { LogPayload, LoggerType, LogLevel } from "./common/logger.js"; |
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is better because it'd be good to have the Promise resolve to unknown only in the end so we can always have a state of
pending -> unknown
and notunknown -> pending -> unknown
(even though in practice we instantly override this increate
right nowThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, we can't supply the promise through the constructor since we need the
DeviceId
instance to construct the promise itself. Technically, the only reason to have it assigned here is to make the types nicer and ensure we don't have to write?
all over the place when we know for sure the field is initialized at this point. So we have 3 options (with no preference which one to go for):