Improvements to config file handling

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+### Updated
+
+- Made improvements to prevent creation of corrupted config files and gracefully handle the re-creation of corrupted config files.
+
 ### Fixed
 
 - Write the viewer_emails/editor_emails values when initializing studio reports so the values can be carried over/edited from current state on updates.

src/config.test.ts

@@ -37,6 +37,18 @@ describe("resolveWebBaseUrl", () => {
   });
 });
 
+describe("writeConfig", () => {
+  it("writes valid JSON and leaves no temp files behind", () => {
+    writeConfig({ apiBaseUrl: "https://example.com" });
+
+    const configDir = path.dirname(getConfigPath());
+    const entries = fs.readdirSync(configDir);
+
+    expect(entries).toEqual(["config.json"]);
+    expect(readConfig().apiBaseUrl).toBe("https://example.com");
+  });
+});
+
 describe("readConfig", () => {
   it("returns {} for an empty config object on disk", () => {
     const configPath = getConfigPath();
@@ -46,6 +58,22 @@ describe("readConfig", () => {
     expect(readConfig()).toEqual({});
   });
 
+  it("returns {} instead of throwing when the config file is empty or truncated", () => {
+    const configPath = getConfigPath();
+    fs.mkdirSync(path.dirname(configPath), { recursive: true });
+    fs.writeFileSync(configPath, "");
+
+    expect(readConfig()).toEqual({});
+  });
+
+  it("returns {} instead of throwing when the config file contains invalid JSON", () => {
+    const configPath = getConfigPath();
+    fs.mkdirSync(path.dirname(configPath), { recursive: true });
+    fs.writeFileSync(configPath, '{"apiBaseUrl": "https://exampl');
+
+    expect(readConfig()).toEqual({});
+  });
+
   it("prefers apiBaseUrl when both apiBaseUrl and legacy baseUrl are present", () => {
     const configPath = getConfigPath();
     fs.mkdirSync(path.dirname(configPath), { recursive: true });

src/config.ts

@@ -9,7 +9,7 @@ import type {
   VersionPromptSelection,
 } from "./types.js";
 
-const DEFAULT_API_BASE_URL = "https://api.getdx.com";
+export const DEFAULT_API_BASE_URL = "https://api.getdx.com";
 const DEFAULT_WEB_BASE_URL = "https://app.getdx.com";
 
 // --- Config file I/O ---------------------------------------------------------
@@ -25,7 +25,16 @@ export function readConfig(): StoredConfig {
   }
 
   const content = fs.readFileSync(configPath, "utf8");
-  const raw = JSON.parse(content) as Record<string, unknown>;
+
+  let raw: Record<string, unknown>;
+  try {
+    raw = JSON.parse(content) as Record<string, unknown>;
+  } catch {
+    // The config file can be left empty or truncated if a previous `dx`
+    // process was killed mid-write, or two processes wrote concurrently.
+    // Treat it like a missing config rather than crashing every command.
+    return {};
+  }
 
   const stored: StoredConfig = {};
   if (typeof raw.webBaseUrl === "string") {
@@ -64,12 +73,19 @@ export function persistVersionPromptSelection(
 }
 
 export function writeConfig(config: StoredConfig): void {
-  fs.mkdirSync(getConfigDir(), { recursive: true });
-  fs.writeFileSync(
-    getConfigPath(),
-    JSON.stringify(config, null, 2) + "\n",
-    "utf8",
+  const configDir = getConfigDir();
+  fs.mkdirSync(configDir, { recursive: true });
+
+  const configPath = getConfigPath();
+  // Write to a temp file and rename into place so a crash or a race between
+  // concurrent `dx` processes can't leave config.json empty or truncated —
+  // renames are atomic, so readers always see either the old or new content.
+  const tempPath = path.join(
+    configDir,
+    `.config.json.${process.pid}.${Date.now()}.tmp`,
   );
+  fs.writeFileSync(tempPath, JSON.stringify(config, null, 2) + "\n", "utf8");
+  fs.renameSync(tempPath, configPath);
 }
 
 export function persistBaseUrls(apiBaseUrl: string, webBaseUrl: string): void {

src/loginViaBrowser.test.ts

@@ -166,6 +166,29 @@ describe("loginViaBrowser", () => {
     );
   });
 
+  it("throws CliError when token exchange returns an ok status with an invalid body", async () => {
+    let capturedState = "";
+    mockOpen.mockImplementation(async (url: string) => {
+      capturedState = new URL(url).searchParams.get("state") ?? "";
+    });
+
+    mockListenForCode.mockImplementation(
+      async (callback: (state: string, code: string) => Promise<unknown>) => {
+        return callback(capturedState, "auth-code-xyz");
+      },
+    );
+
+    vi.stubGlobal(
+      "fetch",
+      vi.fn().mockResolvedValue(new Response("", { status: 200 })),
+    );
+
+    const { loginViaBrowser } = await import("./loginViaBrowser.js");
+    await expect(loginViaBrowser("https://app.example.com")).rejects.toThrow(
+      "Authentication failed: token exchange returned an invalid response",
+    );
+  });
+
   it("prints a fallback URL when the browser cannot be opened", async () => {
     const writes: string[] = [];
     vi.spyOn(process.stdout, "write").mockImplementation(((

src/loginViaBrowser.ts

@@ -74,10 +74,20 @@ export async function loginViaBrowser(webBaseUrl: string): Promise<string> {
       };
     }
 
-    const body = (await tokenExchangeResponse.json()) as {
-      access_token: string;
-      redirect_uri: string;
-    };
+    let body: { access_token: string; redirect_uri: string };
+    try {
+      body = (await tokenExchangeResponse.json()) as {
+        access_token: string;
+        redirect_uri: string;
+      };
+    } catch {
+      return {
+        type: "ERROR" as const,
+        error: new CliError(
+          "Authentication failed: token exchange returned an invalid response",
+        ),
+      };
+    }
 
     return {
       type: "SUCCESS" as const,