Skip to content

fix: add error logging to silent catch blocks in selfPR.js#497

Closed
gugli4ifenix-design wants to merge 1 commit intoEvoMap:mainfrom
gugli4ifenix-design:fix/empty-catch-logging
Closed

fix: add error logging to silent catch blocks in selfPR.js#497
gugli4ifenix-design wants to merge 1 commit intoEvoMap:mainfrom
gugli4ifenix-design:fix/empty-catch-logging

Conversation

@gugli4ifenix-design
Copy link
Copy Markdown

Problem

Several catch (_) {} blocks in src/gep/selfPR.js silently swallow errors that can cause PR creation to fail with no diagnostic output. When selfPR fails to write state or generate a summary, users see no indication of what went wrong.

Changes

Added console.warn logging to 3 catch blocks where errors are genuinely hidden:

Location Before After
State file write (L105) catch (_) {} catch (e) { console.warn(...) }
Summary generation (L228) catch (_) {} catch (e) { console.warn(...) }
Fallback summary (L236) catch (_) {} catch (e) { console.warn(...) }

Kept 3 intentionally silent catches unchanged:

  • Optional state file read → returns default
  • Individual file read in diff loop → skips file
  • Temp directory cleanup → best-effort

Context

Found via automated code scanning (132 empty catch blocks across the codebase). This PR addresses the highest-impact file first — selfPR.js is responsible for creating PRs, so silent failures here directly block the evolution loop.

Three empty catch blocks in selfPR.js silently swallowed errors that
could cause PR creation to fail without any diagnostic output:

- State file write failure: now logs warning when state can't be saved
- Summary generation failure: now logs which method failed
- Fallback summary failure: now logs fallback error

Kept intentionally silent catches for:
- Optional state file read (returns default on missing)
- Individual file read in diff collection (skips file)
- Temp directory cleanup (best-effort)
@autogame-17
Copy link
Copy Markdown
Collaborator

Thank you @gugli4ifenix-design — the insight about silent catch (_) {} swallowing state-write failures was valid and we have merged a variant of this change internally.

Shipped in v1.70.0-beta.4 (just published to npm on the beta tag and as a GitHub Release):

  • selfPR.writeState now emits a diagnostic line to stderr when the state-file write fails. The line is gated behind DEBUG / EVOLVER_DEBUG so production logs remain noise-free, and is wrapped in its own try/catch so a broken stderr cannot crash the evolution loop.

Not adopted (intentional, with reasoning):

  • catch (_) {} at the readState site — the default-value fallback here is the correct behavior; logging on a missing state file would fire on every first-run install.
  • catch (_) {} at the computeDiffHash per-file read — skipping a missing file inside the diff loop is an expected code path, not an error worth surfacing.
  • catch (_) {} at the two getGitDiff fallbacks (git diff HEAD --git diff --) — the first catch is designed to fall through to the second form for dirty/unstaged files, so logging it would produce a false alarm on every normal working-tree diff.

The reason we cannot merge this PR directly is structural, not about the change itself: this repository is built from an internal maintainer branch and published here as a distribution artifact. External code PRs are not merged into the upstream; we instead port the intent manually, which is what happened here. You will be credited in the release notes for v1.70.0-beta.4.

Separately — thank you for also filing #499. It is being handled on its own thread.

Closing this PR as adopted-internally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants