Fix SonarCloud code smells (8 of 19)

src/cli.ts

@@ -1,5 +1,6 @@
-import { loadConfig } from './config.js';
-import { createRingApi } from './ring.js';
+import type { RingApi } from 'ring-client-api';
+import { loadConfig, type AppConfig } from './config.js';
+import { createRingApi, formatCameraList } from './ring.js';
 import { recordClip } from './recorder.js';
 import { log } from './log.js';
 
@@ -21,36 +22,9 @@ async function main(): Promise<void> {
 
   try {
     if (command === 'list') {
-      const cameras = await api.getCameras();
-      if (cameras.length === 0) {
-        log.warn('No cameras found on this account.');
-        return;
-      }
-      log.info(`Found ${cameras.length} camera(s):`);
-      for (const c of cameras) {
-        const kind = c.isDoorbot ? 'doorbell' : 'camera';
-        const battery = c.batteryLevel != null ? `${c.batteryLevel}%` : 'wired';
-        process.stdout.write(`  #${c.id}  ${c.name}  [${kind}, ${c.model}, battery: ${battery}]\n`);
-      }
-      return;
-    }
-
-    if (command === 'record') {
-      const args = parseArgs(rest);
-      const seconds = args.seconds ? Number(args.seconds) : cfg.clipLengthSeconds;
-      if (!Number.isFinite(seconds) || seconds <= 0) throw new Error('--seconds must be a positive number');
-
-      const cameras = await api.getCameras();
-      const match = pickCamera(cameras, args.camera);
-      if (!match) {
-        throw new Error(
-          `No camera matched ${args.camera ? `"${args.camera}"` : '(none specified)'}. ` +
-            `Available: ${cameras.map((c) => `${c.name} (#${c.id})`).join(', ')}`,
-        );
-      }
-      const result = await recordClip(match, cfg, seconds);
-      log.info(`Done: ${result.path}`);
-      return;
+      await runList(api);
+    } else {
+      await runRecord(api, cfg, rest);
     }
   } finally {
     api.disconnect();
@@ -59,6 +33,37 @@ async function main(): Promise<void> {
   }
 }
 
+async function runList(api: RingApi): Promise<void> {
+  const cameras = await api.getCameras();
+  if (cameras.length === 0) {
+    log.warn('No cameras found on this account.');
+    return;
+  }
+  log.info(`Found ${cameras.length} camera(s):`);
+  for (const c of cameras) {
+    const kind = c.isDoorbot ? 'doorbell' : 'camera';
+    const battery = c.batteryLevel == null ? 'wired' : `${c.batteryLevel}%`;
+    process.stdout.write(`  #${c.id}  ${c.name}  [${kind}, ${c.model}, battery: ${battery}]\n`);
+  }
+}
+
+async function runRecord(api: RingApi, cfg: AppConfig, argv: string[]): Promise<void> {
+  const args = parseArgs(argv);
+  const seconds = args.seconds ? Number(args.seconds) : cfg.clipLengthSeconds;
+  if (!Number.isFinite(seconds) || seconds <= 0) throw new Error('--seconds must be a positive number');
+
+  const cameras = await api.getCameras();
+  const match = pickCamera(cameras, args.camera);
+  if (!match) {
+    const requested = args.camera ? `"${args.camera}"` : '(none specified)';
+    throw new Error(
+      `No camera matched ${requested}. Available: ${formatCameraList(cameras)}`,
+    );
+  }
+  const result = await recordClip(match, cfg, seconds);
+  log.info(`Done: ${result.path}`);
+}
+
 function parseArgs(argv: string[]): Record<string, string> {
   const out: Record<string, string> = {};
   for (let i = 0; i < argv.length; i++) {

src/config.ts

@@ -1,7 +1,6 @@
 import { readFileSync, existsSync } from 'node:fs';
-import { resolve, isAbsolute } from 'node:path';
+import { resolve, isAbsolute, dirname } from 'node:path';
 import { fileURLToPath } from 'node:url';
-import { dirname } from 'node:path';
 
 const __dirname = dirname(fileURLToPath(import.meta.url));
 const PROJECT_ROOT = resolve(__dirname, '..');

src/log.ts

@@ -2,6 +2,8 @@
  * Minimal leveled logger. Writes a single line per event with an ISO timestamp
  * so output is greppable and friendly to `journalctl`/pm2 log capture.
  */
+import { inspect } from 'node:util';
+
 type Level = 'info' | 'warn' | 'error' | 'debug';
 
 const DEBUG = process.env.RING_DEBUG === '1' || process.env.RING_DEBUG === 'true';
@@ -11,7 +13,9 @@ function emit(level: Level, msg: string, extra?: unknown): void {
   const stamp = new Date().toISOString();
   const line = `${stamp} [${level.toUpperCase()}] ${msg}`;
   const stream = level === 'error' || level === 'warn' ? process.stderr : process.stdout;
-  if (extra !== undefined) {
+  if (extra === undefined) {
+    stream.write(`${line}\n`);
+  } else {
     let extraStr: string;
     if (typeof extra === 'string') {
       extraStr = extra;
@@ -20,13 +24,13 @@ function emit(level: Level, msg: string, extra?: unknown): void {
         extraStr = JSON.stringify(extra);
       } catch {
         // Circular refs (some Error objects, API responses) must not crash the
-        // logger — it is frequently called from catch blocks.
-        extraStr = String(extra);
+        // logger — it is frequently called from catch blocks. `inspect` handles
+        // cycles and, unlike String(), does not collapse objects to
+        // "[object Object]".
+        extraStr = inspect(extra, { depth: 3, breakLength: Infinity });
       }
     }
     stream.write(`${line} ${extraStr}\n`);
-  } else {
-    stream.write(`${line}\n`);
   }
 }
 

src/ring.ts

@@ -29,7 +29,7 @@ export function readToken(cfg: AppConfig): string {
       `Token file ${cfg.tokenPath} is unreadable or not valid JSON (${(err as Error).message}). Re-run \`npm run auth\`.`,
     );
   }
-  if (!raw || !raw.refreshToken) {
+  if (!raw?.refreshToken) {
     throw new Error(`Token file ${cfg.tokenPath} is missing "refreshToken". Re-run \`npm run auth\`.`);
   }
   return raw.refreshToken;
@@ -81,14 +81,20 @@ export function createRingApi(cfg: AppConfig): RingApi {
   return api;
 }
 
+/** Render cameras as `Name (#id), Other (#id)` for error and warning messages. */
+export function formatCameraList(cameras: readonly Pick<RingCamera, 'name' | 'id'>[]): string {
+  if (cameras.length === 0) return '(none)';
+  return cameras.map((c) => `${c.name} (#${c.id})`).join(', ');
+}
+
 /** Get cameras filtered by the config `cameras` selector. */
 export async function getSelectedCameras(api: RingApi, cfg: AppConfig): Promise<RingCamera[]> {
   const all = await api.getCameras();
   const selected = all.filter((c) => cameraSelected(cfg, c.name, c.id));
   if (selected.length === 0) {
     log.warn(
       `No cameras matched the config filter ${JSON.stringify(cfg.cameras)}. ` +
-        `Found: ${all.map((c) => `${c.name} (#${c.id})`).join(', ') || '(none)'}`,
+        `Found: ${formatCameraList(all)}`,
     );
   }
   return selected;

src/verify.ts

@@ -20,6 +20,11 @@ const execFileP = promisify(execFile);
  *   3. probe the clip for playable video (and report audio)
  *   4. (optional) wait for a real motion/ding event and confirm auto-recording
  */
+type Config = ReturnType<typeof loadConfig>;
+type Clip = Awaited<ReturnType<typeof recordClip>>;
+
+const step = (n: number, msg: string) => log.info(`[${n}] ${msg}`);
+
 async function main(): Promise<void> {
   const args = parseArgs(process.argv.slice(2));
   const seconds = numArg(args.seconds, 10, 1, '--seconds');
@@ -28,53 +33,15 @@ async function main(): Promise<void> {
   const api = createRingApi(cfg);
 
   let failures = 0;
-  const step = (n: number, msg: string) => log.info(`[${n}] ${msg}`);
 
   try {
-    // 1. AUTH + LIST ---------------------------------------------------------
-    step(1, 'Authenticating and listing cameras…');
-    const cameras = await api.getCameras();
-    if (cameras.length === 0) throw new Error('Authenticated but found 0 cameras on the account.');
-    for (const c of cameras) {
-      const battery = c.batteryLevel != null ? `${c.batteryLevel}%` : 'wired';
-      log.info(`    #${c.id}  ${c.name}  [${c.isDoorbot ? 'doorbell' : 'camera'}, ${c.model}, ${battery}]`);
-    }
-    log.info(`    ✓ Auth OK, ${cameras.length} camera(s) listed.`);
-
+    const cameras = await stepAuthAndList(api);
     const target = pickCamera(cameras, args.camera);
     if (!target) throw new Error(`No camera matched "${args.camera}".`);
 
-    // 2. CAPTURE -------------------------------------------------------------
-    step(2, `Capturing a ${seconds}s live clip from "${target.name}"…`);
-    const clip = await recordClip(target, cfg, seconds);
-    log.info(`    ✓ Wrote ${clip.path} (${(clip.bytes / 1024 / 1024).toFixed(2)} MB).`);
-
-    // 3. PROBE ---------------------------------------------------------------
-    step(3, 'Probing the clip for playable streams…');
-    const streams = await probeStreams(clip.path);
-    const hasVideo = streams.includes('video');
-    const hasAudio = streams.includes('audio');
-    if (!hasVideo) {
-      failures++;
-      log.error('    ✗ No video stream found — clip is not playable.');
-    } else {
-      log.info('    ✓ Video stream present, clip is playable.');
-    }
-    if (hasAudio) log.info('    ✓ Audio stream present.');
-    else log.warn('    ! No audio stream (Ring audio can be off in the app, or omitted for this device).');
-
-    // 4. MOTION TRIGGER (optional) ------------------------------------------
-    if (watchMotion > 0) {
-      step(4, `Waiting up to ${watchMotion}s for a real motion/ding on "${target.name}" — trigger it now (walk in front / press the doorbell)…`);
-      const fired = await waitForTriggeredRecording(target, cfg, watchMotion);
-      if (fired) log.info('    ✓ Motion/ding fired and an auto-recording completed end-to-end.');
-      else {
-        failures++;
-        log.error(`    ✗ No motion/ding event within ${watchMotion}s. Re-run with a longer --watch-motion and trigger motion.`);
-      }
-    } else {
-      step(4, 'Skipping motion test (pass --watch-motion <seconds> and trigger motion to verify it live).');
-    }
+    const clip = await stepCapture(target, cfg, seconds);
+    failures += await stepProbe(clip);
+    failures += await stepWatchMotion(target, cfg, watchMotion);
 
     log.info(failures === 0 ? '\nVERIFY: all checks passed.' : `\nVERIFY: ${failures} check(s) failed.`);
   } finally {
@@ -83,6 +50,62 @@ async function main(): Promise<void> {
   }
 }
 
+/** 1. Authenticate and list the real cameras on the account. */
+async function stepAuthAndList(api: ReturnType<typeof createRingApi>): Promise<RingCamera[]> {
+  step(1, 'Authenticating and listing cameras…');
+  const cameras = await api.getCameras();
+  if (cameras.length === 0) throw new Error('Authenticated but found 0 cameras on the account.');
+  for (const c of cameras) {
+    const battery = c.batteryLevel == null ? 'wired' : `${c.batteryLevel}%`;
+    log.info(`    #${c.id}  ${c.name}  [${c.isDoorbot ? 'doorbell' : 'camera'}, ${c.model}, ${battery}]`);
+  }
+  log.info(`    ✓ Auth OK, ${cameras.length} camera(s) listed.`);
+  return cameras;
+}
+
+/** 2. Capture a live clip to disk. */
+async function stepCapture(target: RingCamera, cfg: Config, seconds: number): Promise<Clip> {
+  step(2, `Capturing a ${seconds}s live clip from "${target.name}"…`);
+  const clip = await recordClip(target, cfg, seconds);
+  log.info(`    ✓ Wrote ${clip.path} (${(clip.bytes / 1024 / 1024).toFixed(2)} MB).`);
+  return clip;
+}
+
+/** 3. Probe the clip for a playable video stream. Returns the failure count. */
+async function stepProbe(clip: Clip): Promise<number> {
+  step(3, 'Probing the clip for playable streams…');
+  const streams = await probeStreams(clip.path);
+  let failures = 0;
+
+  if (streams.includes('video')) {
+    log.info('    ✓ Video stream present, clip is playable.');
+  } else {
+    failures++;
+    log.error('    ✗ No video stream found — clip is not playable.');
+  }
+
+  if (streams.includes('audio')) log.info('    ✓ Audio stream present.');
+  else log.warn('    ! No audio stream (Ring audio can be off in the app, or omitted for this device).');
+
+  return failures;
+}
+
+/** 4. Optionally wait for a real motion/ding and confirm auto-recording. Returns the failure count. */
+async function stepWatchMotion(target: RingCamera, cfg: Config, watchMotion: number): Promise<number> {
+  if (watchMotion <= 0) {
+    step(4, 'Skipping motion test (pass --watch-motion <seconds> and trigger motion to verify it live).');
+    return 0;
+  }
+
+  step(4, `Waiting up to ${watchMotion}s for a real motion/ding on "${target.name}" — trigger it now (walk in front / press the doorbell)…`);
+  if (await waitForTriggeredRecording(target, cfg, watchMotion)) {
+    log.info('    ✓ Motion/ding fired and an auto-recording completed end-to-end.');
+    return 0;
+  }
+  log.error(`    ✗ No motion/ding event within ${watchMotion}s. Re-run with a longer --watch-motion and trigger motion.`);
+  return 1;
+}
+
 /** Return the list of codec_types ("video"/"audio") present in a media file. */
 async function probeStreams(file: string): Promise<string[]> {
   const { stdout } = await execFileP('ffprobe', [
@@ -99,7 +122,7 @@ async function probeStreams(file: string): Promise<string[]> {
  * Subscribe to motion + ding for one camera; on the first event, record a short
  * clip and resolve true once it's on disk. Resolves false on timeout.
  */
-function waitForTriggeredRecording(camera: RingCamera, cfg: ReturnType<typeof loadConfig>, timeoutSec: number): Promise<boolean> {
+function waitForTriggeredRecording(camera: RingCamera, cfg: Config, timeoutSec: number): Promise<boolean> {
   return new Promise((resolve) => {
     const subs: Subscription[] = [];
     let done = false;

test/logic.test.mjs

@@ -1,3 +1,3 @@
 // Hermetic logic tests — no Ring account or network required.
 // Run with: npm test   (builds to dist/ first, then runs this against dist).
 import { clipFilename, slugify, pruneOldClips, ensureDir } from '../dist/files.js';
@@ -42,7 +42,7 @@
 function spy() {
   const calls = []; let resolveLast;
   const fn = (camera, c, seconds) => { calls.push({ seconds }); return new Promise((res) => { resolveLast = () => res({ camera: camera.name, path: 'x', bytes: 1, seconds }); }); };
-  return { fn, calls, finish: () => resolveLast && resolveLast() };
+  return { fn, calls, finish: () => resolveLast?.() };
 }
 
 {