From 3acd259e3cc8fd1cf853dabef67c544fe406851d Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 29 Sep 2021 15:36:04 -0700 Subject: [PATCH 1/7] feat(cli): add test for readSocketPath --- src/node/cli.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/node/cli.ts b/src/node/cli.ts index 088431b7aa02..e713dc9bcb6b 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -667,6 +667,7 @@ export const shouldOpenInExistingInstance = async (args: Args): Promise => { try { return await fs.readFile(path.join(os.tmpdir(), "vscode-ipc"), "utf8") From c8964c76abb606d5454fb649f9e4886ab9e513fe Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 29 Sep 2021 15:43:25 -0700 Subject: [PATCH 2/7] refactor(cli): clean up and export readSocketPath --- src/node/cli.ts | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/node/cli.ts b/src/node/cli.ts index e713dc9bcb6b..3607948847e9 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -656,6 +656,16 @@ export const shouldRunVsCodeCli = (args: Args): boolean => { return extensionRelatedArgs.some((arg) => argKeys.includes(arg)) } +export async function readSocketPath(): Promise { + try { + return await fs.readFile(path.join(os.tmpdir(), "vscode-ipc"), "utf8") + } catch (error) { + if (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 @@ -667,18 +677,6 @@ export const shouldOpenInExistingInstance = async (args: Args): Promise => { - try { - return await fs.readFile(path.join(os.tmpdir(), "vscode-ipc"), "utf8") - } catch (error) { - if (error.code !== "ENOENT") { - throw error - } - } - 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) => { From 32efeab11eb3684767332ce621af74e0cf2b1ce7 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 29 Sep 2021 15:43:47 -0700 Subject: [PATCH 3/7] fixup --- src/node/cli.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/node/cli.ts b/src/node/cli.ts index 3607948847e9..bc7333350f4d 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -656,6 +656,12 @@ export const shouldRunVsCodeCli = (args: Args): boolean => { return extensionRelatedArgs.some((arg) => argKeys.includes(arg)) } +/** + * Reads the socketPath which defaults to a temporary directory + * with another directory called vscode-ipc. + * + * If it can't read the path, it throws an error and returns undefined. + */ export async function readSocketPath(): Promise { try { return await fs.readFile(path.join(os.tmpdir(), "vscode-ipc"), "utf8") From 09fa806be09e26c8f43a0d4d67e2e7c7b62d60df Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 29 Sep 2021 15:45:59 -0700 Subject: [PATCH 4/7] fixup --- src/node/cli.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node/cli.ts b/src/node/cli.ts index bc7333350f4d..512a51118419 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -4,7 +4,7 @@ import yaml from "js-yaml" import * as os from "os" import * as path from "path" import { Args as VsArgs } from "../../typings/ipc" -import { canConnect, generateCertificate, generatePassword, humanPath, paths } from "./util" +import { canConnect, generateCertificate, generatePassword, humanPath, isNodeJSErrnoException, paths } from "./util" export enum Feature { /** Web socket compression. */ @@ -666,7 +666,7 @@ export async function readSocketPath(): Promise { try { return await fs.readFile(path.join(os.tmpdir(), "vscode-ipc"), "utf8") } catch (error) { - if (error.code !== "ENOENT") { + if (!isNodeJSErrnoException(error) || error.code !== "ENOENT") { throw error } } From 20ce76886e94d7fcd74687a18382020f52e7f3d6 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 29 Sep 2021 16:02:27 -0700 Subject: [PATCH 5/7] wip: add one passing test --- test/unit/node/cli.test.ts | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/unit/node/cli.test.ts b/test/unit/node/cli.test.ts index 97b648788440..f1978c4197da 100644 --- a/test/unit/node/cli.test.ts +++ b/test/unit/node/cli.test.ts @@ -8,6 +8,7 @@ import { bindAddrFromArgs, defaultConfigFile, parse, + readSocketPath, setDefaults, shouldOpenInExistingInstance, shouldRunVsCodeCli, @@ -655,3 +656,33 @@ password: ${password} cert: false`) }) }) + +describe.only("readSocketPath", () => { + const vscodeIpcPath = path.join(os.tmpdir(), "vscode-ipc") + const fileContents = "readSocketPath file contents" + + beforeEach(async () => { + await fs.writeFile(vscodeIpcPath, fileContents) + }) + + afterEach(async () => { + await fs.rmdir(vscodeIpcPath, { recursive: true }) + }) + + it.skip("should throw an error if it can't read the file", async () => { + // TODO@jsjoeio - implement + // TODO@jsjoeio - implement + const p = await readSocketPath() + console.log(p) + expect(p).toThrowError("oops") + }) + it.skip("should return undefined if it can't read the file", async () => { + // TODO@jsjoeio - implement + const socketPath = await readSocketPath() + expect(socketPath).toBeUndefined() + }) + it("should return the file contents", async () => { + const contents = await readSocketPath() + expect(contents).toBe(fileContents) + }) +}) From e107a1546a6c10aea473cd1e9afda1a302673c1d Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Wed, 29 Sep 2021 16:03:27 -0700 Subject: [PATCH 6/7] wip: add notes --- src/node/cli.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/node/cli.ts b/src/node/cli.ts index 512a51118419..ba1fc6ef096c 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -663,6 +663,8 @@ export const shouldRunVsCodeCli = (args: Args): boolean => { * If it can't read the path, it throws an error and returns undefined. */ export async function readSocketPath(): Promise { + // TODO@jsjoeio - we should make this function pure and pass in the path + // to the file it reads try { return await fs.readFile(path.join(os.tmpdir(), "vscode-ipc"), "utf8") } catch (error) { From 87370ed0b7319a712152416d4130ee94e2da16a1 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Thu, 30 Sep 2021 16:34:17 -0700 Subject: [PATCH 7/7] refactor: clean up --- src/node/cli.ts | 16 ++++++++++------ src/node/util.ts | 2 +- test/unit/node/cli.test.ts | 35 +++++++++++++++++++++-------------- 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/node/cli.ts b/src/node/cli.ts index ba1fc6ef096c..63076ee49db5 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -6,6 +6,8 @@ import * as path from "path" import { Args as VsArgs } from "../../typings/ipc" import { canConnect, generateCertificate, generatePassword, humanPath, isNodeJSErrnoException, paths } from "./util" +const DEFAULT_SOCKET_PATH = path.join(os.tmpdir(), "vscode-ipc") + export enum Feature { /** Web socket compression. */ PermessageDeflate = "permessage-deflate", @@ -662,18 +664,20 @@ export const shouldRunVsCodeCli = (args: Args): boolean => { * * If it can't read the path, it throws an error and returns undefined. */ -export async function readSocketPath(): Promise { - // TODO@jsjoeio - we should make this function pure and pass in the path - // to the file it reads +export async function readSocketPath(path: string): Promise { try { - return await fs.readFile(path.join(os.tmpdir(), "vscode-ipc"), "utf8") + 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 @@ -691,7 +695,7 @@ export const shouldOpenInExistingInstance = async (args: Args): Promise 0) { - return readSocketPath() + return readSocketPath(DEFAULT_SOCKET_PATH) } // It's possible the user is trying to spawn another instance of code-server. @@ -699,7 +703,7 @@ export const shouldOpenInExistingInstance = async (args: Args): Promise 0) { - const socketPath = await readSocketPath() + const socketPath = await readSocketPath(DEFAULT_SOCKET_PATH) if (socketPath && (await canConnect(socketPath))) { return socketPath } diff --git a/src/node/util.ts b/src/node/util.ts index ce92a3522535..e61be4ef8a8a 100644 --- a/src/node/util.ts +++ b/src/node/util.ts @@ -531,5 +531,5 @@ export function escapeHtml(unsafe: string): string { * it has a .code property. */ export function isNodeJSErrnoException(error: unknown): error is NodeJS.ErrnoException { - return error instanceof Error && (error as NodeJS.ErrnoException).code !== undefined + return error !== undefined && (error as NodeJS.ErrnoException).code !== undefined } diff --git a/test/unit/node/cli.test.ts b/test/unit/node/cli.test.ts index f1978c4197da..c6f346d22691 100644 --- a/test/unit/node/cli.test.ts +++ b/test/unit/node/cli.test.ts @@ -14,9 +14,8 @@ import { shouldRunVsCodeCli, splitOnFirstEquals, } from "../../../src/node/cli" -import { tmpdir } from "../../../src/node/constants" import { generatePassword, paths } from "../../../src/node/util" -import { useEnv } from "../../utils/helpers" +import { tmpdir, useEnv } from "../../utils/helpers" type Mutable = { -readonly [P in keyof T]: T[P] @@ -379,10 +378,11 @@ describe("parser", () => { describe("cli", () => { let args: Mutable = { _: [] } - const testDir = path.join(tmpdir, "tests/cli") const vscodeIpcPath = path.join(os.tmpdir(), "vscode-ipc") + let testDir: string beforeAll(async () => { + testDir = await tmpdir("cli") await fs.rmdir(testDir, { recursive: true }) await fs.mkdir(testDir, { recursive: true }) }) @@ -657,32 +657,39 @@ cert: false`) }) }) -describe.only("readSocketPath", () => { - const vscodeIpcPath = path.join(os.tmpdir(), "vscode-ipc") +describe("readSocketPath", () => { const fileContents = "readSocketPath file contents" + let tmpDirPath: string + let tmpFilePath: string beforeEach(async () => { - await fs.writeFile(vscodeIpcPath, fileContents) + tmpDirPath = await tmpdir("readSocketPath") + tmpFilePath = path.join(tmpDirPath, "readSocketPath.txt") + await fs.writeFile(tmpFilePath, fileContents) }) afterEach(async () => { - await fs.rmdir(vscodeIpcPath, { recursive: true }) + await fs.rmdir(tmpDirPath, { recursive: true }) }) - it.skip("should throw an error if it can't read the file", async () => { + 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 - const p = await readSocketPath() - console.log(p) - expect(p).toThrowError("oops") + expect(() => readSocketPath(tmpDirPath)).rejects.toThrow("EISDIR") }) - it.skip("should return undefined if it can't read the file", async () => { + it("should return undefined if it can't read the file", async () => { // TODO@jsjoeio - implement - const socketPath = await readSocketPath() + const socketPath = await readSocketPath(path.join(tmpDirPath, "not-a-file")) expect(socketPath).toBeUndefined() }) it("should return the file contents", async () => { - const contents = await readSocketPath() + 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) + }) })