fix(hooks): improve error handling and edge cases for image analysis

- Add empty model string validation in config command

- Add --enable/--disable conflict detection

- Truncate long model names in display (>40 chars)

- Add EACCES/EPERM file permission error handler

- Add filesystem error classification (ENOSPC, EROFS)

- Log config upgrade save failures instead of silent catch
This commit is contained in:
kaitranntt
2026-02-03 22:32:49 -05:00
parent 2b0717ed53
commit cb8de2c8e8
3 changed files with 164 additions and 28 deletions
+22 -1
View File
@@ -517,6 +517,25 @@ function outputApiError(filePath, statusCode, responseBody) {
process.exit(2);
}
/**
* File permission error
*/
function outputFileAccessError(filePath, error) {
const output = formatErrorOutput(
filePath,
ERROR_CODES.UNKNOWN,
`File access denied: ${error}`,
[
'Check file permissions: ls -l ' + filePath,
isWindows ? 'Run terminal as Administrator if needed' : 'Use sudo or adjust file ownership',
'Verify file is readable by current user',
'Move file to accessible location',
]
);
console.log(JSON.stringify(output));
process.exit(2);
}
/**
* Unknown/fallback error (replaces old outputError)
*/
@@ -685,7 +704,7 @@ async function processHook() {
debugLog('File encoded', {
mediaType: mediaType,
base64Length: `${(base64Data.length / 1024).toFixed(1)} KB`,
base64Length: `${(base64Data.length / 1024).toFixed(1)}KB`,
});
// Analyze via CLIProxy
@@ -730,6 +749,8 @@ async function processHook() {
outputTimeoutError(filePath, timeout);
} else if (errMsg.includes('ECONNREFUSED') || errMsg.includes('ENOTFOUND')) {
outputCliProxyUnavailableError(filePath, `http://${CLIPROXY_HOST}:${CLIPROXY_PORT}`);
} else if (errMsg.includes('EACCES') || errMsg.includes('EPERM')) {
outputFileAccessError(filePath, errMsg);
} else {
outputUnknownError(filePath, errMsg);
}
+16 -2
View File
@@ -130,7 +130,9 @@ function showStatus(forceReload = false): void {
provider as keyof typeof DEFAULT_IMAGE_ANALYSIS_CONFIG.provider_models
] === model;
const suffix = isDefault ? dim(' (default)') : '';
console.log(` ${color(provider.padEnd(10), 'command')} ${model}${suffix}`);
// Edge case #3: Long model name truncation
const truncatedModel = model.length > 40 ? model.slice(0, 37) + '...' : model;
console.log(` ${color(provider.padEnd(10), 'command')} ${truncatedModel}${suffix}`);
}
}
console.log('');
@@ -158,6 +160,12 @@ export async function handleConfigImageAnalysisCommand(args: string[]): Promise<
return;
}
// Validate conflicting flags (Edge case #2: --enable + --disable conflict)
if (options.enable && options.disable) {
console.error(fail('Cannot use --enable and --disable together'));
process.exit(1);
}
// Apply changes if any options provided
let hasChanges = false;
const config = loadOrCreateUnifiedConfig();
@@ -185,9 +193,15 @@ export async function handleConfigImageAnalysisCommand(args: string[]): Promise<
console.error(info(`Valid providers: ${validProviders.join(', ')}`));
process.exit(1);
}
// Validate model name (Edge case #1: Empty model string validation)
const model = options.setModel.model;
if (!model || model.trim() === '') {
console.error(fail('Model name cannot be empty'));
process.exit(1);
}
imageConfig.provider_models = {
...imageConfig.provider_models,
[options.setModel.provider]: options.setModel.model,
[options.setModel.provider]: model,
};
hasChanges = true;
}
+126 -25
View File
@@ -30,6 +30,8 @@ import { isUnifiedConfigEnabled } from './feature-flags';
const CONFIG_YAML = 'config.yaml';
const CONFIG_JSON = 'config.json';
const CONFIG_LOCK = 'config.yaml.lock';
const LOCK_STALE_MS = 5000; // Lock is stale after 5 seconds
/**
* Get path to unified config.yaml
@@ -45,6 +47,71 @@ export function getConfigJsonPath(): string {
return path.join(getCcsDir(), CONFIG_JSON);
}
/**
* Get path to config lockfile
*/
function getLockFilePath(): string {
return path.join(getCcsDir(), CONFIG_LOCK);
}
/**
* Acquire lockfile for config write operations.
* Returns true if lock acquired, false if already locked by another process.
* Cleans up stale locks (older than LOCK_STALE_MS).
*/
function acquireLock(): boolean {
const lockPath = getLockFilePath();
const lockData = `${process.pid}\n${Date.now()}`;
try {
// Check if lock exists
if (fs.existsSync(lockPath)) {
const content = fs.readFileSync(lockPath, 'utf8');
const [pidStr, timestampStr] = content.trim().split('\n');
const timestamp = parseInt(timestampStr, 10);
// Check if lock is stale
if (Date.now() - timestamp > LOCK_STALE_MS) {
// Stale lock - remove and acquire
fs.unlinkSync(lockPath);
} else {
// Check if process still exists
try {
process.kill(parseInt(pidStr, 10), 0); // Signal 0 checks if process exists
// Process exists - lock is valid
return false;
} catch {
// Process doesn't exist - remove stale lock
fs.unlinkSync(lockPath);
}
}
}
// Acquire lock
fs.writeFileSync(lockPath, lockData, { mode: 0o600 });
return true;
} catch {
// Lock acquisition failed
return false;
}
}
/**
* Release lockfile after config write operation.
*/
function releaseLock(): void {
const lockPath = getLockFilePath();
try {
if (fs.existsSync(lockPath)) {
fs.unlinkSync(lockPath);
}
} catch {
// Ignore cleanup errors
}
}
/**
* Check if unified config.yaml exists
*/
@@ -105,8 +172,9 @@ export function loadUnifiedConfig(): UnifiedConfig | null {
console.error(`[i] Config upgraded to v${UNIFIED_CONFIG_VERSION}`);
}
return upgraded;
} catch {
// Ignore save errors during upgrade - config still works
} catch (saveError) {
console.error('[!] Config upgrade failed to save:', (saveError as Error).message);
// Continue using the upgraded version in-memory even if save fails
}
}
@@ -559,39 +627,72 @@ function generateYamlWithComments(config: UnifiedConfig): string {
/**
* Save unified config to YAML file.
* Uses atomic write (temp file + rename) to prevent corruption.
* Uses lockfile to prevent concurrent writes.
*/
export function saveUnifiedConfig(config: UnifiedConfig): void {
const yamlPath = getConfigYamlPath();
const dir = path.dirname(yamlPath);
// Ensure directory exists
if (!fs.existsSync(dir)) {
fs.mkdirSync(dir, { recursive: true, mode: 0o700 });
// Acquire lock (retry for up to 1 second)
const maxRetries = 10;
const retryDelayMs = 100;
let lockAcquired = false;
for (let i = 0; i < maxRetries; i++) {
if (acquireLock()) {
lockAcquired = true;
break;
}
// Wait before retry
const start = Date.now();
while (Date.now() - start < retryDelayMs) {
// Busy wait
}
}
// Ensure version is set
config.version = UNIFIED_CONFIG_VERSION;
// Generate YAML with section comments
const yamlContent = generateYamlWithComments(config);
const content = generateYamlHeader() + yamlContent;
// Atomic write: write to temp file, then rename
const tempPath = `${yamlPath}.tmp.${process.pid}`;
if (!lockAcquired) {
throw new Error('Config file is locked by another process. Wait a moment and try again.');
}
try {
fs.writeFileSync(tempPath, content, { mode: 0o600 });
fs.renameSync(tempPath, yamlPath);
} catch (err) {
// Clean up temp file on error
if (fs.existsSync(tempPath)) {
try {
fs.unlinkSync(tempPath);
} catch {
// Ignore cleanup errors
}
// Ensure directory exists
if (!fs.existsSync(dir)) {
fs.mkdirSync(dir, { recursive: true, mode: 0o700 });
}
throw err;
// Ensure version is set
config.version = UNIFIED_CONFIG_VERSION;
// Generate YAML with section comments
const yamlContent = generateYamlWithComments(config);
const content = generateYamlHeader() + yamlContent;
// Atomic write: write to temp file, then rename
const tempPath = `${yamlPath}.tmp.${process.pid}`;
try {
fs.writeFileSync(tempPath, content, { mode: 0o600 });
fs.renameSync(tempPath, yamlPath);
} catch (error) {
// Clean up temp file on error
if (fs.existsSync(tempPath)) {
try {
fs.unlinkSync(tempPath);
} catch {
// Ignore cleanup errors
}
}
// Classify filesystem errors
const err = error as NodeJS.ErrnoException;
if (err.code === 'ENOSPC') {
throw new Error('Disk full - cannot save config. Free up space and try again.');
} else if (err.code === 'EROFS' || err.code === 'EACCES') {
throw new Error(`Cannot write config - check file permissions: ${err.message}`);
}
throw error;
}
} finally {
// Always release lock
releaseLock();
}
}