domains / / the-pr-agent/pr-agent
Exception Handling Policy
Adopt an exception-handling standard that distinguishes **critical control-flow** from **non-critical side effects**, while enforcing safe, observable exception patterns.
Adopt an exception-handling standard that distinguishes critical control-flow from non-critical side effects, while enforcing safe, observable exception patterns.
1) Use framework-correct error propagation
- In FastAPI/Starlette handlers, use
raise HTTPException(...)(neverreturn HTTPException(...)).
2) Prefer specific exceptions where the library provides/uses distinct types
- Catch the exceptions that actually occur (e.g., AWS IMDS flows:
BotoCoreErrorandClientError; OpenAI/LiteLLM retries: include timeout/connection types you want retried). - For retry decorators or recovery paths, the exception tuple must match the real transient failures.
3) For non-critical side effects, fail open—but document and log
- Operations like “cleanup after update”, “load dedup markers”, “best-effort provider probes” must never break publishing.
- If you must use a broad
except, keep it broad only in these fail-open zones, and add a comment explaining why narrowing would be unsafe.
4) Re-raise correctly to preserve traceback
- Prefer
raiseoverraise ewhen rethrowing.
5) Don’t use bare/empty catches
- Avoid
except:and avoid emptyexceptblocks. - If a catch is intentionally ignored, at least log (or include structured artifacts) explaining the degraded behavior.
6) Make errors observable and stable
- Log with context; when relevant, attach error details via structured artifacts instead of embedding unstable exception formatting.
- Don’t assume env vars are always present—guard and wrap file/env-dependent code in try/except.
Example (safe fail-open cleanup + correct re-raise + framework raise)
from fastapi import HTTPException
from pr_agent.log import get_logger
def handler():
# framework boundary: must raise
raise HTTPException(status_code=503, detail="Service unavailable")
def best_effort_cleanup(git_provider, comment_url, cleanup_fn):
# non-critical side effect: fail-open
try:
cleanup_fn()
except Exception as cleanup_error:
get_logger().warning(
f"Cleanup failed for {comment_url}; continuing. error={cleanup_error}"
)
def retryable_call():
try:
do_request()
except KnownTransientError:
raise # allow retry framework to handle
except Exception:
get_logger().exception("Unexpected error; not retrying")
raise
When to broaden exceptions: only for explicitly fail-open side effects (dedup loads, cleanup, best-effort provider operations). Otherwise, narrow to specific, meaningful exceptions and re-raise properly when you must abort.