ci: check-empty-catch gates a discarded .catch(() => {}) (void or a reason); fix the 23 sites (#213)

.github/scripts/check-empty-catch.mjs

@@ -1,8 +1,12 @@
 // A catch clause whose block holds neither a statement nor a comment swallows the
 // error with nothing on record saying why that is safe (#205). The comment is the
 // reason; a block with statements handles the error and needs none. Only the
-// statement form is checked: the `void p.catch(() => {})` promise form is a separate
-// fire-and-forget idiom, and test helpers may retry through a bare catch.
+// statement form of `catch` is checked there; the promise form is the next rule.
+// Promise form (#213): a `.catch` whose handler does nothing (`() => {}`, `() => undefined`,
+// `() => null`) and whose result is dropped (a statement or an `await` statement) needs a
+// `void` prefix or a one-line reason comment (in the handler, after the call on the same
+// line, or on the line above the statement). A handler that returns a fallback VALUE that is
+// used (`const x = await p.catch(() => undefined)`, an argument, a `return`) stays allowed.
 // The text `catch {}` inside a comment, a string, a template or a regex literal is
 // not code, so it is never reported (#213).
 // Usage: node .github/scripts/check-empty-catch.mjs   (exit 1 lists every violation)
@@ -145,6 +149,74 @@ export function scanSource(name, source) {
   return found
 }
 
+// A `.catch` handler that does nothing: an arrow whose body is empty, or just undefined / null.
+const IDLE_CATCH = /\.catch\(\s*(?:\([^()]*\)|[\w$]+)\s*=>\s*(\{\s*\}|undefined\b|null\b|void\s+0\b)\s*\)/g
+
+// Where the statement holding the call at `at` begins, walking back over balanced brackets.
+// Returns { start, argument }: `argument` when an unmatched `(` / `[` / `,` shows the call sits
+// inside another expression (an argument, an element), so its result is used.
+function statementStart(code, at) {
+  let depth = 0
+  for (let i = at - 1; i >= 0; i -= 1) {
+    const char = code[i]
+    if (char === ')' || char === ']') depth += 1
+    else if (char === '(' || char === '[') {
+      if (depth === 0) return { start: i + 1, argument: true }
+      depth -= 1
+    } else if (depth === 0) {
+      if (char === ';' || char === '{' || char === '}') return { start: i + 1, argument: false }
+      if (char === ',') return { start: i + 1, argument: true }
+      if (char === '\n') {
+        // A new statement begins here unless this line, or the one before it, continues an expression.
+        const lineStart = i + 1
+        const lineEnd = code.indexOf('\n', lineStart)
+        const line = code.slice(lineStart, lineEnd === -1 ? code.length : lineEnd).trim()
+        const before = code.slice(0, i).trimEnd().slice(-2)
+        const continues = /^[.?:+\-*/%&|=<>,]/.test(line) && !/^(?:\+\+|--)/.test(line)
+        const open = /[.?:+\-*/%&|=<>,(\[{]$/.test(before) && !/(?:\+\+|--|\)|\})$/.test(before)
+        if (!continues && !open && line !== '') return { start: lineStart, argument: false }
+      }
+    }
+  }
+  return { start: 0, argument: false }
+}
+
+// `text` with the inside of every (...), [...] and {...} removed.
+function outside(text) {
+  let depth = 0
+  let out = ''
+  for (const char of text) {
+    if ('([{'.includes(char)) { depth += 1; out += depth === 1 ? char : ''; continue }
+    if (')]}'.includes(char)) { depth -= 1; out += depth === 0 ? char : ''; continue }
+    if (depth === 0) out += char
+  }
+  return out
+}
+
+export function scanPromiseSource(name, source) {
+  const found = []
+  const { bare, commented } = maskSource(source)
+  for (const match of bare.matchAll(IDLE_CATCH)) {
+    const { start, argument } = statementStart(bare, match.index)
+    if (argument) continue
+    const head = bare.slice(start, match.index).trim()
+    // The statement's own text, without what sits inside brackets (callbacks, arguments, objects).
+    const top = outside(head)
+    // A used value: returned, thrown, declared, assigned, an arrow's expression body, a ternary, negated.
+    if (/^(?:return|throw|yield|const|let|var|export)\b|=>|(?<![=!<>])=(?![=>])|\?(?!\.)\s|^!/.test(top)) continue
+    if (/\bvoid\b/.test(top)) continue
+    // A reason: a comment in the handler, after the call on its line, or on the line above the statement.
+    const handler = commented.slice(match.index, match.index + match[0].length)
+    const eol = commented.indexOf('\n', match.index + match[0].length)
+    const rest = commented.slice(match.index + match[0].length, eol === -1 ? commented.length : eol)
+    const startLine = commented.lastIndexOf('\n', commented.indexOf(bare.slice(start).trimStart().slice(0, 1) || ' ', start))
+    const above = commented.slice(commented.lastIndexOf('\n', Math.max(0, startLine - 1)) + 1, startLine === -1 ? 0 : startLine)
+    if (/\/\*|\/\//.test(handler) || /\/\*|\/\//.test(rest) || /^\s*(?:\/\/|\/\*|\*)/.test(above)) continue
+    found.push(`${name}:${source.slice(0, match.index).split('\n').length}`)
+  }
+  return found
+}
+
 const under = (dir, names) => {
   const out = []
   for (const entry of readdirSync(dir, { withFileTypes: true })) {
@@ -171,9 +243,15 @@ export function scanTree(root) {
   return collectTargets(root).flatMap((path) => scanSource(path, readFileSync(path, 'utf8')))
 }
 
+export function scanPromiseTree(root) {
+  return collectTargets(root).flatMap((path) => scanPromiseSource(path, readFileSync(path, 'utf8')))
+}
+
 if (process.argv[1] === import.meta.filename) {
   const root = join(import.meta.dirname, '..', '..')
   const violations = scanTree(root)
+  const dropped = scanPromiseTree(root)
   for (const where of violations) console.log(`${where}: empty catch: the error is swallowed with no reason on record`)
-  if (violations.length > 0) process.exit(1)
+  for (const where of dropped) console.log(`${where}: .catch handler does nothing and its result is dropped: add \`void\` or a one-line reason comment`)
+  if (violations.length + dropped.length > 0) process.exit(1)
 }

.github/scripts/check-empty-catch.test.mjs

@@ -1,7 +1,7 @@
 import { test } from 'node:test'
 import assert from 'node:assert/strict'
 import { join } from 'node:path'
-import { maskSource, scanSource, scanTree } from './check-empty-catch.mjs'
+import { maskSource, scanPromiseSource, scanPromiseTree, scanSource, scanTree } from './check-empty-catch.mjs'
 
 test('scanSource: a catch block with no statement and no comment is a violation', () => {
   assert.deepEqual(scanSource('a.js', 'try { x() } catch {}\n'), ['a.js:1'])
@@ -17,7 +17,8 @@ test('scanSource: a comment in the block is the reason; statements handle the er
   assert.deepEqual(scanSource('a.js', 'try { x() } catch {\n  // retries on the next poll\n}\n'), [])
 })
 
-test('scanSource: the promise form, plain blocks and other identifiers are out of scope', () => {
+test('scanSource: the promise form, plain blocks and other identifiers are out of scope here', () => {
+  assert.deepEqual(scanSource('a.js', 'save().catch(() => {})\n'), [])
   assert.deepEqual(scanSource('a.js', 'void save().catch(() => {})\n'), [])
   assert.deepEqual(scanSource('a.js', 'for (;;) {}\nconst o = { catch: 1 }\n'), [])
 })
@@ -65,6 +66,57 @@ test('maskSource keeps length and line breaks, blanks literals, and keeps commen
   assert.equal(maskSource("const e = '😀'\n").bare.length, "const e = '😀'\n".length)
 })
 
+const promise = source => scanPromiseSource('a.js', source)
+
+test('promise form: a discarded `.catch(() => {})` needs `void` or a one-line reason (#213)', () => {
+  assert.deepEqual(promise('save().catch(() => {})\n'), ['a.js:1'])
+  assert.deepEqual(promise('await save().catch(() => {})\n'), ['a.js:1'])
+  assert.deepEqual(promise('await save().catch((e) => {})\n'), ['a.js:1'])
+  assert.deepEqual(promise('await save().catch(() => undefined)\n'), ['a.js:1'])
+  assert.deepEqual(promise('await save().catch(() => null)\n'), ['a.js:1'])
+  assert.deepEqual(promise('if (fresh) await unlink(path).catch(() => {})\n'), ['a.js:1'])
+  assert.deepEqual(promise('for (const id of ids) await drop(id).catch(() => {})\n'), ['a.js:1'])
+  assert.deepEqual(promise('x.then((v) => { use(v) }).catch(() => {})\n'), ['a.js:1'], 'a callback in the chain is not a used value')
+  assert.deepEqual(promise('async function f() {\n  await a()\n  await b()\n    .catch(() => {})\n}\n'), ['a.js:4'], 'a multi-line chain is one statement')
+  assert.deepEqual(promise('run(() => { save().catch(() => {}) })\n'), ['a.js:1'], 'a statement in a callback body is dropped too')
+})
+
+test('promise form: `void` or a reason comment is the answer', () => {
+  assert.deepEqual(promise('void save().catch(() => {})\n'), [])
+  assert.deepEqual(promise('if (fresh) void save().catch(() => {})\n'), [])
+  assert.deepEqual(promise('void save().then(() => b()).catch(() => {})\n'), [])
+  assert.deepEqual(promise('await unlink(path).catch(() => { /* may not exist */ })\n'), [])
+  assert.deepEqual(promise('await unlink(path).catch(() => {}) // may not exist\n'), [])
+  assert.deepEqual(promise('// may not exist\nawait unlink(path).catch(() => {})\n'), [])
+  assert.deepEqual(promise('/* may not exist */\nawait unlink(path).catch(() => {})\n'), [])
+  assert.deepEqual(promise('await unlink(path).catch(() => {})\n\n// a later comment is not its reason\n'), ['a.js:1'])
+})
+
+test('promise form: a fallback value that is used stays allowed', () => {
+  assert.deepEqual(promise('const info = await load().catch(() => undefined)\n'), [])
+  assert.deepEqual(promise('const info = await load().catch(() => null)\n'), [])
+  assert.deepEqual(promise('info = await load().catch(() => undefined)\n'), [])
+  assert.deepEqual(promise('return load().catch(() => undefined)\n'), [])
+  assert.deepEqual(promise('if ((await load().catch(() => undefined))?.ok) go()\n'), [])
+  assert.deepEqual(promise('use(await load().catch(() => undefined))\n'), [])
+  assert.deepEqual(promise('const member = known ?? await register(bot).catch(() => undefined)\n'), [])
+  assert.deepEqual(promise('const refresh = () => call(x).catch(() => {})\n'), [])
+  assert.deepEqual(promise('writing = run.catch(() => {})\n'), [])
+  assert.deepEqual(promise('list.map(item => item.done.catch(() => {}))\n'), [])
+  assert.deepEqual(promise('const v = ok ? await a().catch(() => undefined) : 1\n'), [])
+})
+
+test('promise form: handlers that do something, and literals, are not reported', () => {
+  assert.deepEqual(promise('await save().catch(() => log())\n'), [])
+  assert.deepEqual(promise('await save().catch((e) => { warn(e) })\n'), [])
+  assert.deepEqual(promise('// await save().catch(() => {})\n'), [])
+  assert.deepEqual(promise("const hint = 'save().catch(() => {})'\n"), [])
+})
+
+test('the shipped sources carry no bare discarded `.catch` (#213)', () => {
+  assert.deepEqual(scanPromiseTree(join(import.meta.dirname, '..', '..')), [])
+})
+
 test('the shipped sources carry no bare catch', () => {
   assert.deepEqual(scanTree(join(import.meta.dirname, '..', '..')), [])
 })

core/client.js

@@ -8595,7 +8595,7 @@ function useMemoryCount(botId, actions) {
     countListeners.add(listener);
     listener();
     let stop = false;
-    actions.memoryView?.(botId).then((view) => {
+    void actions.memoryView?.(botId).then((view) => {
       if (!stop) noteCount(botId, view.entries.length);
     }).catch(() => {
     });
@@ -9927,7 +9927,7 @@ function useMicLevel(enabled) {
     let audio = null;
     let frame = 0;
     let stopped = false;
-    navigator.mediaDevices.getUserMedia({ audio: true }).then((media) => {
+    void navigator.mediaDevices.getUserMedia({ audio: true }).then((media) => {
       if (stopped) {
         media.getTracks().forEach((track) => track.stop());
         return;

core/src/client/bot-browsers.js

@@ -171,7 +171,7 @@ export function createBotBrowsers({ bridge, createSource, storage = globalThis.l
   async function create(tabId) {
     const botId = owner.get(tabId)
     const reservation = await bridge.acquire(`bot:${botId}`)
-    if (disposed) { await bridge.release(reservation.lease).catch(() => {}); return undefined }
+    if (disposed) { await bridge.release(reservation.lease).catch(() => { /* best effort: the lease may already be gone */ }); return undefined }
     const element = doc.createElement('webview')
     // The shell forwards app shortcuts from a focused webview that carries these
     // attributes, as it does for its own Sidebar browser.
@@ -293,7 +293,7 @@ export function createBotBrowsers({ bridge, createSource, storage = globalThis.l
   async function closeTab(botId, tabId) {
     const state = tabs.getSnapshot()[botId]
     if (state === undefined || !state.ids.includes(tabId)) return
-    await opening.get(tabId)?.catch(() => {})
+    await opening.get(tabId)?.catch(() => { /* its opener reports a failure; this only waits for it to settle */ })
     if (shown === tabId) await show(null)
     await release(tabId)
     // The waits above can overlap a new tab or an active-tab switch; drop the
@@ -375,7 +375,7 @@ export function createBotBrowsers({ bridge, createSource, storage = globalThis.l
   async function forget(botId) {
     ensureTabs(botId)
     for (const tabId of tabs.getSnapshot()[botId]?.ids ?? []) {
-      await opening.get(tabId)?.catch(() => {})
+      await opening.get(tabId)?.catch(() => { /* its opener reports a failure; this only waits for it to settle */ })
       if (shown === tabId) await show(null)
       await release(tabId)
       owner.delete(tabId)

core/src/client/index.js

@@ -338,7 +338,7 @@ export function apply(ctx) {
 
   ctx.inject(['shortcuts'], scope => { scope.effect(() => holdShellShortcuts(scope.shortcuts), 'ds-bot: settings shortcut') })
 
-  const after = promise => promise.then(async (value) => { await pull().catch(() => {}); return value })
+  const after = promise => promise.then(async (value) => { await pull().catch(() => { /* a stale team list is fixed by the next pull; the call itself succeeded */ }); return value })
   // A Bot id, or any part of its chat, opens the Bot's current part.
   const partFor = (id) => {
     const bot = roster.getSnapshot().byId[id]
@@ -423,7 +423,7 @@ export function apply(ctx) {
     // `last` is the Bot conversation the view held before it cleared: when that part
     // was archived for a fresh one, the view goes on in the fresh one.
     landOn: async (id, last) => {
-      if (last) await pull().catch(() => {})
+      if (last) await pull().catch(() => { /* a stale team list is fixed by the next pull */ })
       const bot = last ? roster.getSnapshot().byId[last.botId] : undefined
       setPlain(false)
       const target = bot && currentPart(bot) !== last.sessionId ? currentPart(bot) : partFor(id)

core/src/client/memory-panel.js

@@ -27,7 +27,7 @@ export function useMemoryCount(botId, actions) {
     countListeners.add(listener)
     listener()
     let stop = false
-    actions.memoryView?.(botId).then((view) => { if (!stop) noteCount(botId, view.entries.length) }).catch(() => {})
+    void actions.memoryView?.(botId).then((view) => { if (!stop) noteCount(botId, view.entries.length) }).catch(() => {})
     return () => { stop = true; countListeners.delete(listener) }
   }, [botId])
   return count

core/src/client/voice.js

@@ -27,7 +27,7 @@ function useMicLevel(enabled) {
     let audio = null
     let frame = 0
     let stopped = false
-    navigator.mediaDevices.getUserMedia({ audio: true }).then((media) => {
+    void navigator.mediaDevices.getUserMedia({ audio: true }).then((media) => {
       if (stopped) { media.getTracks().forEach(track => track.stop()); return }
       stream = media
       audio = new AudioContext()

core/src/host/agents.js

@@ -129,8 +129,8 @@ export function install(rt) {
       // tools never denied.
       const createdSessions = [...agent.sessions]
       delete state.agents[agent.id]
-      for (const sessionId of createdSessions) await ctx.workspaceRegistry.archiveSession(sessionId).catch(() => {})
-      await rt.save().catch(() => {})
+      for (const sessionId of createdSessions) await ctx.workspaceRegistry.archiveSession(sessionId).catch(() => { /* rollback is best effort; the original error is what the caller sees */ })
+      await rt.save().catch(() => { /* rollback is best effort; the original error is what the caller sees */ })
       throw error
     }
     return agent

core/src/host/capabilities.js

@@ -264,7 +264,7 @@ export function install(rt) {
     return { transport: 'streamable-http', serverName: item.name, url: item.url, headers: resolveValues(item.headers, botId), failOnStartupError: true }
   }
   const disposeMount = async mount => {
-    if (mount?.fiber) await Promise.resolve().then(() => mount.fiber.dispose()).catch(() => {})
+    if (mount?.fiber) await Promise.resolve().then(() => mount.fiber.dispose()).catch(() => { /* a mount that fails to dispose must not block unloading */ })
   }
   const mountOne = async (agent, bot, agentMounts, name) => {
     const sessionId = agent.session.id

core/src/host/chat-index.js

@@ -82,7 +82,7 @@ export function install(rt) {
       // A damaged file is rebuilt from the logs once.
       for (const fresh of [false, true]) {
         try {
-          if (fresh) await unlink(indexPath).catch(() => {})
+          if (fresh) await unlink(indexPath).catch(() => { /* the index file may not exist yet */ })
           return createIndex(new DatabaseSync(indexPath))
         } catch (error) {
           if (fresh) rt.warn('ds-bot: no chat index, read_own_chat scans the logs: %s', error)

core/src/host/connectors.js

@@ -51,7 +51,7 @@ export function install(rt) {
   const unmount = async () => {
     const fiber = mount?.fiber
     mount = null
-    if (fiber !== undefined) await Promise.resolve().then(() => fiber.dispose()).catch(() => {})
+    if (fiber !== undefined) await Promise.resolve().then(() => fiber.dispose()).catch(() => { /* a fiber that fails to dispose must not block unloading */ })
   }
 
   // The MCP client wraps the server's answer (a 401, say) as the cause.

core/src/host/diagnostics.js

@@ -219,7 +219,7 @@ export function install(rt) {
   })
 
   async function report(body = {}) {
-    await rt.loadSecrets?.().catch(() => {})
+    await rt.loadSecrets?.().catch(() => { /* without stored secrets the report is still built, redacting what it can */ })
     const list = await entries()
     const client = sanitizeClient(body.client)
     const clientEntries = sanitizeClientEntries(body.clientErrors)
@@ -234,7 +234,7 @@ export function install(rt) {
   }
 
   async function logText() {
-    await rt.loadSecrets?.().catch(() => {})
+    await rt.loadSecrets?.().catch(() => { /* without stored secrets the log is still built, redacting what it can */ })
     const list = await entries()
     return list.map(entry => (entry.stack ? `${entryLine(entry)}\n${entry.stack.replace(/^/gm, '    ')}` : entryLine(entry))).join('\n')
   }

core/src/host/memory.js

@@ -649,7 +649,7 @@ export function install(rt) {
     news.delete(botId)
     return queued(async () => {
       if (rt.readOnly() !== undefined) return
-      await unlink(join(root, 'summaries', `${safeId(botId)}.json`)).catch(() => {})
+      await unlink(join(root, 'summaries', `${safeId(botId)}.json`)).catch(() => { /* the summary file may not exist */ })
       const dir = dirOf(ownScope(botId))
       if (!existsSync(dir)) return
       const name = `${safeId(botId)}-${new Date().toISOString().replace(/[:.]/g, '-')}`

core/src/host/secrets.js

@@ -123,7 +123,7 @@ export function install(rt) {
           ...(Number.isFinite(entry.lastUsedAt) ? { lastUsedAt: entry.lastUsedAt } : {}),
         })
       }
-      if (!loadedReadOnly) await chmod(path, 0o600).catch(() => {})
+      if (!loadedReadOnly) await chmod(path, 0o600).catch(() => { /* best effort: some file systems have no modes */ })
     } catch (error) {
       if (error?.code === 'ENOENT') secrets.clear()
       else rt.warn('ds-bot: unreadable secrets file %s (%s)', path, reason(error))
@@ -144,7 +144,7 @@ export function install(rt) {
         await chmod(temp, 0o600)
         await rename(temp, path)
       } catch (error) {
-        await unlink(temp).catch(() => {})
+        await unlink(temp).catch(() => { /* the temp file may already be gone */ })
         throw error
       }
     })
@@ -357,7 +357,7 @@ export function install(rt) {
     }
     if (touched) {
       changed()
-      await persist().catch(() => {})
+      await persist().catch(() => { /* the change stays in memory and the next save writes it */ })
     }
   }
 

core/src/host/store.js

@@ -451,7 +451,7 @@ export function install(rt) {
         seen = stamp()
         rt.capabilitiesStateSaved?.()
       } catch (error) {
-        await unlink(temp).catch(() => {})
+        await unlink(temp).catch(() => { /* the temp file may already be gone */ })
         throw error
       }
     })