explicitly check home dir for user config (#558)

Author: mythmon

src/commandInstruction.ts

@@ -1,3 +1,4 @@
+import {getObservableUiOrigin} from "./observableApiClient.js";
 import type {TtyColor} from "./tty.js";
 import {bold, magenta} from "./tty.js";
 
@@ -21,3 +22,7 @@ export function commandInstruction(
 
   return color(`${prefix} ${command}`);
 }
+
+export const commandRequiresAuthenticationMessage = `You need to be authenticated to ${
+  getObservableUiOrigin().hostname
+} to run this command. Please run ${commandInstruction("login")}.`;

src/deploy.ts

@@ -6,9 +6,11 @@ import type {Config} from "./config.js";
 import {CliError, isHttpError} from "./error.js";
 import type {Logger, Writer} from "./logger.js";
 import {ObservableApiClient} from "./observableApiClient.js";
+import type {ConfigEffects} from "./observableApiConfig.js";
 import {
   type ApiKey,
   type DeployConfig,
+  defaultEffects as defaultConfigEffects,
   getDeployConfig,
   getObservableApiKey,
   setDeployConfig
@@ -20,8 +22,8 @@ export interface DeployOptions {
   config: Config;
 }
 
-export interface DeployEffects {
-  getObservableApiKey: (logger: Logger) => Promise<ApiKey>;
+export interface DeployEffects extends ConfigEffects {
+  getObservableApiKey: (effects?: DeployEffects) => Promise<ApiKey>;
   getDeployConfig: (sourceRoot: string) => Promise<DeployConfig | null>;
   setDeployConfig: (sourceRoot: string, config: DeployConfig) => Promise<void>;
   isTty: boolean;
@@ -32,6 +34,7 @@ export interface DeployEffects {
 }
 
 const defaultEffects: DeployEffects = {
+  ...defaultConfigEffects,
   getObservableApiKey,
   getDeployConfig,
   setDeployConfig,
@@ -46,7 +49,7 @@ const defaultEffects: DeployEffects = {
 export async function deploy({config}: DeployOptions, effects = defaultEffects): Promise<void> {
   Telemetry.record({event: "deploy", step: "start"});
   const {logger} = effects;
-  const apiKey = await effects.getObservableApiKey(logger);
+  const apiKey = await effects.getObservableApiKey(effects);
   const apiClient = new ObservableApiClient({apiKey});
 
   // Check configuration

src/observableApiAuth.ts

@@ -9,26 +9,29 @@ import {commandInstruction} from "./commandInstruction.js";
 import {HttpError, isHttpError} from "./error.js";
 import type {Logger} from "./logger.js";
 import {ObservableApiClient, getObservableUiOrigin} from "./observableApiClient.js";
-import {type ApiKey, getObservableApiKey, setObservableApiKey} from "./observableApiConfig.js";
+import type {ConfigEffects} from "./observableApiConfig.js";
+import {
+  type ApiKey,
+  defaultEffects as defaultConfigEffects,
+  getObservableApiKey,
+  setObservableApiKey
+} from "./observableApiConfig.js";
 
 const OBSERVABLE_UI_ORIGIN = getObservableUiOrigin();
 
-export const commandRequiresAuthenticationMessage = `You need to be authenticated to ${
-  getObservableUiOrigin().hostname
-} to run this command. Please run ${commandInstruction("login")}.`;
-
 /** Actions this command needs to take wrt its environment that may need mocked out. */
-export interface CommandEffects {
+export interface AuthEffects extends ConfigEffects {
   openUrlInBrowser: (url: string) => Promise<void>;
   logger: Logger;
   isTty: boolean;
   waitForEnter: () => Promise<void>;
-  getObservableApiKey: (logger: Logger) => Promise<ApiKey>;
+  getObservableApiKey: (effects: AuthEffects) => Promise<ApiKey>;
   setObservableApiKey: (info: {id: string; key: string} | null) => Promise<void>;
   exitSuccess: () => void;
 }
 
-const defaultEffects: CommandEffects = {
+const defaultEffects: AuthEffects = {
+  ...defaultConfigEffects,
   openUrlInBrowser: async (target) => void (await open(target)),
   logger: console,
   isTty: isatty(process.stdin.fd),
@@ -73,7 +76,7 @@ export async function logout(effects = defaultEffects) {
 
 export async function whoami(effects = defaultEffects) {
   const {logger} = effects;
-  const apiKey = await effects.getObservableApiKey(logger);
+  const apiKey = await effects.getObservableApiKey(effects);
   const apiClient = new ObservableApiClient({apiKey});
 
   try {
@@ -104,11 +107,11 @@ export async function whoami(effects = defaultEffects) {
 class LoginServer {
   private _server: ReturnType<typeof createServer>;
   private _nonce: string;
-  private _effects: CommandEffects;
+  private _effects: AuthEffects;
   isRunning: boolean;
   private _sockets: Set<Socket>;
 
-  constructor({nonce, effects}: {nonce: string; effects: CommandEffects}) {
+  constructor({nonce, effects}: {nonce: string; effects: AuthEffects}) {
     this._nonce = nonce;
     this._effects = effects;
     this._server = createServer();

src/observableApiConfig.ts

@@ -1,9 +1,28 @@
 import fs from "node:fs/promises";
 import os from "node:os";
 import path from "node:path";
-import {isEnoent} from "./error.js";
-import type {Logger} from "./logger.js";
-import {commandRequiresAuthenticationMessage} from "./observableApiAuth.js";
+import {commandRequiresAuthenticationMessage} from "./commandInstruction.js";
+import {CliError, isEnoent} from "./error.js";
+
+export interface ConfigEffects {
+  readFile: (path: string, encoding: "utf8") => Promise<string>;
+  writeFile: (path: string, contents: string) => Promise<void>;
+  env: typeof process.env;
+  cwd: typeof process.cwd;
+  mkdir: (path: string, options?: {recursive?: boolean}) => Promise<void>;
+  homedir: typeof os.homedir;
+}
+
+export const defaultEffects: ConfigEffects = {
+  readFile: (path, encoding) => fs.readFile(path, encoding),
+  writeFile: fs.writeFile,
+  mkdir: async (path, options) => {
+    await fs.mkdir(path, options);
+  },
+  env: process.env,
+  cwd: process.cwd,
+  homedir: os.homedir
+};
 
 const userConfigName = ".observablehq";
 interface UserConfig {
@@ -22,17 +41,16 @@ export type ApiKey =
   | {source: "env"; envVar: string; key: string}
   | {source: "test"; key: string};
 
-export async function getObservableApiKey(logger: Logger = console): Promise<ApiKey> {
+export async function getObservableApiKey(effects: ConfigEffects = defaultEffects): Promise<ApiKey> {
   const envVar = "OBSERVABLE_TOKEN";
-  if (process.env[envVar]) {
-    return {source: "env", envVar, key: process.env[envVar]};
+  if (effects.env[envVar]) {
+    return {source: "env", envVar, key: effects.env[envVar]};
   }
   const {config, configPath} = await loadUserConfig();
   if (config.auth?.key) {
     return {source: "file", filePath: configPath, key: config.auth.key};
   }
-  logger.log(commandRequiresAuthenticationMessage);
-  process.exit(1);
+  throw new CliError(commandRequiresAuthenticationMessage);
 }
 
 export async function setObservableApiKey(info: null | {id: string; key: string}): Promise<void> {
@@ -45,11 +63,14 @@ export async function setObservableApiKey(info: null | {id: string; key: string}
   await writeUserConfig({config, configPath});
 }
 
-export async function getDeployConfig(sourceRoot: string): Promise<DeployConfig | null> {
-  const deployConfigPath = path.join(process.cwd(), sourceRoot, ".observablehq", "deploy.json");
+export async function getDeployConfig(
+  sourceRoot: string,
+  effects: ConfigEffects = defaultEffects
+): Promise<DeployConfig | null> {
+  const deployConfigPath = path.join(effects.cwd(), sourceRoot, ".observablehq", "deploy.json");
   let content: string | null = null;
   try {
-    content = await fs.readFile(deployConfigPath, "utf8");
+    content = await effects.readFile(deployConfigPath, "utf8");
   } catch (error) {
     content = "{}";
   }
@@ -59,41 +80,58 @@ export async function getDeployConfig(sourceRoot: string): Promise<DeployConfig
   return {projectId};
 }
 
-export async function setDeployConfig(sourceRoot: string, newConfig: DeployConfig): Promise<void> {
-  const dir = path.join(process.cwd(), sourceRoot, ".observablehq");
+export async function setDeployConfig(
+  sourceRoot: string,
+  newConfig: DeployConfig,
+  effects: ConfigEffects = defaultEffects
+): Promise<void> {
+  const dir = path.join(effects.cwd(), sourceRoot, ".observablehq");
   const deployConfigPath = path.join(dir, "deploy.json");
   const oldConfig = (await getDeployConfig(sourceRoot)) || {};
   const merged = {...oldConfig, ...newConfig};
-  await fs.mkdir(dir, {recursive: true});
-  await fs.writeFile(deployConfigPath, JSON.stringify(merged, null, 2) + "\n");
+  await effects.mkdir(dir, {recursive: true});
+  await effects.writeFile(deployConfigPath, JSON.stringify(merged, null, 2) + "\n");
 }
 
-async function loadUserConfig(): Promise<{configPath: string; config: UserConfig}> {
-  let cursor = path.resolve(process.cwd());
-  while (true) {
-    const configPath = path.join(cursor, userConfigName);
+export async function loadUserConfig(
+  effects: ConfigEffects = defaultEffects
+): Promise<{configPath: string; config: UserConfig}> {
+  const homeConfigPath = path.join(effects.homedir(), userConfigName);
+
+  function* pathsToTry(): Generator<string> {
+    let cursor = path.resolve(effects.cwd());
+    while (true) {
+      yield path.join(cursor, userConfigName);
+      const nextCursor = path.dirname(cursor);
+      if (nextCursor === cursor) break;
+      cursor = nextCursor;
+    }
+    yield homeConfigPath;
+  }
+
+  for (const configPath of pathsToTry()) {
     let content: string | null = null;
     try {
-      content = await fs.readFile(configPath, "utf8");
+      content = await effects.readFile(configPath, "utf8");
     } catch (error) {
       if (!isEnoent(error)) throw error;
-      const nextCursor = path.dirname(cursor);
-      if (nextCursor === cursor) break;
-      cursor = nextCursor;
     }
 
     if (content !== null) {
       try {
         return {config: JSON.parse(content), configPath};
-      } catch (err) {
-        console.error(`Problem parsing config file at ${configPath}: ${err}`);
+      } catch (error) {
+        throw new CliError(`Problem parsing config file at ${configPath}: ${error}`, {cause: error});
       }
     }
   }
 
-  return {config: {}, configPath: path.join(os.homedir(), userConfigName)};
+  return {config: {}, configPath: homeConfigPath};
 }
 
-async function writeUserConfig({configPath, config}: {configPath: string; config: UserConfig}): Promise<void> {
-  await fs.writeFile(configPath, JSON.stringify(config, null, 2));
+async function writeUserConfig(
+  {configPath, config}: {configPath: string; config: UserConfig},
+  effects: ConfigEffects = defaultEffects
+): Promise<void> {
+  await effects.writeFile(configPath, JSON.stringify(config, null, 2));
 }

test/deploy-test.ts

@@ -1,15 +1,15 @@
 import assert, {fail} from "node:assert";
 import {Readable, Writable} from "node:stream";
+import {commandRequiresAuthenticationMessage} from "../src/commandInstruction.js";
 import {normalizeConfig} from "../src/config.js";
 import type {DeployEffects} from "../src/deploy.js";
 import {deploy, promptConfirm} from "../src/deploy.js";
 import {CliError, isHttpError} from "../src/error.js";
-import type {Logger} from "../src/logger.js";
-import {commandRequiresAuthenticationMessage} from "../src/observableApiAuth.js";
 import type {DeployConfig} from "../src/observableApiConfig.js";
 import {MockLogger} from "./mocks/logger.js";
 import {getCurentObservableApi, mockObservableApi} from "./mocks/observableApi.js";
 import {invalidApiKey, validApiKey} from "./mocks/observableApi.js";
+import {MockConfigEffects} from "./observableApiConfig-test.js";
 
 // These files are implicitly generated by the CLI. This may change over time,
 // so they’re enumerated here for clarity. TODO We should enforce that these
@@ -39,7 +39,7 @@ interface MockDeployEffectsOptions {
   debug?: boolean;
 }
 
-class MockDeployEffects implements DeployEffects {
+class MockDeployEffects extends MockConfigEffects implements DeployEffects {
   public logger = new MockLogger();
   public input = new Readable();
   public output: NodeJS.WritableStream;
@@ -59,6 +59,7 @@ class MockDeployEffects implements DeployEffects {
     outputColumns = 80,
     debug = false
   }: MockDeployEffectsOptions = {}) {
+    super();
     this.observableApiKey = apiKey;
     this.deployConfig = deployConfig;
     this.isTty = isTty;
@@ -88,9 +89,10 @@ class MockDeployEffects implements DeployEffects {
     });
   }
 
-  async getObservableApiKey(logger: Logger) {
+  async getObservableApiKey(effects: DeployEffects = this) {
+    if (effects !== this) throw new Error("don't pass unrelated effects to mock effects methods");
     if (!this.observableApiKey) {
-      logger.log(commandRequiresAuthenticationMessage);
+      effects?.logger.log(commandRequiresAuthenticationMessage);
       throw new Error("no key available in this test");
     }
     return {source: "test" as const, key: this.observableApiKey};