From fb73742b2bf23cb7afddbba501b633cc806635ce Mon Sep 17 00:00:00 2001 From: Sean Lee Date: Wed, 14 Jun 2023 17:32:07 -0400 Subject: [PATCH] Prefer matching editor sessions when opening files. (#6191) Signed-off-by: Sean Lee --- patches/store-socket.diff | 122 ++++++++++++-- src/node/app.ts | 40 +++-- src/node/cli.ts | 56 +++---- src/node/main.ts | 8 +- src/node/vscodeSocket.ts | 206 +++++++++++++++++++++++ test/unit/node/app.test.ts | 4 +- test/unit/node/cli.test.ts | 189 ++++++++++++++-------- test/unit/node/vscodeSocket.test.ts | 243 ++++++++++++++++++++++++++++ test/utils/helpers.ts | 49 ++++++ 9 files changed, 786 insertions(+), 131 deletions(-) create mode 100644 src/node/vscodeSocket.ts create mode 100644 test/unit/node/vscodeSocket.test.ts diff --git a/patches/store-socket.diff b/patches/store-socket.diff index 74a7b635b..5f878032a 100644 --- a/patches/store-socket.diff +++ b/patches/store-socket.diff @@ -1,4 +1,4 @@ -Store a static reference to the IPC socket +Store the IPC socket with workspace metadata. This lets us use it to open files inside code-server from outside of code-server. @@ -9,6 +9,8 @@ To test this: It should open in your existing code-server instance. +When the extension host is terminated, the socket is unregistered. + Index: code-server/lib/vscode/src/vs/workbench/api/node/extHostExtensionService.ts =================================================================== --- code-server.orig/lib/vscode/src/vs/workbench/api/node/extHostExtensionService.ts @@ -18,20 +20,114 @@ Index: code-server/lib/vscode/src/vs/workbench/api/node/extHostExtensionService. * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ - -+import { promises as fs } from 'fs'; -+import * as os from 'os' ++import * as os from 'os'; ++import * as _http from 'http'; +import * as path from 'vs/base/common/path'; import * as performance from 'vs/base/common/performance'; import { createApiFactoryAndRegisterActors } from 'vs/workbench/api/common/extHost.api.impl'; import { RequireInterceptor } from 'vs/workbench/api/common/extHostRequireInterceptor'; -@@ -72,6 +74,10 @@ export class ExtHostExtensionService ext - if (this._initData.remote.isRemote && this._initData.remote.authority) { - const cliServer = this._instaService.createInstance(CLIServer); - process.env['VSCODE_IPC_HOOK_CLI'] = cliServer.ipcHandlePath; -+ -+ fs.writeFile(path.join(os.tmpdir(), 'vscode-ipc'), cliServer.ipcHandlePath).catch((error) => { -+ this._logService.error(error); -+ }); - } +@@ -17,6 +19,7 @@ import { ExtensionRuntime } from 'vs/wor + import { CLIServer } from 'vs/workbench/api/node/extHostCLIServer'; + import { realpathSync } from 'vs/base/node/extpath'; + import { ExtHostConsoleForwarder } from 'vs/workbench/api/node/extHostConsoleForwarder'; ++import { IExtHostWorkspace } from '../common/extHostWorkspace'; - // Module loading tricks + class NodeModuleRequireInterceptor extends RequireInterceptor { + +@@ -79,6 +82,52 @@ export class ExtHostExtensionService ext + await interceptor.install(); + performance.mark('code/extHost/didInitAPI'); + ++ (async () => { ++ const socketPath = process.env['VSCODE_IPC_HOOK_CLI']; ++ if (!socketPath) { ++ return; ++ } ++ const workspace = this._instaService.invokeFunction((accessor) => { ++ const workspaceService = accessor.get(IExtHostWorkspace); ++ return workspaceService.workspace; ++ }); ++ const entry = { ++ workspace, ++ socketPath ++ }; ++ const message = JSON.stringify({entry}); ++ const codeServerSocketPath = path.join(os.tmpdir(), 'code-server-ipc.sock'); ++ await new Promise((resolve, reject) => { ++ const opts: _http.RequestOptions = { ++ path: '/add-session', ++ socketPath: codeServerSocketPath, ++ method: 'POST', ++ headers: { ++ 'content-type': 'application/json', ++ } ++ }; ++ const req = _http.request(opts, (res) => { ++ res.on('error', reject); ++ res.on('end', () => { ++ try { ++ if (res.statusCode === 200) { ++ resolve(); ++ } else { ++ reject(new Error('Unexpected status code: ' + res.statusCode)); ++ } ++ } catch (e: unknown) { ++ reject(e); ++ } ++ }); ++ }); ++ req.on('error', reject); ++ req.write(message); ++ req.end(); ++ }); ++ })().catch(error => { ++ this._logService.error(error); ++ }); ++ + // Do this when extension service exists, but extensions are not being activated yet. + const configProvider = await this._extHostConfiguration.getConfigProvider(); + await connectProxyResolver(this._extHostWorkspace, configProvider, this, this._logService, this._mainThreadTelemetryProxy, this._initData); +Index: code-server/lib/vscode/src/vs/workbench/api/node/extensionHostProcess.ts +=================================================================== +--- code-server.orig/lib/vscode/src/vs/workbench/api/node/extensionHostProcess.ts ++++ code-server/lib/vscode/src/vs/workbench/api/node/extensionHostProcess.ts +@@ -3,6 +3,9 @@ + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + ++import * as os from 'os'; ++import * as _http from 'http'; ++import * as path from 'vs/base/common/path'; + import * as nativeWatchdog from 'native-watchdog'; + import * as net from 'net'; + import * as minimist from 'minimist'; +@@ -400,7 +403,28 @@ async function startExtensionHostProcess + ); + + // rewrite onTerminate-function to be a proper shutdown +- onTerminate = (reason: string) => extensionHostMain.terminate(reason); ++ onTerminate = (reason: string) => { ++ extensionHostMain.terminate(reason); ++ ++ const socketPath = process.env['VSCODE_IPC_HOOK_CLI']; ++ if (!socketPath) { ++ return; ++ } ++ const message = JSON.stringify({socketPath}); ++ const codeServerSocketPath = path.join(os.tmpdir(), 'code-server-ipc.sock'); ++ const opts: _http.RequestOptions = { ++ path: '/delete-session', ++ socketPath: codeServerSocketPath, ++ method: 'POST', ++ headers: { ++ 'content-type': 'application/json', ++ 'accept': 'application/json' ++ } ++ }; ++ const req = _http.request(opts); ++ req.write(message); ++ req.end(); ++ }; + } + + startExtensionHostProcess().catch((err) => console.log(err)); diff --git a/src/node/app.ts b/src/node/app.ts index 3e6e4bfa0..2cc858f18 100644 --- a/src/node/app.ts +++ b/src/node/app.ts @@ -9,9 +9,11 @@ import * as util from "../common/util" import { DefaultedArgs } from "./cli" import { disposer } from "./http" import { isNodeJSErrnoException } from "./util" +import { DEFAULT_SOCKET_PATH, EditorSessionManager, makeEditorSessionManagerServer } from "./vscodeSocket" import { handleUpgrade } from "./wsRouter" -type ListenOptions = Pick +type SocketOptions = { socket: string; "socket-mode"?: string } +type ListenOptions = DefaultedArgs | SocketOptions export interface App extends Disposable { /** Handles regular HTTP requests. */ @@ -20,12 +22,18 @@ export interface App extends Disposable { wsRouter: Express /** The underlying HTTP server. */ server: http.Server + /** Handles requests to the editor session management API. */ + editorSessionManagerServer: http.Server } -export const listen = async (server: http.Server, { host, port, socket, "socket-mode": mode }: ListenOptions) => { - if (socket) { +const isSocketOpts = (opts: ListenOptions): opts is SocketOptions => { + return !!(opts as SocketOptions).socket || !(opts as DefaultedArgs).host +} + +export const listen = async (server: http.Server, opts: ListenOptions) => { + if (isSocketOpts(opts)) { try { - await fs.unlink(socket) + await fs.unlink(opts.socket) } catch (error: any) { handleArgsSocketCatchError(error) } @@ -38,18 +46,20 @@ export const listen = async (server: http.Server, { host, port, socket, "socket- server.on("error", (err) => util.logError(logger, "http server error", err)) resolve() } - if (socket) { - server.listen(socket, onListen) + if (isSocketOpts(opts)) { + server.listen(opts.socket, onListen) } else { // [] is the correct format when using :: but Node errors with them. - server.listen(port, host.replace(/^\[|\]$/g, ""), onListen) + server.listen(opts.port, opts.host.replace(/^\[|\]$/g, ""), onListen) } }) // NOTE@jsjoeio: we need to chmod after the server is finished // listening. Otherwise, the socket may not have been created yet. - if (socket && mode) { - await fs.chmod(socket, mode) + if (isSocketOpts(opts)) { + if (opts["socket-mode"]) { + await fs.chmod(opts.socket, opts["socket-mode"]) + } } } @@ -70,14 +80,22 @@ export const createApp = async (args: DefaultedArgs): Promise => { ) : http.createServer(router) - const dispose = disposer(server) + const disposeServer = disposer(server) await listen(server, args) const wsRouter = express() handleUpgrade(wsRouter, server) - return { router, wsRouter, server, dispose } + const editorSessionManager = new EditorSessionManager() + const editorSessionManagerServer = await makeEditorSessionManagerServer(DEFAULT_SOCKET_PATH, editorSessionManager) + const disposeEditorSessionManagerServer = disposer(editorSessionManagerServer) + + const dispose = async () => { + await Promise.all([disposeServer(), disposeEditorSessionManagerServer()]) + } + + return { router, wsRouter, server, dispose, editorSessionManagerServer } } /** diff --git a/src/node/cli.ts b/src/node/cli.ts index ad8bd9f81..fe7350f54 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -3,17 +3,8 @@ import { promises as fs } from "fs" import { load } from "js-yaml" import * as os from "os" import * as path from "path" -import { - canConnect, - generateCertificate, - generatePassword, - humanPath, - paths, - isNodeJSErrnoException, - splitOnFirstEquals, -} from "./util" - -const DEFAULT_SOCKET_PATH = path.join(os.tmpdir(), "vscode-ipc") +import { generateCertificate, generatePassword, humanPath, paths, splitOnFirstEquals } from "./util" +import { DEFAULT_SOCKET_PATH, EditorSessionManagerClient } from "./vscodeSocket" export enum Feature { // No current experimental features! @@ -591,9 +582,7 @@ export async function setDefaults(cliArgs: UserProvidedArgs, configArgs?: Config } args["proxy-domain"] = finalProxies - if (typeof args._ === "undefined") { - args._ = [] - } + args._ = getResolvedPathsFromArgs(args) return { ...args, @@ -602,6 +591,10 @@ export async function setDefaults(cliArgs: UserProvidedArgs, configArgs?: Config } as DefaultedArgs // TODO: Technically no guarantee this is fulfilled. } +export function getResolvedPathsFromArgs(args: UserProvidedArgs): string[] { + return (args._ ?? []).map((p) => path.resolve(p)) +} + /** * Helper function to return the default config file. * @@ -741,27 +734,6 @@ function bindAddrFromAllSources(...argsConfig: UserProvidedArgs[]): Addr { return addr } -/** - * Reads the socketPath based on path passed in. - * - * The one usually passed in is the DEFAULT_SOCKET_PATH. - * - * If it can't read the path, it throws an error and returns undefined. - */ -export async function readSocketPath(path: string): Promise { - try { - return await fs.readFile(path, "utf8") - } catch (error) { - // If it doesn't exist, we don't care. - // But if it fails for some reason, we should throw. - // We want to surface that to the user. - if (!isNodeJSErrnoException(error) || error.code !== "ENOENT") { - throw error - } - } - return undefined -} - /** * Determine if it looks like the user is trying to open a file or folder in an * existing instance. The arguments here should be the arguments the user @@ -774,6 +746,14 @@ export const shouldOpenInExistingInstance = async (args: UserProvidedArgs): Prom return process.env.VSCODE_IPC_HOOK_CLI } + const paths = getResolvedPathsFromArgs(args) + const client = new EditorSessionManagerClient(DEFAULT_SOCKET_PATH) + + // If we can't connect to the socket then there's no existing instance. + if (!(await client.canConnect())) { + return undefined + } + // If these flags are set then assume the user is trying to open in an // existing instance since these flags have no effect otherwise. const openInFlagCount = ["reuse-window", "new-window"].reduce((prev, cur) => { @@ -781,7 +761,7 @@ export const shouldOpenInExistingInstance = async (args: UserProvidedArgs): Prom }, 0) if (openInFlagCount > 0) { logger.debug("Found --reuse-window or --new-window") - return readSocketPath(DEFAULT_SOCKET_PATH) + return await client.getConnectedSocketPath(paths[0]) } // It's possible the user is trying to spawn another instance of code-server. @@ -790,8 +770,8 @@ export const shouldOpenInExistingInstance = async (args: UserProvidedArgs): Prom // 2. That a file or directory was passed. // 3. That the socket is active. if (Object.keys(args).length === 1 && typeof args._ !== "undefined" && args._.length > 0) { - const socketPath = await readSocketPath(DEFAULT_SOCKET_PATH) - if (socketPath && (await canConnect(socketPath))) { + const socketPath = await client.getConnectedSocketPath(paths[0]) + if (socketPath) { logger.debug("Found existing code-server socket") return socketPath } diff --git a/src/node/main.ts b/src/node/main.ts index d2f9c8b38..5af1f5c6a 100644 --- a/src/node/main.ts +++ b/src/node/main.ts @@ -1,7 +1,6 @@ import { field, logger } from "@coder/logger" import http from "http" import * as os from "os" -import path from "path" import { Disposable } from "../common/emitter" import { plural } from "../common/util" import { createApp, ensureAddress } from "./app" @@ -70,9 +69,8 @@ export const openInExistingInstance = async (args: DefaultedArgs, socketPath: st forceNewWindow: args["new-window"], gotoLineMode: true, } - const paths = args._ || [] - for (let i = 0; i < paths.length; i++) { - const fp = path.resolve(paths[i]) + for (let i = 0; i < args._.length; i++) { + const fp = args._[i] if (await isDirectory(fp)) { pipeArgs.folderURIs.push(fp) } else { @@ -123,10 +121,12 @@ export const runCodeServer = async ( const app = await createApp(args) const protocol = args.cert ? "https" : "http" const serverAddress = ensureAddress(app.server, protocol) + const sessionServerAddress = app.editorSessionManagerServer.address() const disposeRoutes = await register(app, args) logger.info(`Using config file ${humanPath(os.homedir(), args.config)}`) logger.info(`${protocol.toUpperCase()} server listening on ${serverAddress.toString()}`) + logger.info(`Session server listening on ${sessionServerAddress?.toString()}`) if (args.auth === AuthType.Password) { logger.info(" - Authentication is enabled") diff --git a/src/node/vscodeSocket.ts b/src/node/vscodeSocket.ts new file mode 100644 index 000000000..1bdb895f8 --- /dev/null +++ b/src/node/vscodeSocket.ts @@ -0,0 +1,206 @@ +import { logger } from "@coder/logger" +import express from "express" +import * as http from "http" +import * as os from "os" +import * as path from "path" +import { HttpCode } from "../common/http" +import { listen } from "./app" +import { canConnect } from "./util" + +// Socket path of the daemonized code-server instance. +export const DEFAULT_SOCKET_PATH = path.join(os.tmpdir(), "code-server-ipc.sock") + +export interface EditorSessionEntry { + workspace: { + id: string + folders: { + uri: { + path: string + } + }[] + } + + socketPath: string +} + +interface DeleteSessionRequest { + socketPath: string +} + +interface AddSessionRequest { + entry: EditorSessionEntry +} + +interface GetSessionResponse { + socketPath?: string +} + +export async function makeEditorSessionManagerServer( + codeServerSocketPath: string, + editorSessionManager: EditorSessionManager, +): Promise { + const router = express() + + // eslint-disable-next-line import/no-named-as-default-member + router.use(express.json()) + + router.get("/session", async (req, res) => { + const filePath = req.query.filePath as string + if (!filePath) { + res.status(HttpCode.BadRequest).send("filePath is required") + return + } + try { + const socketPath = await editorSessionManager.getConnectedSocketPath(filePath) + const response: GetSessionResponse = { socketPath } + res.json(response) + } catch (error: unknown) { + res.status(HttpCode.ServerError).send(error) + } + }) + + router.post("/add-session", async (req, res) => { + const request = req.body as AddSessionRequest + if (!request.entry) { + res.status(400).send("entry is required") + } + editorSessionManager.addSession(request.entry) + res.status(200).send() + }) + + router.post("/delete-session", async (req, res) => { + const request = req.body as DeleteSessionRequest + if (!request.socketPath) { + res.status(400).send("socketPath is required") + } + editorSessionManager.deleteSession(request.socketPath) + res.status(200).send() + }) + + const server = http.createServer(router) + await listen(server, { socket: codeServerSocketPath }) + return server +} + +export class EditorSessionManager { + // Map from socket path to EditorSessionEntry. + private entries = new Map() + + addSession(entry: EditorSessionEntry): void { + logger.debug(`Adding session to session registry: ${entry.socketPath}`) + this.entries.set(entry.socketPath, entry) + } + + getCandidatesForFile(filePath: string): EditorSessionEntry[] { + const matchCheckResults = new Map() + + const checkMatch = (entry: EditorSessionEntry): boolean => { + if (matchCheckResults.has(entry.socketPath)) { + return matchCheckResults.get(entry.socketPath)! + } + const result = entry.workspace.folders.some((folder) => filePath.startsWith(folder.uri.path + path.sep)) + matchCheckResults.set(entry.socketPath, result) + return result + } + + return Array.from(this.entries.values()) + .reverse() // Most recently registered first. + .sort((a, b) => { + // Matches first. + const aMatch = checkMatch(a) + const bMatch = checkMatch(b) + if (aMatch === bMatch) { + return 0 + } + if (aMatch) { + return -1 + } + return 1 + }) + } + + deleteSession(socketPath: string): void { + logger.debug(`Deleting session from session registry: ${socketPath}`) + this.entries.delete(socketPath) + } + + /** + * Returns the best socket path that we can connect to. + * We also delete any sockets that we can't connect to. + */ + async getConnectedSocketPath(filePath: string): Promise { + const candidates = this.getCandidatesForFile(filePath) + let match: EditorSessionEntry | undefined = undefined + + for (const candidate of candidates) { + if (await canConnect(candidate.socketPath)) { + match = candidate + break + } + this.deleteSession(candidate.socketPath) + } + + return match?.socketPath + } +} + +export class EditorSessionManagerClient { + constructor(private codeServerSocketPath: string) {} + + async canConnect() { + return canConnect(this.codeServerSocketPath) + } + + async getConnectedSocketPath(filePath: string): Promise { + const response = await new Promise((resolve, reject) => { + const opts = { + path: "/session?filePath=" + encodeURIComponent(filePath), + socketPath: this.codeServerSocketPath, + method: "GET", + } + const req = http.request(opts, (res) => { + let rawData = "" + res.setEncoding("utf8") + res.on("data", (chunk) => { + rawData += chunk + }) + res.on("end", () => { + try { + const obj = JSON.parse(rawData) + if (res.statusCode === 200) { + resolve(obj) + } else { + reject(new Error("Unexpected status code: " + res.statusCode)) + } + } catch (e: unknown) { + reject(e) + } + }) + }) + req.on("error", reject) + req.end() + }) + return response.socketPath + } + + // Currently only used for tests. + async addSession(request: AddSessionRequest): Promise { + await new Promise((resolve, reject) => { + const opts = { + path: "/add-session", + socketPath: this.codeServerSocketPath, + method: "POST", + headers: { + "content-type": "application/json", + accept: "application/json", + }, + } + const req = http.request(opts, () => { + resolve() + }) + req.on("error", reject) + req.write(JSON.stringify(request)) + req.end() + }) + } +} diff --git a/test/unit/node/app.test.ts b/test/unit/node/app.test.ts index b2c169a2e..5ce0d07f0 100644 --- a/test/unit/node/app.test.ts +++ b/test/unit/node/app.test.ts @@ -17,7 +17,7 @@ describe("createApp", () => { beforeAll(async () => { mockLogger() - const testName = "unlink-socket" + const testName = "app" await clean(testName) tmpDirPath = await tmpdir(testName) tmpFilePath = path.join(tmpDirPath, "unlink-socket-file") @@ -103,7 +103,7 @@ describe("createApp", () => { const app = await createApp(defaultArgs) - expect(unlinkSpy).toHaveBeenCalledTimes(1) + expect(unlinkSpy).toHaveBeenCalledWith(tmpFilePath) app.dispose() }) diff --git a/test/unit/node/cli.test.ts b/test/unit/node/cli.test.ts index 0d84d72ae..f8cb5b0c9 100644 --- a/test/unit/node/cli.test.ts +++ b/test/unit/node/cli.test.ts @@ -1,14 +1,11 @@ import { Level, logger } from "@coder/logger" import { promises as fs } from "fs" -import * as net from "net" -import * as os from "os" import * as path from "path" import { UserProvidedArgs, bindAddrFromArgs, defaultConfigFile, parse, - readSocketPath, setDefaults, shouldOpenInExistingInstance, toCodeArgs, @@ -20,7 +17,13 @@ import { } from "../../../src/node/cli" import { shouldSpawnCliProcess } from "../../../src/node/main" import { generatePassword, paths } from "../../../src/node/util" -import { clean, useEnv, tmpdir } from "../../utils/helpers" +import { + DEFAULT_SOCKET_PATH, + EditorSessionManager, + EditorSessionManagerClient, + makeEditorSessionManagerServer, +} from "../../../src/node/vscodeSocket" +import { clean, useEnv, tmpdir, listenOn } from "../../utils/helpers" // The parser should not set any defaults so the caller can determine what // values the user actually set. These are only set after explicitly calling @@ -487,7 +490,7 @@ describe("parser", () => { describe("cli", () => { const testName = "cli" - const vscodeIpcPath = path.join(os.tmpdir(), "vscode-ipc") + let tmpDirPath: string beforeAll(async () => { await clean(testName) @@ -495,7 +498,7 @@ describe("cli", () => { beforeEach(async () => { delete process.env.VSCODE_IPC_HOOK_CLI - await fs.rm(vscodeIpcPath, { force: true, recursive: true }) + tmpDirPath = await tmpdir(testName) }) it("should use existing if inside code-server", async () => { @@ -509,54 +512,152 @@ describe("cli", () => { }) it("should use existing if --reuse-window is set", async () => { + const server = await makeEditorSessionManagerServer(DEFAULT_SOCKET_PATH, new EditorSessionManager()) + const args: UserProvidedArgs = {} args["reuse-window"] = true await expect(shouldOpenInExistingInstance(args)).resolves.toStrictEqual(undefined) - await fs.writeFile(vscodeIpcPath, "test") - await expect(shouldOpenInExistingInstance(args)).resolves.toStrictEqual("test") + const socketPath = path.join(tmpDirPath, "socket") + const client = new EditorSessionManagerClient(DEFAULT_SOCKET_PATH) + await client.addSession({ + entry: { + workspace: { + id: "aaa", + folders: [ + { + uri: { + path: "/aaa", + }, + }, + ], + }, + socketPath, + }, + }) + const vscodeSockets = listenOn(socketPath) + + await expect(shouldOpenInExistingInstance(args)).resolves.toStrictEqual(socketPath) args.port = 8081 - await expect(shouldOpenInExistingInstance(args)).resolves.toStrictEqual("test") + await expect(shouldOpenInExistingInstance(args)).resolves.toStrictEqual(socketPath) + + server.close() + vscodeSockets.close() }) it("should use existing if --new-window is set", async () => { + const server = await makeEditorSessionManagerServer(DEFAULT_SOCKET_PATH, new EditorSessionManager()) + const args: UserProvidedArgs = {} args["new-window"] = true - expect(await shouldOpenInExistingInstance(args)).toStrictEqual(undefined) + await expect(shouldOpenInExistingInstance(args)).resolves.toStrictEqual(undefined) - await fs.writeFile(vscodeIpcPath, "test") - expect(await shouldOpenInExistingInstance(args)).toStrictEqual("test") + const socketPath = path.join(tmpDirPath, "socket") + const client = new EditorSessionManagerClient(DEFAULT_SOCKET_PATH) + await client.addSession({ + entry: { + workspace: { + id: "aaa", + folders: [ + { + uri: { + path: "/aaa", + }, + }, + ], + }, + socketPath, + }, + }) + const vscodeSockets = listenOn(socketPath) + + expect(await shouldOpenInExistingInstance(args)).toStrictEqual(socketPath) args.port = 8081 - expect(await shouldOpenInExistingInstance(args)).toStrictEqual("test") + expect(await shouldOpenInExistingInstance(args)).toStrictEqual(socketPath) + + server.close() + vscodeSockets.close() }) it("should use existing if no unrelated flags are set, has positional, and socket is active", async () => { + const server = await makeEditorSessionManagerServer(DEFAULT_SOCKET_PATH, new EditorSessionManager()) + const args: UserProvidedArgs = {} expect(await shouldOpenInExistingInstance(args)).toStrictEqual(undefined) args._ = ["./file"] expect(await shouldOpenInExistingInstance(args)).toStrictEqual(undefined) - const testDir = await tmpdir(testName) - const socketPath = path.join(testDir, "socket") - await fs.writeFile(vscodeIpcPath, socketPath) - expect(await shouldOpenInExistingInstance(args)).toStrictEqual(undefined) - - await new Promise((resolve) => { - const server = net.createServer(() => { - // Close after getting the first connection. - server.close() - }) - server.once("listening", () => resolve(server)) - server.listen(socketPath) + const client = new EditorSessionManagerClient(DEFAULT_SOCKET_PATH) + const socketPath = path.join(tmpDirPath, "socket") + await client.addSession({ + entry: { + workspace: { + id: "aaa", + folders: [ + { + uri: { + path: "/aaa", + }, + }, + ], + }, + socketPath, + }, }) + const vscodeSockets = listenOn(socketPath) expect(await shouldOpenInExistingInstance(args)).toStrictEqual(socketPath) args.port = 8081 expect(await shouldOpenInExistingInstance(args)).toStrictEqual(undefined) + + server.close() + vscodeSockets.close() + }) + + it("should prefer matching sessions for only the first path", async () => { + const server = await makeEditorSessionManagerServer(DEFAULT_SOCKET_PATH, new EditorSessionManager()) + const client = new EditorSessionManagerClient(DEFAULT_SOCKET_PATH) + await client.addSession({ + entry: { + workspace: { + id: "aaa", + folders: [ + { + uri: { + path: "/aaa", + }, + }, + ], + }, + socketPath: `${tmpDirPath}/vscode-ipc-aaa.sock`, + }, + }) + await client.addSession({ + entry: { + workspace: { + id: "bbb", + folders: [ + { + uri: { + path: "/bbb", + }, + }, + ], + }, + socketPath: `${tmpDirPath}/vscode-ipc-bbb.sock`, + }, + }) + listenOn(`${tmpDirPath}/vscode-ipc-aaa.sock`, `${tmpDirPath}/vscode-ipc-bbb.sock`) + + const args: UserProvidedArgs = {} + args._ = ["/aaa/file", "/bbb/file"] + expect(await shouldOpenInExistingInstance(args)).toStrictEqual(`${tmpDirPath}/vscode-ipc-aaa.sock`) + + server.close() }) }) @@ -729,44 +830,6 @@ cert: false`) }) }) -describe("readSocketPath", () => { - const fileContents = "readSocketPath file contents" - let tmpDirPath: string - let tmpFilePath: string - - const testName = "readSocketPath" - beforeAll(async () => { - await clean(testName) - }) - - beforeEach(async () => { - tmpDirPath = await tmpdir(testName) - tmpFilePath = path.join(tmpDirPath, "readSocketPath.txt") - await fs.writeFile(tmpFilePath, fileContents) - }) - - it("should throw an error if it can't read the file", async () => { - // TODO@jsjoeio - implement - // Test it on a directory.... ESDIR - // TODO@jsjoeio - implement - expect(() => readSocketPath(tmpDirPath)).rejects.toThrow("EISDIR") - }) - it("should return undefined if it can't read the file", async () => { - // TODO@jsjoeio - implement - const socketPath = await readSocketPath(path.join(tmpDirPath, "not-a-file")) - expect(socketPath).toBeUndefined() - }) - it("should return the file contents", async () => { - const contents = await readSocketPath(tmpFilePath) - expect(contents).toBe(fileContents) - }) - it("should return the same file contents for two different calls", async () => { - const contents1 = await readSocketPath(tmpFilePath) - const contents2 = await readSocketPath(tmpFilePath) - expect(contents2).toBe(contents1) - }) -}) - describe("toCodeArgs", () => { const vscodeDefaults = { ...defaults, diff --git a/test/unit/node/vscodeSocket.test.ts b/test/unit/node/vscodeSocket.test.ts new file mode 100644 index 000000000..7c5999415 --- /dev/null +++ b/test/unit/node/vscodeSocket.test.ts @@ -0,0 +1,243 @@ +import { EditorSessionManager } from "../../../src/node/vscodeSocket" +import { clean, tmpdir, listenOn } from "../../utils/helpers" + +describe("EditorSessionManager", () => { + let tmpDirPath: string + + const testName = "esm" + + beforeAll(async () => { + await clean(testName) + }) + + beforeEach(async () => { + tmpDirPath = await tmpdir(testName) + }) + + describe("getCandidatesForFile", () => { + it("should prefer the last added socket path for a matching path", async () => { + const manager = new EditorSessionManager() + manager.addSession({ + workspace: { + id: "aaa", + folders: [ + { + uri: { + path: "/aaa", + }, + }, + ], + }, + socketPath: `${tmpDirPath}/vscode-ipc-aaa-1.sock`, + }) + manager.addSession({ + workspace: { + id: "aaa", + folders: [ + { + uri: { + path: "/aaa", + }, + }, + ], + }, + socketPath: `${tmpDirPath}/vscode-ipc-aaa-2.sock`, + }) + manager.addSession({ + workspace: { + id: "bbb", + folders: [ + { + uri: { + path: "/bbb", + }, + }, + ], + }, + socketPath: `${tmpDirPath}/vscode-ipc-bbb.sock`, + }) + const socketPaths = manager.getCandidatesForFile("/aaa/some-file:1:1") + expect(socketPaths.map((x) => x.socketPath)).toEqual([ + // Matches + `${tmpDirPath}/vscode-ipc-aaa-2.sock`, + `${tmpDirPath}/vscode-ipc-aaa-1.sock`, + // Non-matches + `${tmpDirPath}/vscode-ipc-bbb.sock`, + ]) + }) + + it("should return the last added socketPath if there are no matches", async () => { + const manager = new EditorSessionManager() + manager.addSession({ + workspace: { + id: "aaa", + folders: [ + { + uri: { + path: "/aaa", + }, + }, + ], + }, + socketPath: `${tmpDirPath}/vscode-ipc-aaa.sock`, + }) + manager.addSession({ + workspace: { + id: "bbb", + folders: [ + { + uri: { + path: "/bbb", + }, + }, + ], + }, + socketPath: `${tmpDirPath}/vscode-ipc-bbb.sock`, + }) + const socketPaths = manager.getCandidatesForFile("/ccc/some-file:1:1") + expect(socketPaths.map((x) => x.socketPath)).toEqual([ + `${tmpDirPath}/vscode-ipc-bbb.sock`, + `${tmpDirPath}/vscode-ipc-aaa.sock`, + ]) + }) + + it("does not just directly do a substring match", async () => { + const manager = new EditorSessionManager() + manager.addSession({ + workspace: { + id: "aaa", + folders: [ + { + uri: { + path: "/aaa", + }, + }, + ], + }, + socketPath: `${tmpDirPath}/vscode-ipc-aaa.sock`, + }) + manager.addSession({ + workspace: { + id: "bbb", + folders: [ + { + uri: { + path: "/bbb", + }, + }, + ], + }, + socketPath: `${tmpDirPath}/vscode-ipc-bbb.sock`, + }) + const entries = manager.getCandidatesForFile("/aaaxxx/some-file:1:1") + expect(entries.map((x) => x.socketPath)).toEqual([ + `${tmpDirPath}/vscode-ipc-bbb.sock`, + `${tmpDirPath}/vscode-ipc-aaa.sock`, + ]) + }) + }) + + describe("getConnectedSocketPath", () => { + it("should return socket path if socket is active", async () => { + listenOn(`${tmpDirPath}/vscode-ipc-aaa.sock`).once() + const manager = new EditorSessionManager() + manager.addSession({ + workspace: { + id: "aaa", + folders: [ + { + uri: { + path: "/aaa", + }, + }, + ], + }, + socketPath: `${tmpDirPath}/vscode-ipc-aaa.sock`, + }) + const socketPath = await manager.getConnectedSocketPath("/aaa/some-file:1:1") + expect(socketPath).toBe(`${tmpDirPath}/vscode-ipc-aaa.sock`) + }) + + it("should return undefined if socket is inactive", async () => { + const manager = new EditorSessionManager() + manager.addSession({ + workspace: { + id: "aaa", + folders: [ + { + uri: { + path: "/aaa", + }, + }, + ], + }, + socketPath: `${tmpDirPath}/vscode-ipc-aaa.sock`, + }) + const socketPath = await manager.getConnectedSocketPath("/aaa/some-file:1:1") + expect(socketPath).toBeUndefined() + }) + + it("should return undefined given no matching active sockets", async () => { + const vscodeSockets = listenOn(`${tmpDirPath}/vscode-ipc-bbb.sock`) + const manager = new EditorSessionManager() + manager.addSession({ + workspace: { + id: "aaa", + folders: [ + { + uri: { + path: "/aaa", + }, + }, + ], + }, + socketPath: `${tmpDirPath}/vscode-ipc-aaa.sock`, + }) + const socketPath = await manager.getConnectedSocketPath("/aaa/some-file:1:1") + expect(socketPath).toBeUndefined() + vscodeSockets.close() + }) + + it("should return undefined if there are no entries", async () => { + const manager = new EditorSessionManager() + const socketPath = await manager.getConnectedSocketPath("/aaa/some-file:1:1") + expect(socketPath).toBeUndefined() + }) + + it("should return most recently used socket path available", async () => { + listenOn(`${tmpDirPath}/vscode-ipc-aaa-1.sock`).once() + const manager = new EditorSessionManager() + manager.addSession({ + workspace: { + id: "aaa", + folders: [ + { + uri: { + path: "/aaa", + }, + }, + ], + }, + socketPath: `${tmpDirPath}/vscode-ipc-aaa-1.sock`, + }) + manager.addSession({ + workspace: { + id: "aaa", + folders: [ + { + uri: { + path: "/aaa", + }, + }, + ], + }, + socketPath: `${tmpDirPath}/vscode-ipc-aaa-2.sock`, + }) + + const socketPath = await manager.getConnectedSocketPath("/aaa/some-file:1:1") + expect(socketPath).toBe(`${tmpDirPath}/vscode-ipc-aaa-1.sock`) + // Failed sockets should be removed from the entries. + expect((manager as any).entries.has(`${tmpDirPath}/vscode-ipc-aaa-2.sock`)).toBe(false) + }) + }) +}) diff --git a/test/utils/helpers.ts b/test/utils/helpers.ts index cd54be486..52a0e85b4 100644 --- a/test/utils/helpers.ts +++ b/test/utils/helpers.ts @@ -150,3 +150,52 @@ export function getMaybeProxiedPathname(url: URL): string { return url.pathname } + +interface FakeVscodeSockets { + /* If called, closes all servers after the first connection. */ + once(): FakeVscodeSockets + + /* Manually close all servers. */ + close(): Promise +} + +/** + * Creates servers for each socketPath specified. + */ +export function listenOn(...socketPaths: string[]): FakeVscodeSockets { + let once = false + const servers = socketPaths.map((socketPath) => { + const server = net.createServer(() => { + if (once) { + close() + } + }) + server.listen(socketPath) + return server + }) + + async function close() { + await Promise.all( + servers.map( + (server) => + new Promise((resolve, reject) => { + server.close((err) => { + if (err) { + reject(err) + return + } + resolve() + }) + }), + ), + ) + } + const fakeVscodeSockets = { + close, + once: () => { + once = true + return fakeVscodeSockets + }, + } + return fakeVscodeSockets +}