domains / app-frameworks / can1357/oh-my-pi
Never silently swallow errors
Empty `catch {}` blocks hide real failures and make debugging impossible; several reviews found bugs that existed only because a failure path logged nothing. Treat every catch as a decision point, not a place to discard information:
Empty catch {} blocks hide real failures and make debugging impossible; several reviews found bugs that existed only because a failure path logged nothing. Treat every catch as a decision point, not a place to discard information:
- Never leave a catch block empty. Log the error with enough context (operation, identifiers,
error.message) so a future failure is diagnosable, unless the condition is a genuinely expected race that is explicitly documented as such. - Distinguish expected contention from real failure. Blindly adding
logger.warnto every catch produces noise for ordinary races (e.g.ENOENTbecause another process already removed a lock/file). Check for the known benign error code first and return/continue quietly; reservewarn/errorlogs for failures that actually need attention:try { await fs.rm(lockDir, { recursive: true, force: true }); } catch (error) { if (isEnoent(error)) return; // another holder already cleaned it up logger.warn("failed to release lock dir", { lockDir, error: String(error) }); } - Never let a secondary (cleanup/reporting) failure mask the primary one. When a catch block performs cleanup or calls a user-supplied error callback, wrap that secondary action in its own try/catch, log if it fails, and still surface/propagate the original error — don’t let the cleanup’s exception replace it or disappear silently.
const reportError = (error: Error): void => { if (reported) return; reported = true; try { onError?.(error); } catch (hookError) { logger.warn("error handler itself threw", { error: String(hookError) }); } }; try { await op(); } catch (error) { reportError(toError(error)); // original error is still the one reported } - Honor “never rejects” / “report once” contracts consistently. If an API promises it never rejects (e.g. to avoid an unhandled rejection crashing the host), every code path implementing that API — not just the first one patched — must funnel through the same guarded reporter, and that reporter itself must be exception-safe.
Applying these rules turns a swallowed failure into either a traceable log line or a correctly-propagated error, and prevents unrelated secondary failures from hiding the root cause.