kjh0623 commented on PR #70057: URL: https://github.com/apache/airflow/pull/70057#issuecomment-5018414432
Good question — the broad catch is intentional, but you're right that the observability trade-off deserves a callout. **Why `except Exception` rather than a specific `OperationalError`:** this middleware is a generic auth layer that delegates the actual lookup to the configured auth manager (`_refresh_user` → `resolve_user_from_token`). It doesn't — and shouldn't — know which backend that auth manager uses, so it can't enumerate which exceptions are "transient infra." Catching `sqlalchemy.exc.OperationalError` here would bake a DB-backend assumption into a layer that's meant to be backend-agnostic, and it would still let any *other* transient cause surface as the exact bare 500 this PR is trying to remove. **Why failing open is safe regardless of the exception type:** the refresh is a best-effort, proactive step, not the authoritative auth check. If it doesn't inject a user, the route's own auth dependency revalidates the token exactly as it does today when the middleware chooses not to refresh. So a failure here is never a security downgrade — it just means "no proactive refresh this time," identical to the existing no-refresh path. **On the part I think is fair — masking programming errors:** they aren't silently swallowed (the traceback is logged with `exc_info=True`), but WARNING is easy to miss, and a monitor keyed on 5xx would go quiet on a genuine regression. If you'd prefer, I'll switch the unexpected arm to `log.exception(...)` (ERROR level) so real bugs still surface loudly in monitoring while transient blips stay non-fatal. That keeps the resilience without hiding regressions. Would that address the concern, or would you rather I narrow the catch despite the coupling/coverage downsides above? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
