diff --git a/lib/errors.ts b/lib/errors.ts index a44081cd..d1b95e03 100644 --- a/lib/errors.ts +++ b/lib/errors.ts @@ -164,3 +164,24 @@ export class CodexRateLimitError extends CodexError { this.accountId = options?.accountId; } } + +/** + * Storage-specific error with a filesystem code, target path, and user-facing hint. + */ +export class StorageError extends CodexError { + override readonly name = "StorageError"; + readonly path: string; + readonly hint: string; + + constructor( + message: string, + code: string, + path: string, + hint: string, + cause?: Error, + ) { + super(message, { code, cause }); + this.path = path; + this.hint = hint; + } +} diff --git a/lib/storage.ts b/lib/storage.ts index 0509925d..a529807f 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -3,6 +3,7 @@ import { createHash } from "node:crypto"; import { existsSync, promises as fs } from "node:fs"; import { basename, dirname, isAbsolute, join, relative } from "node:path"; import { ACCOUNT_LIMITS } from "./constants.js"; +import { StorageError } from "./errors.js"; import { createLogger } from "./logger.js"; import { exportNamedBackupFile, @@ -11,6 +12,12 @@ import { } from "./named-backup-export.js"; import { MODEL_FAMILIES, type ModelFamily } from "./prompts/codex.js"; import { AnyAccountStorageSchema, getValidationErrors } from "./schemas.js"; + +import { toStorageError } from "./storage/error-hints.js"; + +export { StorageError } from "./errors.js"; +export { formatStorageErrorHint } from "./storage/error-hints.js"; + import { type AccountMetadataV1, type AccountMetadataV3, @@ -122,7 +129,9 @@ export interface NamedBackupSummary { mtimeMs: number; } -async function collectNamedBackups(storagePath: string): Promise { +async function collectNamedBackups( + storagePath: string, +): Promise { const backupRoot = getNamedBackupRoot(storagePath); let entries: Array<{ isFile(): boolean; name: string }>; try { @@ -147,12 +156,15 @@ async function collectNamedBackups(storagePath: string): Promise null); if (statsAfter && statsAfter.mtimeMs !== statsBefore.mtimeMs) { - log.debug("backup file changed between stat and load, mtime may be stale", { - candidatePath, - fileName: entry.name, - beforeMtimeMs: statsBefore.mtimeMs, - afterMtimeMs: statsAfter.mtimeMs, - }); + log.debug( + "backup file changed between stat and load, mtime may be stale", + { + candidatePath, + fileName: entry.name, + beforeMtimeMs: statsBefore.mtimeMs, + afterMtimeMs: statsAfter.mtimeMs, + }, + ); } candidates.push({ path: candidatePath, @@ -161,17 +173,20 @@ async function collectNamedBackups(storagePath: string): Promise = Promise.resolve(); const transactionSnapshotContext = new AsyncLocalStorage<{ snapshot: AccountStorageV3 | null; @@ -949,7 +914,9 @@ export async function restoreAccountsFromBackup( !relativePath.startsWith("..") && !isAbsolute(relativePath); if (!isInsideBackupRoot) { - throw new Error(`Backup path must stay inside ${resolvedBackupRoot}: ${path}`); + throw new Error( + `Backup path must stay inside ${resolvedBackupRoot}: ${path}`, + ); } const { normalized } = await (async () => { @@ -958,15 +925,15 @@ export async function restoreAccountsFromBackup( } catch (error) { const code = (error as NodeJS.ErrnoException).code; if (code === "ENOENT") { - throw new Error( - `Backup file no longer exists: ${path}`, - ); + throw new Error(`Backup file no longer exists: ${path}`); } throw error; } })(); if (!normalized || normalized.accounts.length === 0) { - throw new Error(`Backup does not contain any accounts: ${resolvedBackupPath}`); + throw new Error( + `Backup does not contain any accounts: ${resolvedBackupPath}`, + ); } if (options?.persist !== false) { await saveAccounts(normalized); @@ -1330,7 +1297,10 @@ function findCompatibleRefreshTokenMatchIndex( matchingAccount = account; continue; } - const newest: T = selectNewestAccount(matchingAccount ?? undefined, account); + const newest: T = selectNewestAccount( + matchingAccount ?? undefined, + account, + ); if (newest === account) { matchingIndex = i; matchingAccount = account; @@ -2034,23 +2004,20 @@ async function saveAccountsUnlocked(storage: AccountStorageV3): Promise { } const err = error as NodeJS.ErrnoException; - const code = err?.code || "UNKNOWN"; - const hint = formatStorageErrorHint(error, path); + const storageError = toStorageError( + `Failed to save accounts: ${err?.message || "Unknown error"}`, + error, + path, + ); log.error("Failed to save accounts", { path, - code, + code: storageError.code, message: err?.message, - hint, + hint: storageError.hint, }); - throw new StorageError( - `Failed to save accounts: ${err?.message || "Unknown error"}`, - code, - path, - hint, - err instanceof Error ? err : undefined, - ); + throw storageError; } } @@ -2114,7 +2081,9 @@ export async function withAccountAndFlaggedStorageTransaction( accountStorage: AccountStorageV3, flaggedStorage: FlaggedAccountStorageV1, ): Promise => { - const previousAccounts = cloneAccountStorageForPersistence(state.snapshot); + const previousAccounts = cloneAccountStorageForPersistence( + state.snapshot, + ); const nextAccounts = cloneAccountStorageForPersistence(accountStorage); await saveAccountsUnlocked(nextAccounts); try { diff --git a/lib/storage/error-hints.ts b/lib/storage/error-hints.ts new file mode 100644 index 00000000..3d406adf --- /dev/null +++ b/lib/storage/error-hints.ts @@ -0,0 +1,49 @@ +import { StorageError } from "../errors.js"; + +function extractErrorCode(error: unknown): string { + const err = error as NodeJS.ErrnoException; + return err?.code || "UNKNOWN"; +} + +/** + * Format a user-facing hint for storage persistence failures based on errno code. + */ +export function formatStorageErrorHint(error: unknown, path: string): string { + const code = extractErrorCode(error); + const isWindows = process.platform === "win32"; + + switch (code) { + case "EACCES": + case "EPERM": + return isWindows + ? `Permission denied writing to ${path}. Check antivirus exclusions for this folder. Ensure you have write permissions.` + : `Permission denied writing to ${path}. Check folder permissions. Try: chmod 755 ~/.codex`; + case "EBUSY": + return `File is locked at ${path}. The file may be open in another program. Close any editors or processes accessing it.`; + case "ENOENT": + return `Path does not exist: ${path}. Create the parent folder and try again.`; + case "ENOSPC": + return `Disk is full. Free up space and try again. Path: ${path}`; + default: + return isWindows + ? `Failed to write to ${path}. Check folder permissions and ensure path contains no special characters.` + : `Failed to write to ${path}. Check folder permissions and disk space.`; + } +} + +/** + * Wrap an arbitrary storage failure in a StorageError with a derived hint. + */ +export function toStorageError( + message: string, + error: unknown, + path: string, +): StorageError { + return new StorageError( + message, + extractErrorCode(error), + path, + formatStorageErrorHint(error, path), + error instanceof Error ? error : undefined, + ); +} diff --git a/test/storage.test.ts b/test/storage.test.ts index e69feac2..7ea802b7 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -27,6 +27,7 @@ import { withAccountAndFlaggedStorageTransaction, withAccountStorageTransaction, } from "../lib/storage.js"; +import { toStorageError } from "../lib/storage/error-hints.js"; // Mocking the behavior we're about to implement for TDD // Since the functions aren't in lib/storage.ts yet, we'll need to mock them or @@ -78,6 +79,21 @@ describe("storage", () => { expect(error.hint).toContain("Permission denied writing"); expect(error.path).toBe("/tmp/openai-codex-accounts.json"); }); + + it("wraps unknown failures with a StorageError", () => { + const cause = Object.assign(new Error("file locked"), { code: "EBUSY" }); + const error = toStorageError( + "failed to persist accounts", + cause, + "/tmp/openai-codex-accounts.json", + ); + + expect(error).toBeInstanceOf(StorageError); + expect(error.code).toBe("EBUSY"); + expect(error.path).toBe("/tmp/openai-codex-accounts.json"); + expect(error.hint).toContain("File is locked"); + expect(error.cause).toBe(cause); + }); }); describe("deduplication", () => { it("preserves activeIndexByFamily when shared accountId entries remain distinct without email", () => { @@ -1223,13 +1239,6 @@ describe("storage", () => { expect(hint).toContain("Disk is full"); }); - it("should return empty file hint for EEMPTY", () => { - const err = { code: "EEMPTY" } as NodeJS.ErrnoException; - const hint = formatStorageErrorHint(err, testPath); - - expect(hint).toContain("empty"); - }); - it("should return generic hint for unknown error codes", () => { const err = { code: "UNKNOWN_CODE" } as NodeJS.ErrnoException; const hint = formatStorageErrorHint(err, testPath);