fix: add debug logging to empty catch blocks across infrastructure and domain#616
fix: add debug logging to empty catch blocks across infrastructure and domain#616carlos-alm merged 5 commits intomainfrom
Conversation
…d domain layers Impact: 8 functions changed, 33 affected
|
Claude finished @carlos-alm's task —— View job PR Review: fix: add debug logging to empty catch blocks across infrastructure and domainReview Progress
Root Cause AnalysisProblem Being Solved: Empty catch blocks that silently swallow errors without any logging or handling, flagged by Titan audit as Pillar II violations (Data & Type Sovereignty) Phase Assessment ResultsPhase 2: Configuration Change Detection ✅PASS - No configuration changes detected. No biome rules relaxed, no test timeouts increased, no build configuration modified. Phase 3: Test Integrity ✅PASS - No test files modified, no assertions weakened, no Phase 4: Code Quality ✅PASS - Changes are well-implemented:
Specific Changes Reviewed: src/infrastructure/config.ts (4 locations):
src/infrastructure/native.ts (2 locations):
src/domain/parser.ts (3 locations):
src/domain/graph/resolve.ts (4 locations):
All changes follow consistent patterns and maintain existing error handling behavior while adding visibility. Backlog Compliance
Critical ConcernsNone identified - This is a straightforward observability improvement addressing legitimate silent error swallowing. Final Recommendation
Minor suggestions for future PRs:
This is exactly the type of low-risk, high-value observability improvement that helps maintain a healthy codebase. |
Greptile SummaryThis PR addresses a Titan audit quality finding (Pillar II — silent error swallowing) by replacing empty Confidence Score: 5/5Safe to merge — changes are purely additive debug logging with no behavior changes. All changes are mechanical replacements of empty catch blocks with no-op-by-default No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Error thrown in catch block] --> B{Before this PR}
A --> C{After this PR}
B --> D[Silent: empty catch]
C --> E{verbose mode?}
E -->|no| F[No-op: debug gated off]
E -->|yes| G["stderr: [codegraph DEBUG] <fn>: ...message"]
G --> H[Execution continues with fallback value]
F --> H
Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
…-logging Impact: 1 functions changed, 2 affected
| /* ignore */ | ||
| } catch (e) { | ||
| debug( | ||
| `resolveWorkspaceEntry: package.json probe failed for ${pkgDir}: ${(e as Error).message}`, |
There was a problem hiding this comment.
(e as Error).message unsafe for non-Error throws
The pattern (e as Error).message is used consistently across all 4 changed files. In TypeScript strict mode, e is unknown, and if anything other than an Error instance is thrown (e.g. a plain string, null, or a non-standard exception from the WASM layer), .message will be undefined, producing unhelpful log output like readPackageExports: failed to read package.json in /path: undefined.
A safer, idiomatic alternative:
| `resolveWorkspaceEntry: package.json probe failed for ${pkgDir}: ${(e as Error).message}`, | |
| `resolveWorkspaceEntry: package.json probe failed for ${pkgDir}: ${e instanceof Error ? e.message : String(e)}`, |
The same pattern appears in all new debug() calls across these files:
src/infrastructure/config.ts:315, 349, 367src/infrastructure/native.ts:31, 98src/domain/graph/resolve.ts:70, 522, 544, 585src/domain/parser.ts:443, 462, 576, 582
For the Node.js built-ins (fs, JSON.parse, require) these always throw Error instances, so the risk is low in practice — but the WASM tree.delete() calls could theoretically throw a non-Error value.
There was a problem hiding this comment.
Addressed in follow-up PR #630 — all 14 occurrences of \ in this PR's catch blocks have been replaced with \ for safe handling of non-Error throws.
Summary
debug()logging across 4 files that failed Titan auditlogger.ts-- no new dependenciesTitan Audit Context
Changes
src/infrastructure/config.ts-- 4 empty catches replaced with debug loggingsrc/infrastructure/native.ts-- 2 empty catches replaced with debug loggingsrc/domain/parser.ts-- empty catch replaced with debug loggingsrc/domain/graph/resolve.ts-- empty catch replaced with debug loggingMetrics Impact
Test plan