diff --git a/CHANGELOG.md b/CHANGELOG.md index aa9c2e4..87c38c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **cache**: A cache backend that fails to load (e.g. `better-sqlite3` ABI mismatch after a Node major upgrade, `ERR_DLOPEN_FAILED`) is no longer misclassified as database corruption. Previously the constructor's catch-all renamed the user's healthy `cache.db` to `cache.db.corrupt-` and recreated an empty database — verified to quarantine a 2,646-entry cache that passed `integrity_check`. Native-module load failures now leave the database and its `-wal`/`-shm` sidecars untouched; genuine corruption still triggers the rename-aside recovery. `deepl translate` and `deepl write` degrade to running without a cache (single warning per process, exit code 0) instead of crashing, since the cache backend is loaded lazily behind a warn-once latch; `deepl cache …` subcommands, which cannot run cacheless, fail with an actionable error suggesting a reinstall or matching Node version. - **sync**: `deepl sync --frozen` now reports an accurate key count when drift is caused by a newly-added target locale. Previously the message read `Sync drift detected: 0 new, 0 stale keys.` because the frozen branch in `processBucket` short-circuited before promoting current-status keys missing a target-locale translation into `newKeys` — even though `--dry-run` against the same state correctly reported the backfill count. The drift exit code (10) is unchanged. The drift message now also surfaces `deletedKeys` and only mentions nonzero categories, mirroring the success-path summary format. ### Security diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 06492d2..89966e8 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -417,16 +417,20 @@ rm ~/.cache/deepl-cli/cache.db # or ~/.deepl-cli/cache.db for legacy installat deepl cache enable ``` -### "Cache database corrupted" with NODE_MODULE_VERSION mismatch +### "Translation cache backend failed to load" with NODE_MODULE_VERSION mismatch **Cause:** The `better-sqlite3` native addon was compiled for a different Node.js version than the one currently running. This happens when you switch Node.js versions (e.g., via `nvm use`, `fnm`, or a system upgrade) without rebuilding native modules. +Translation and write commands keep working with caching disabled for the run; your cache database is not modified. `deepl cache` subcommands fail until the backend loads again. + **Solution:** ```bash npm rebuild better-sqlite3 ``` +or reinstall the CLI / switch back to the Node.js version it was installed with. + --- ## Document Translation Issues diff --git a/src/cli/cache-loader.ts b/src/cli/cache-loader.ts new file mode 100644 index 0000000..db8d996 --- /dev/null +++ b/src/cli/cache-loader.ts @@ -0,0 +1,47 @@ +/** + * Lazy cache-service loader for the CLI entry point. + * The cache backend (a native module today, node:sqlite tomorrow) can + * fail to load on runtimes it wasn't built for. Commands that merely + * benefit from a cache must degrade to running without one instead of + * crashing, with a single warning per process. + */ + +import type { CacheService, CacheServiceOptions } from '../storage/cache.js'; +import { Logger } from '../utils/logger.js'; +import { errorMessage } from '../utils/error-message.js'; +import { isNativeModuleLoadError } from '../utils/native-module-error.js'; + +type CacheModule = Pick; + +export function createCacheServiceGetter( + getOptions: () => CacheServiceOptions, + importCacheModule: () => Promise = () => import('../storage/cache.js'), +): () => Promise { + let instance: CacheService | undefined; + let unavailable = false; + + return async (): Promise => { + if (instance || unavailable) { + return instance; + } + try { + const { CacheService: CacheSvc } = await importCacheModule(); + instance = CacheSvc.getInstance(getOptions()); + } catch (error) { + unavailable = true; + const detail = errorMessage(error); + if (isNativeModuleLoadError(error)) { + Logger.warn( + `Translation cache backend failed to load (${detail}). ` + + 'Your cache database has not been modified. Caching is disabled for this run. ' + + 'Reinstall the CLI, or run it with the Node.js version it was installed with, to restore caching.', + ); + } else { + Logger.warn( + `Translation cache is unavailable (${detail}). Caching is disabled for this run.`, + ); + } + } + return instance; + }; +} diff --git a/src/cli/commands/register-cache.ts b/src/cli/commands/register-cache.ts index eeb30d9..f41f18f 100644 --- a/src/cli/commands/register-cache.ts +++ b/src/cli/commands/register-cache.ts @@ -3,17 +3,29 @@ import chalk from 'chalk'; import type { ConfigService } from '../../storage/config.js'; import type { CacheService } from '../../storage/cache.js'; import { Logger } from '../../utils/logger.js'; +import { ConfigError } from '../../utils/errors.js'; export function registerCache( program: Command, deps: { getConfigService: () => ConfigService; - getCacheService: () => Promise; + getCacheService: () => Promise; handleError: (error: unknown) => never; }, ): void { const { getConfigService, getCacheService, handleError } = deps; + async function requireCacheService(): Promise { + const cacheService = await getCacheService(); + if (!cacheService) { + throw new ConfigError( + 'Cache backend is unavailable, so cache commands cannot run.', + 'Reinstall the CLI, or run it with the Node.js version it was installed with.', + ); + } + return cacheService; + } + program .command('cache') .description('Manage translation cache') @@ -32,7 +44,7 @@ Examples: .action(async (options: { format?: string }) => { try { const { CacheCommand } = await import('./cache.js'); - const cacheCommand = new CacheCommand(await getCacheService(), getConfigService()); + const cacheCommand = new CacheCommand(await requireCacheService(), getConfigService()); const stats = await cacheCommand.stats(); if (options.format === 'json') { Logger.output(JSON.stringify(stats, null, 2)); @@ -61,7 +73,7 @@ Examples: try { if (options.dryRun) { const { CacheCommand } = await import('./cache.js'); - const cacheCommand = new CacheCommand(await getCacheService(), getConfigService()); + const cacheCommand = new CacheCommand(await requireCacheService(), getConfigService()); const stats = await cacheCommand.stats(); const totalSizeMB = (stats.totalSize / (1024 * 1024)).toFixed(2); const lines = [ @@ -83,7 +95,7 @@ Examples: } const { CacheCommand } = await import('./cache.js'); - const cacheCommand = new CacheCommand(await getCacheService(), getConfigService()); + const cacheCommand = new CacheCommand(await requireCacheService(), getConfigService()); await cacheCommand.clear(); Logger.success(chalk.green('\u2713 Cache cleared successfully')); } catch (error) { @@ -106,7 +118,7 @@ Examples: } const { CacheCommand } = await import('./cache.js'); - const cacheCommand = new CacheCommand(await getCacheService(), getConfigService()); + const cacheCommand = new CacheCommand(await requireCacheService(), getConfigService()); await cacheCommand.enable(maxSizeBytes); Logger.success(chalk.green('\u2713 Cache enabled')); @@ -126,7 +138,7 @@ Examples: .action(async () => { try { const { CacheCommand } = await import('./cache.js'); - const cacheCommand = new CacheCommand(await getCacheService(), getConfigService()); + const cacheCommand = new CacheCommand(await requireCacheService(), getConfigService()); await cacheCommand.disable(); Logger.success(chalk.green('\u2713 Cache disabled')); } catch (error) { diff --git a/src/cli/commands/service-factory.ts b/src/cli/commands/service-factory.ts index 5a2d784..31210c1 100644 --- a/src/cli/commands/service-factory.ts +++ b/src/cli/commands/service-factory.ts @@ -22,7 +22,7 @@ export interface ServiceDeps { createDeepLClient: CreateDeepLClient; getApiKeyAndOptions: GetApiKeyAndOptions; getConfigService: () => ConfigService; - getCacheService: () => Promise; + getCacheService: () => Promise; handleError: (error: unknown) => never; } diff --git a/src/cli/index.ts b/src/cli/index.ts index fa2408d..c6acc40 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -11,7 +11,7 @@ import { readFileSync } from 'fs'; import { fileURLToPath } from 'url'; import { dirname, join, resolve, isAbsolute, extname } from 'path'; import { ConfigService } from '../storage/config.js'; -import type { CacheService } from '../storage/cache.js'; +import { createCacheServiceGetter } from './cache-loader.js'; import { resolvePaths } from '../utils/paths.js'; import type { DeepLClient } from '../api/deepl-client.js'; import { Logger } from '../utils/logger.js'; @@ -57,22 +57,17 @@ const paths = resolvePaths(); // Create config service - can be overridden by --config flag let configService = new ConfigService(paths.configFile); -let cacheService: CacheService | null = null; - -async function getCacheService(): Promise { - if (!cacheService) { - const { CacheService: CacheSvc } = await import('../storage/cache.js'); - const configTtl = configService.getValue('cache.ttl'); - const configMaxSize = configService.getValue('cache.maxSize'); - cacheService = CacheSvc.getInstance({ - dbPath: paths.cacheFile, - // Config TTL is in seconds, CacheService expects milliseconds - ttl: configTtl !== undefined ? configTtl * 1000 : undefined, - maxSize: configMaxSize, - }); - } - return cacheService; -} + +const getCacheService = createCacheServiceGetter(() => { + const configTtl = configService.getValue('cache.ttl'); + const configMaxSize = configService.getValue('cache.maxSize'); + return { + dbPath: paths.cacheFile, + // Config TTL is in seconds, CacheService expects milliseconds + ttl: configTtl !== undefined ? configTtl * 1000 : undefined, + maxSize: configMaxSize, + }; +}); /** * Handle error and exit with appropriate exit code diff --git a/src/services/translation.ts b/src/services/translation.ts index 802198e..76f77ea 100644 --- a/src/services/translation.ts +++ b/src/services/translation.ts @@ -6,7 +6,7 @@ import * as crypto from 'crypto'; import { DeepLClient, TranslationResult, isTranslationResult, UsageInfo, LanguageInfo } from '../api/deepl-client.js'; import { ConfigService } from '../storage/config.js'; -import { CacheService } from '../storage/cache.js'; +import type { CacheService } from '../storage/cache.js'; import { TranslationOptions, Language, TranslationMemory } from '../types/index.js'; import { Logger } from '../utils/logger.js'; import { mapWithConcurrency, MULTI_TARGET_CONCURRENCY } from '../utils/concurrency.js'; @@ -40,14 +40,16 @@ export const TRANSLATE_BATCH_SIZE = 50; // DeepL API max texts per request export class TranslationService { private client: DeepLClient; private config: ConfigService; - private cache: CacheService; + // No cache means "run cacheless" — the CLI passes undefined when the + // cache backend is unavailable (see cli/cache-loader.ts). + private cache?: CacheService; private languageCache: Map<'source' | 'target', { data: LanguageInfo[]; timestamp: number }> = new Map(); private readonly LANGUAGE_CACHE_TTL = 24 * 60 * 60 * 1000; // 24 hours constructor(client: DeepLClient, config: ConfigService, cache?: CacheService) { this.client = client; this.config = config; - this.cache = cache ?? CacheService.getInstance(); + this.cache = cache; } /** @@ -112,7 +114,7 @@ export class TranslationService { const cacheKey = this.generateCacheKey(processedText, translationOptions); if (shouldUseCache) { - const cachedResult = this.cache.get(cacheKey, isTranslationResult); + const cachedResult = this.cache?.get(cacheKey, isTranslationResult); if (cachedResult) { Logger.verbose('[verbose] Cache hit'); return { @@ -131,7 +133,7 @@ export class TranslationService { // Store in cache if (shouldUseCache) { - this.cache.set(cacheKey, result); + this.cache?.set(cacheKey, result); } return { @@ -198,7 +200,7 @@ export class TranslationService { const cacheKey = this.generateCacheKey(text, translationOptions); if (cacheEnabled) { - const cachedResult = this.cache.get(cacheKey, isTranslationResult); + const cachedResult = this.cache?.get(cacheKey, isTranslationResult); if (cachedResult) { results[i] = cachedResult; continue; @@ -281,7 +283,7 @@ export class TranslationService { // Cache the result (only once per unique text) if (cacheEnabled) { const cacheKey = this.generateCacheKey(text, translationOptions); - this.cache.set(cacheKey, result); + this.cache?.set(cacheKey, result); } } } diff --git a/src/services/write.ts b/src/services/write.ts index 05b3fc2..71135e9 100644 --- a/src/services/write.ts +++ b/src/services/write.ts @@ -6,7 +6,7 @@ import * as crypto from 'crypto'; import { DeepLClient } from '../api/deepl-client.js'; import { ConfigService } from '../storage/config.js'; -import { CacheService } from '../storage/cache.js'; +import type { CacheService } from '../storage/cache.js'; import { WriteOptions, WriteImprovement, isWriteImprovementArray } from '../types/index.js'; import { Logger } from '../utils/logger.js'; import { ValidationError, ConfigError } from '../utils/errors.js'; @@ -18,7 +18,9 @@ export interface WriteServiceOptions { export class WriteService { private client: DeepLClient; private config: ConfigService; - private cache: CacheService; + // No cache means "run cacheless" — the CLI passes undefined when the + // cache backend is unavailable (see cli/cache-loader.ts). + private cache?: CacheService; constructor(client: DeepLClient, config: ConfigService, cache?: CacheService) { if (!client) { @@ -31,7 +33,7 @@ export class WriteService { this.client = client; this.config = config; - this.cache = cache ?? CacheService.getInstance(); + this.cache = cache; } /** @@ -62,7 +64,7 @@ export class WriteService { const cacheKey = this.generateCacheKey(text, options); if (shouldUseCache) { - const cachedResult = this.cache.get(cacheKey, isWriteImprovementArray); + const cachedResult = this.cache?.get(cacheKey, isWriteImprovementArray); if (cachedResult) { Logger.verbose('[verbose] Cache hit'); return cachedResult; @@ -73,7 +75,7 @@ export class WriteService { const improvements = await this.client.improveText(text, options); if (shouldUseCache) { - this.cache.set(cacheKey, improvements); + this.cache?.set(cacheKey, improvements); } return improvements; diff --git a/src/storage/cache.ts b/src/storage/cache.ts index 9cd8197..492b515 100644 --- a/src/storage/cache.ts +++ b/src/storage/cache.ts @@ -10,6 +10,7 @@ import { resolvePaths } from '../utils/paths.js'; import { ConfigError } from '../utils/errors.js'; import { Logger } from '../utils/logger.js'; import { errorMessage } from '../utils/error-message.js'; +import { isNativeModuleLoadError } from '../utils/native-module-error.js'; export interface CacheServiceOptions { dbPath?: string; @@ -69,6 +70,12 @@ export class CacheService { try { this.openDatabase(dbPath); } catch (error) { + // A backend that cannot load (ABI mismatch after a Node upgrade, + // missing binding) is not corruption: the database on disk is + // healthy, so renaming it aside would throw away a warm cache. + if (isNativeModuleLoadError(error)) { + throw error; + } // Corrupted DB: rename-aside rather than unlink, so the user keeps // 30 days of cache history (and a forensic artifact) instead of // losing both silently. Suffix with a timestamp so repeated diff --git a/src/utils/native-module-error.ts b/src/utils/native-module-error.ts new file mode 100644 index 0000000..d13e270 --- /dev/null +++ b/src/utils/native-module-error.ts @@ -0,0 +1,24 @@ +/** + * Classifies errors thrown while loading the cache's storage backend. + * These indicate the module itself cannot load (ABI mismatch after a + * Node upgrade, missing binding, runtime without node:sqlite) — the + * database file on disk is healthy and must not be touched. + */ + +const NATIVE_LOAD_ERROR_CODES = new Set([ + 'ERR_DLOPEN_FAILED', + 'ERR_UNKNOWN_BUILTIN_MODULE', + 'MODULE_NOT_FOUND', + 'ERR_MODULE_NOT_FOUND', +]); + +export function isNativeModuleLoadError(error: unknown): boolean { + if (!(error instanceof Error)) { + return false; + } + const code = (error as NodeJS.ErrnoException).code; + if (code && NATIVE_LOAD_ERROR_CODES.has(code)) { + return true; + } + return error.message.includes('NODE_MODULE_VERSION'); +} diff --git a/tests/e2e/cli-cache-degraded.e2e.test.ts b/tests/e2e/cli-cache-degraded.e2e.test.ts new file mode 100644 index 0000000..6d1b75a --- /dev/null +++ b/tests/e2e/cli-cache-degraded.e2e.test.ts @@ -0,0 +1,158 @@ +/** + * E2E Tests for degraded cache backend + * Simulates a native-module load failure (e.g. ABI mismatch after a + * `brew upgrade node`) by hijacking module resolution for better-sqlite3 + * in the CLI subprocess. The CLI must keep translating/writing with the + * cache disabled, warn exactly once, and never touch the cache database. + */ + +import { spawn, ChildProcess } from 'child_process'; +import * as fs from 'fs'; +import * as path from 'path'; +import { createTestConfigDir, createTestDir, makeNodeRunCLI } from '../helpers'; + +const ABI_ERROR_SNIPPET = + 'was compiled against a different Node.js version using NODE_MODULE_VERSION 137'; + +const BREAK_SQLITE_PRELOAD = `'use strict'; +const { registerHooks } = require('node:module'); +registerHooks({ + resolve(specifier, context, nextResolve) { + if (specifier === 'better-sqlite3') { + const err = new Error( + "The module '/fake/better_sqlite3.node' ${ABI_ERROR_SNIPPET}. " + + 'This version of Node.js requires NODE_MODULE_VERSION 127.', + ); + err.code = 'ERR_DLOPEN_FAILED'; + throw err; + } + return nextResolve(specifier, context); + }, +}); +`; + +describe('CLI with unavailable cache backend E2E', () => { + const testConfig = createTestConfigDir('e2e-cache-degraded'); + const testFiles = createTestDir('e2e-cache-degraded-files'); + let mockServerProcess: ChildProcess; + let baseUrl: string; + let preloadPath: string; + let cacheDbPath: string; + + let runCLIAll: (command: string, options?: { env?: Record }) => string; + let runCLIExpectError: ( + command: string, + options?: { env?: Record }, + ) => { status: number; output: string }; + + function brokenEnv(): { env: Record } { + return { env: { NODE_OPTIONS: `--require ${preloadPath}` } }; + } + + function startMockServer(): Promise { + return new Promise((resolve, reject) => { + const serverScript = path.join(__dirname, 'mock-deepl-server.cjs'); + const child = spawn('node', [serverScript], { + stdio: ['pipe', 'pipe', 'pipe'], + env: { ...process.env }, + }); + + mockServerProcess = child; + let output = ''; + + child.stdout.on('data', (data: Buffer) => { + output += data.toString(); + const match = output.match(/PORT=(\d+)/); + if (match) { + resolve(parseInt(match[1]!, 10)); + } + }); + + child.on('error', reject); + child.on('exit', (code) => { + if (code !== null && code !== 0) { + reject(new Error(`Mock server exited with code ${code}`)); + } + }); + + setTimeout(() => reject(new Error('Mock server did not start within 15s')), 15000).unref(); + }); + } + + beforeAll(async () => { + const helpers = makeNodeRunCLI(testConfig.path, { noColor: true, timeout: 15000 }); + runCLIAll = helpers.runCLIAll; + runCLIExpectError = helpers.runCLIExpectError; + + preloadPath = path.join(testFiles.path, 'break-better-sqlite3.cjs'); + fs.writeFileSync(preloadPath, BREAK_SQLITE_PRELOAD); + + const mockPort = await startMockServer(); + baseUrl = `http://127.0.0.1:${mockPort}`; + + const config = { + auth: { apiKey: 'mock-api-key-for-testing:fx' }, + api: { baseUrl, usePro: false }, + defaults: { targetLangs: [], formality: 'default', preserveFormatting: true }, + cache: { enabled: true, maxSize: 1048576, ttl: 2592000 }, + output: { format: 'text', verbose: false, color: false }, + }; + fs.writeFileSync(path.join(testConfig.path, 'config.json'), JSON.stringify(config, null, 2)); + + // Populate a healthy cache database with a working backend first. + cacheDbPath = path.join(testConfig.path, 'cache.db'); + runCLIAll('translate "Hello" --to es'); + if (!fs.existsSync(cacheDbPath)) { + throw new Error(`Seeding run did not create ${cacheDbPath}`); + } + }, 30000); + + afterAll(() => { + if (mockServerProcess) { + mockServerProcess.kill('SIGTERM'); + } + testConfig.cleanup(); + testFiles.cleanup(); + }); + + function listCorruptFiles(): string[] { + return fs.readdirSync(testConfig.path).filter((f) => f.includes('.corrupt-')); + } + + it('translate succeeds with exit code 0 and exactly one warning', () => { + const before = fs.readFileSync(cacheDbPath); + + const output = runCLIAll('translate "Hello world" --to es', brokenEnv()); + + expect(output).toContain('Hola mundo'); + const warnings = output.split('Caching is disabled for this run').length - 1; + expect(warnings).toBe(1); + + // The healthy database was not quarantined or modified. + expect(listCorruptFiles()).toEqual([]); + expect(fs.readFileSync(cacheDbPath).equals(before)).toBe(true); + }); + + it('write succeeds with exit code 0 and a warning', () => { + const output = runCLIAll('write "helo wrld" --lang en-US', brokenEnv()); + + expect(output).toContain('Caching is disabled for this run'); + expect(listCorruptFiles()).toEqual([]); + }); + + it('cache stats fails with a clear, actionable error', () => { + const result = runCLIExpectError('cache stats', brokenEnv()); + + expect(result.status).not.toBe(0); + expect(result.output.toLowerCase()).toContain('unavailable'); + expect(listCorruptFiles()).toEqual([]); + }); + + it('still uses the cache normally when the backend loads fine', () => { + const output = runCLIAll('translate "Hello" --to es'); + + expect(output).toContain('Hola'); + expect(output).not.toContain('Caching is disabled for this run'); + expect(listCorruptFiles()).toEqual([]); + }); +}); diff --git a/tests/unit/cache-loader.test.ts b/tests/unit/cache-loader.test.ts new file mode 100644 index 0000000..9c15a57 --- /dev/null +++ b/tests/unit/cache-loader.test.ts @@ -0,0 +1,123 @@ +/** + * Tests for the lazy cache-service getter used by the CLI entry point. + * A cache backend that cannot load must degrade to "no cache" with a + * single warning, not crash the command or retry on every call. + */ + +jest.mock('../../src/utils/logger', () => ({ + Logger: { + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + success: jest.fn(), + output: jest.fn(), + }, +})); + +import { createCacheServiceGetter } from '../../src/cli/cache-loader'; +import { Logger } from '../../src/utils/logger'; +import type { CacheService } from '../../src/storage/cache'; + +function makeError(message: string, code?: string): Error { + const error = new Error(message); + if (code) { + (error as NodeJS.ErrnoException).code = code; + } + return error; +} + +describe('createCacheServiceGetter', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('when the cache module loads', () => { + it('returns the singleton constructed with the resolved options', async () => { + const fakeInstance = { get: jest.fn() } as unknown as CacheService; + const getInstance = jest.fn().mockReturnValue(fakeInstance); + const importer = jest.fn().mockResolvedValue({ + CacheService: { getInstance }, + }); + const options = { dbPath: '/tmp/cache.db', ttl: 1000, maxSize: 42 }; + + const getCacheService = createCacheServiceGetter(() => options, importer); + + await expect(getCacheService()).resolves.toBe(fakeInstance); + expect(getInstance).toHaveBeenCalledWith(options); + expect(Logger.warn).not.toHaveBeenCalled(); + }); + + it('reuses the instance instead of re-importing', async () => { + const fakeInstance = {} as CacheService; + const importer = jest.fn().mockResolvedValue({ + CacheService: { getInstance: jest.fn().mockReturnValue(fakeInstance) }, + }); + + const getCacheService = createCacheServiceGetter(() => ({}), importer); + + await getCacheService(); + await expect(getCacheService()).resolves.toBe(fakeInstance); + expect(importer).toHaveBeenCalledTimes(1); + }); + }); + + describe('when the dynamic import rejects (e.g. ABI mismatch)', () => { + const abiError = makeError( + "The module '/x/better_sqlite3.node' was compiled against a different Node.js version using NODE_MODULE_VERSION 137.", + 'ERR_DLOPEN_FAILED', + ); + + it('returns undefined instead of throwing', async () => { + const importer = jest.fn().mockRejectedValue(abiError); + const getCacheService = createCacheServiceGetter(() => ({}), importer); + + await expect(getCacheService()).resolves.toBeUndefined(); + }); + + it('warns exactly once, tells the user what to do, and says caching is off', async () => { + const importer = jest.fn().mockRejectedValue(abiError); + const getCacheService = createCacheServiceGetter(() => ({}), importer); + + await getCacheService(); + await getCacheService(); + await getCacheService(); + + expect(Logger.warn).toHaveBeenCalledTimes(1); + const warning = (Logger.warn as jest.Mock).mock.calls[0]![0] as string; + expect(warning).toContain('Caching is disabled for this run'); + expect(warning).toContain('has not been modified'); + expect(warning).toMatch(/reinstall|Node\.js version/i); + }); + + it('latches: does not retry the import on subsequent calls', async () => { + const importer = jest.fn().mockRejectedValue(abiError); + const getCacheService = createCacheServiceGetter(() => ({}), importer); + + await getCacheService(); + await getCacheService(); + + expect(importer).toHaveBeenCalledTimes(1); + }); + }); + + describe('when construction fails after a successful import', () => { + it('degrades to undefined with a single warning', async () => { + const importer = jest.fn().mockResolvedValue({ + CacheService: { + getInstance: jest.fn(() => { + throw makeError('disk I/O error'); + }), + }, + }); + const getCacheService = createCacheServiceGetter(() => ({}), importer); + + await expect(getCacheService()).resolves.toBeUndefined(); + await expect(getCacheService()).resolves.toBeUndefined(); + + expect(Logger.warn).toHaveBeenCalledTimes(1); + const warning = (Logger.warn as jest.Mock).mock.calls[0]![0] as string; + expect(warning).toContain('disk I/O error'); + expect(warning).toContain('Caching is disabled for this run'); + }); + }); +}); diff --git a/tests/unit/cache-native-failure.test.ts b/tests/unit/cache-native-failure.test.ts new file mode 100644 index 0000000..7d08541 --- /dev/null +++ b/tests/unit/cache-native-failure.test.ts @@ -0,0 +1,125 @@ +/** + * Tests for CacheService native-module load failure classification. + * A native binding that fails to load (e.g. ABI mismatch after a Node + * upgrade) must NOT be treated as database corruption: the DB file and + * its -wal/-shm sidecars must be left untouched. Genuine open failures + * must still trigger the rename-aside recovery path. + */ + +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; + +jest.mock('../../src/utils/logger', () => ({ + Logger: { + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + success: jest.fn(), + output: jest.fn(), + }, +})); + +let mockConstructorError: Error | null = null; + +// A plain function, not jest.fn(): the project's resetMocks setting would +// strip a jest.fn implementation between tests. +jest.mock('better-sqlite3', () => + function MockDatabase(): never { + throw mockConstructorError ?? new Error('mockConstructorError not set'); + }, +); + +import { CacheService } from '../../src/storage/cache'; +import { Logger } from '../../src/utils/logger'; + +function makeError(message: string, code?: string): Error { + const error = new Error(message); + if (code) { + (error as NodeJS.ErrnoException).code = code; + } + return error; +} + +describe('CacheService native-module load failure', () => { + let testCacheDir: string; + let testCachePath: string; + const dbContent = 'pretend this is a healthy 1MB sqlite database'; + const walContent = 'wal sidecar'; + const shmContent = 'shm sidecar'; + + beforeEach(() => { + jest.clearAllMocks(); + testCacheDir = fs.mkdtempSync(path.join(os.tmpdir(), 'deepl-cli-native-fail-')); + testCachePath = path.join(testCacheDir, 'cache.db'); + fs.writeFileSync(testCachePath, dbContent); + fs.writeFileSync(testCachePath + '-wal', walContent); + fs.writeFileSync(testCachePath + '-shm', shmContent); + }); + + afterEach(() => { + fs.rmSync(testCacheDir, { recursive: true, force: true }); + }); + + function listCorruptBackups(): string[] { + return fs.readdirSync(testCacheDir).filter((f) => f.includes('.corrupt-')); + } + + describe.each([ + [ + 'ERR_DLOPEN_FAILED (ABI mismatch)', + makeError( + "The module '/x/better_sqlite3.node' was compiled against a different Node.js version using NODE_MODULE_VERSION 137. This version of Node.js requires NODE_MODULE_VERSION 127.", + 'ERR_DLOPEN_FAILED', + ), + ], + [ + 'ERR_UNKNOWN_BUILTIN_MODULE (missing node:sqlite)', + makeError("No such built-in module: node:sqlite", 'ERR_UNKNOWN_BUILTIN_MODULE'), + ], + [ + 'NODE_MODULE_VERSION message without a code', + makeError('was compiled against a different Node.js version using NODE_MODULE_VERSION 137'), + ], + [ + 'MODULE_NOT_FOUND (binding removed)', + makeError("Cannot find module 'better-sqlite3'", 'MODULE_NOT_FOUND'), + ], + ])('%s', (_label, error) => { + it('rethrows without renaming the database or its sidecars', () => { + mockConstructorError = error; + + expect(() => new CacheService({ dbPath: testCachePath })).toThrow(error.message); + + expect(fs.readFileSync(testCachePath, 'utf-8')).toBe(dbContent); + expect(fs.readFileSync(testCachePath + '-wal', 'utf-8')).toBe(walContent); + expect(fs.readFileSync(testCachePath + '-shm', 'utf-8')).toBe(shmContent); + expect(listCorruptBackups()).toEqual([]); + }); + + it('does not log the corruption warning', () => { + mockConstructorError = error; + + expect(() => new CacheService({ dbPath: testCachePath })).toThrow(); + + expect(Logger.warn).not.toHaveBeenCalledWith( + expect.stringContaining('corrupted'), + ); + }); + }); + + describe('genuine open failure (not a load failure)', () => { + it('still renames the database aside', () => { + mockConstructorError = makeError('file is not a database', 'SQLITE_NOTADB'); + + // Recreation also fails (the mock always throws), so the constructor + // rethrows — but the rename-aside must have happened first. + expect(() => new CacheService({ dbPath: testCachePath })).toThrow('file is not a database'); + + expect(fs.existsSync(testCachePath)).toBe(false); + const backups = listCorruptBackups(); + expect(backups.some((f) => f.startsWith('cache.db.corrupt-'))).toBe(true); + expect(Logger.warn).toHaveBeenCalledWith(expect.stringContaining('Cache database corrupted')); + }); + }); +}); diff --git a/tests/unit/register-cache-registration.test.ts b/tests/unit/register-cache-registration.test.ts index 4b2324f..4535a6f 100644 --- a/tests/unit/register-cache-registration.test.ts +++ b/tests/unit/register-cache-registration.test.ts @@ -139,6 +139,23 @@ describe('registerCache', () => { }); }); + describe('unavailable cache backend', () => { + it.each([ + ['stats', ['cache', 'stats']], + ['clear', ['cache', 'clear', '--yes']], + ['enable', ['cache', 'enable']], + ['disable', ['cache', 'disable']], + ])('cache %s reports a clear error when getCacheService resolves undefined', async (_name, args) => { + getCacheService.mockResolvedValue(undefined); + await program.parseAsync(['node', 'test', ...args]); + expect(handleError).toHaveBeenCalledWith( + expect.objectContaining({ message: expect.stringMatching(/cache.*unavailable/i) }), + ); + const { CacheCommand } = require('../../src/cli/commands/cache'); + expect(CacheCommand as jest.Mock).not.toHaveBeenCalled(); + }); + }); + describe('cache clear', () => { it('should clear with --yes flag', async () => { mockCacheCommandInstance.clear.mockResolvedValue(undefined); diff --git a/tests/unit/translation-service.test.ts b/tests/unit/translation-service.test.ts index 16127cf..f40b2b8 100644 --- a/tests/unit/translation-service.test.ts +++ b/tests/unit/translation-service.test.ts @@ -1453,4 +1453,36 @@ describe('TranslationService', () => { }); }); + describe('without a cache backend', () => { + let cachelessService: TranslationService; + + beforeEach(() => { + cachelessService = new TranslationService(mockDeepLClient, mockConfigService); + }); + + it('should translate even though cache.enabled is true in config', async () => { + mockDeepLClient.translate.mockResolvedValue({ + text: 'Hola', + detectedSourceLang: 'en', + }); + + const result = await cachelessService.translate('Hello', { targetLang: 'es' }); + + expect(result.text).toBe('Hola'); + expect(mockDeepLClient.translate).toHaveBeenCalledTimes(1); + }); + + it('should batch translate without a cache', async () => { + mockDeepLClient.translateBatch.mockResolvedValue([ + { text: 'Hola' }, + { text: 'Mundo' }, + ]); + + const results = await cachelessService.translateBatch(['Hello', 'World'], { targetLang: 'es' }); + + expect(results).toHaveLength(2); + expect(results[0]?.text).toBe('Hola'); + }); + }); + }); diff --git a/tests/unit/write-service.test.ts b/tests/unit/write-service.test.ts index 089d44f..f086dd4 100644 --- a/tests/unit/write-service.test.ts +++ b/tests/unit/write-service.test.ts @@ -547,4 +547,19 @@ describe('WriteService', () => { } }); }); + + describe('without a cache backend', () => { + it('should improve text even though cache.enabled is true in config', async () => { + const cachelessService = new WriteService(mockClient, mockConfigService); + const mockImprovements: WriteImprovement[] = [ + { text: 'Improved.', targetLanguage: 'en-US' }, + ]; + mockClient.improveText.mockResolvedValue(mockImprovements); + + const result = await cachelessService.improve('Test', { targetLang: 'en-US' }); + + expect(result).toHaveLength(1); + expect(mockClient.improveText).toHaveBeenCalledTimes(1); + }); + }); });