Skip to content

Commit 649687d

Browse files
seanthegeekclaude
andcommitted
Add exception-handling and library-signature lessons to AGENTS.md
Two lessons from the migrate refill-enrichment silent-zero-updates bug: - Broad except Exception around a single external call masks programming errors (bad kwargs, ImportError) as silent no-ops. Only catch what you have an actual recovery path for. - Verify external library signatures against the installed version, not from memory or similar-looking code. The parallel=False kwarg was a hallucination that the per-IP except silently ate for an entire release. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 75c051a commit 649687d

1 file changed

Lines changed: 2 additions & 0 deletions

File tree

AGENTS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,8 @@ A domain can only belong to one client at a time. Offboarded domains can be re-a
130130
- **Email cleanup** — handled by the dmarc-msp container's background loop (runs daily), configured via `retention.email_days` in the YAML config. No separate cron container.
131131
- **OpenSearch string fields — always query `.keyword` for aggregations and terms filters.** parsedmarc's dynamic mapping stores every string field as `text` with a `.keyword` subfield. Composite aggregation sources, `terms` filters, and sorts on the bare text field fail with `search_phase_execution_exception: Text fields are not optimised for operations that require per-document field data…`. Use `<field>.keyword`. `exists` queries work on the base field and don't need the subfield.
132132
- **OSD index-pattern field cache — refresh after any field-schema change.** Dashboards index-pattern saved objects cache `attributes.fields` at import time and never auto-refresh. Any operation that adds or renames fields on existing indices (parsedmarc upgrades, `migrate rename-asn-fields`, custom mapping changes) must call `DashboardService.refresh_index_pattern_fields(tenant_name)` per active tenant, or Discover shows "no cached mapping" warnings on the new/renamed fields. `migrate rename-asn-fields` and `migrate all` do this automatically via the `_refresh_tenant_index_patterns` helper in `cli/migrate.py`.
133+
- **Don't use broad `except Exception` to make external calls "safe".** If the only thing a block does is call one external function, `except Exception` inside it almost certainly masks programming errors (bad kwargs, `ImportError`, `AttributeError`) as silent no-ops. `migrate refill-enrichment` silently produced zero updates for an entire release because a per-IP `try/except Exception` in the parsedmarc lookup script turned `TypeError: unexpected keyword argument 'parallel'` into `{ip: None}`. Only catch exceptions you have an expected recovery path for (e.g., `json.JSONDecodeError` when parsing untrusted input). Let the rest propagate so the traceback surfaces on the first failure.
134+
- **Verify external library signatures before calling.** The `parallel=False` bug was a hallucinated kwarg — no such parameter exists on `parsedmarc.utils.get_ip_address_info`. When writing code that calls into parsedmarc or another library, confirm the function signature against the installed version (`python3 -c "import inspect, parsedmarc.utils; print(inspect.signature(parsedmarc.utils.get_ip_address_info))"`) rather than relying on memory or similar-looking code elsewhere.
133135

134136
## Concurrency Model
135137

0 commit comments

Comments
 (0)