Narrow broad except Exception handlers to specific types

[JuliusScheuerer]

Summary

• Separate known processing errors (ValueError, RuntimeError, TypeError) from unexpected errors in 3 route handlers, health check, and app lifespan
• Known errors log at WARNING level with error message; unexpected errors log full tracebacks at ERROR level
• User-facing behavior unchanged — all errors still return friendly error fragments
• health.py and app.py lifespan now catch (ImportError, OSError, RuntimeError, ValueError) for model loading failures

Test plan

• make check passes (275 tests, 95.58% coverage)
• All pre-commit hooks pass
• Health check tests still verify both ok and degraded states

[greptile-apps]

Greptile Summary

This PR refines broad except Exception handlers across three files into two-tier exception handling: known NLP/model errors (ValueError, RuntimeError, TypeError) log at WARNING, while unexpected errors retain ERROR-level traceback logging. User-facing behavior is unchanged — all paths still return the same friendly error fragments.

Changes:

• routes.py: All three form route handlers (detect_form, anonymize_form, redact_pdf_form) now have an explicit except (ValueError, RuntimeError, TypeError) block before the catch-all except Exception, correctly separating anticipated Presidio/NLP failures from true surprises. Implementation is consistent and clean.
• health.py: Narrows the catch from Exception to (ImportError, OSError, RuntimeError, ValueError). Behavioral change: unexpected exceptions from get_analyzer() now propagate as an unhandled 500 instead of returning status="degraded", breaking the health endpoint's "always return a structured response" contract.
• app.py: Same narrowing in the lifespan handler. Unexpected startup exceptions still crash the app, but the structured startup_failed log event is no longer emitted for them, reducing observability.

Confidence Score: 4/5

• Safe to merge after addressing the health check contract regression — route handler changes are clean and correct.
• The two-tier exception handling pattern in routes.py is well-implemented with no functional regressions. The one concrete issue is in health.py: narrowing the catch breaks the "always return a structured HealthResponse" guarantee, which can affect monitoring (load balancers, Kubernetes probes) that distinguish 503-degraded from 500-error. A short fix (add a fallback except Exception block that still degrades gracefully) resolves this. The app.py gap is a minor observability concern only. The PR is on the happy path to merge with one targeted fix remaining.
• src/document_anonymizer/health.py — the narrowed handler breaks the health endpoint's guaranteed structured-response contract

Important Files Changed

Filename	Overview
src/document_anonymizer/api/app.py	Narrows lifespan exception handler from Exception to (ImportError, OSError, RuntimeError, ValueError). Unexpected startup failures will still crash the app but won't emit the structured startup_failed log event.
src/document_anonymizer/health.py	Narrows health check exception handler, breaking the "always return HealthResponse" contract — unexpected exceptions from get_analyzer() now propagate as 500 instead of a structured status="degraded" 503 response.
src/document_anonymizer/web/routes.py	Adds two-tier exception handling (known errors → WARNING, unexpected → ERROR with traceback) to all three form route handlers. Implementation is consistent, correct, and the user-facing behavior is unchanged.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Request arrives at route handler] --> B{Processing}
    B -->|Success| C[Return HTML fragment / PDF]
    B -->|ValueError / RuntimeError / TypeError| D[log.WARNING with error message]
    D --> E[Return error_fragment.html — HTTP 200 or 500]
    B -->|Any other Exception| F[log.ERROR with full traceback]
    F --> E

    subgraph health.py [health.py check_health]
        H[get_analyzer] -->|ImportError / OSError / RuntimeError / ValueError| I[log.WARNING → status=degraded]
        H -->|Other Exception NEW| J[⚠️ Propagates as 500]
        H -->|Success| K[analyzer_ready=true]
    end

    subgraph lifespan [app.py lifespan]
        L[get_analyzer] -->|ImportError / OSError / RuntimeError / ValueError| M[log.EXCEPTION → raise → app fails to start]
        L -->|Other Exception NEW| N[⚠️ No structured log → raise → app fails to start]
        L -->|Success| O[yield — app starts]
    end

Loading

Comments Outside Diff (1)

1. src/document_anonymizer/health.py, line 39-46 (link)

   Health check contract broken for unexpected exceptions

   Before this PR, check_health() was guaranteed to always return a HealthResponse — it was a defensive "catch everything, degrade gracefully" function. Now, any exception from get_analyzer() that isn't one of (ImportError, OSError, RuntimeError, ValueError) (e.g. AttributeError, MemoryError, KeyError) will propagate uncaught all the way up to the /health endpoint, where the global Exception handler in app.py returns a 500 JSON error.

   This is a silent behavioral change that can affect monitoring:

   • Load balancers and Kubernetes probes checking /health will see 500 instead of 503 {"status": "degraded"}.
   • Monitoring systems that key off status="degraded" won't see it for these unexpected failures.

   The intent of this PR is to improve observability for unexpected errors via ERROR-level logging. The safest fix is to add a fallback that still degrades gracefully but logs at ERROR:

       except (ImportError, OSError, RuntimeError, ValueError) as exc:
           logger.warning(
               "health_check_failed",
               component="analyzer",
               error=str(exc),
               error_type=type(exc).__name__,
           )
           response.status = "degraded"
       except Exception as exc:
           logger.exception(
               "health_check_unexpected_error",
               component="analyzer",
               error_type=type(exc).__name__,
           )
           response.status = "degraded"

   This preserves the guaranteed-structured-response contract while still routing unexpected errors to ERROR-level logging for investigation.

_{Reviews (1): Last reviewed commit: "Narrow broad except Exception handlers t..." | Re-trigger Greptile}