Skip to content

fix: add debug logging to silent catch blocks in core GEP modules#498

Closed
gugli4ifenix-design wants to merge 1 commit intoEvoMap:mainfrom
gugli4ifenix-design:fix/silent-error-handling
Closed

fix: add debug logging to silent catch blocks in core GEP modules#498
gugli4ifenix-design wants to merge 1 commit intoEvoMap:mainfrom
gugli4ifenix-design:fix/silent-error-handling

Conversation

@gugli4ifenix-design
Copy link
Copy Markdown

Summary

Add debug-gated error logging to silent catch blocks in src/gep/a2a.js and src/gep/assetStore.js.

Problem

Several empty catch blocks silently swallow errors that could indicate real issues:

  • assetStore.js: Malformed JSONL lines in gene/capsule stores are silently discarded. If an asset file gets corrupted, there is no way to diagnose the issue — the loader simply skips broken entries without any trace.
  • a2a.js: computeAssetId() failures are silently ignored, meaning assets can end up without an asset_id. This breaks deduplication, lineage tracking, and any downstream logic that relies on stable identity.

Solution

All logging is gated behind process.env.DEBUG to preserve current behavior in production while enabling observability during development:

// Before
} catch (e) {}

// After  
} catch (e) { if (process.env.DEBUG) process.stderr.write("context: " + e.message + "\n"); }

Zero behavior change for existing users. Only visible when DEBUG=1 is set.

How found

Automated code quality scan flagged 132 empty catch blocks across the codebase. This PR addresses the ones in core GEP logic where silent failures could mask data integrity issues. Cleanup ops (fs.rmSync, fs.unlinkSync) and format-probing patterns were intentionally left unchanged.

…e.js

Several empty catch blocks silently swallow errors that could indicate real issues:

- assetStore.js: malformed JSONL lines in gene/capsule stores are silently
  discarded, making it impossible to diagnose corrupted asset files.
- a2a.js: computeAssetId failures are silently ignored, meaning assets
  can end up without identity — breaking deduplication and lineage tracking.

All logging is gated behind DEBUG env var to preserve current behavior
in production while enabling observability during development.

Found via automated security/quality scan.
@autogame-17
Copy link
Copy Markdown
Collaborator

Thank you @gugli4ifenix-design for the careful analysis. We evaluated this PR but are not adopting it for the following reasons.

src/gep/a2a.jscomputeAssetId() failures being silent: the intent is correct, but in the maintainer source these call sites either (a) no longer exist in this shape or (b) already propagate errors via the asset-publish result object rather than via throw / silent-catch. Adding a DEBUG log on top of the obfuscated public artifact would not land on the same call site upstream and would drift from the maintainer source on the next release. If you observe an asset in the wild that is missing asset_id, please open a dedicated issue with the asset id / hash and we can investigate it as a data bug instead.

src/gep/assetStore.js — malformed JSONL lines silently discarded: this is actually intentional for two reasons:

  1. JSONL asset stores are append-only and may contain partially-written final lines when the process is killed mid-write. Silently skipping those is the documented recovery contract — surfacing them on every load (even under DEBUG) would create a false-alarm on any healthy restart after a crash.

  2. The store already has a dedicated corruption-report path: assetStore exposes getLoadStats() which surfaces skippedMalformedLines / skippedNonObjectLines counts to callers. That is the correct place to diagnose corruption, not per-line stderr.

    If the DEBUG logging is something you want for your own debugging workflow locally, setting DEBUG=1 and wrapping the loader in a one-line printout in your own script is probably a cleaner path than changing the library behavior.

What we did adopt: PR #497 for selfPR.js — that one we merged a variant of into the internal tree and shipped in v1.70.0-beta.4. See the close comment on #497 for details.

Why we cannot merge the code PR directly even when we adopt the idea: 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 port the intent manually when we agree with it. You will be credited in release notes for the pieces we do adopt.

Thanks again for the scan work — empty catch blocks are a real correctness risk and the specific files you flagged were the right ones to look at first. Closing as evaluated-declined.

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