kjh0623 commented on code in PR #70057:
URL: https://github.com/apache/airflow/pull/70057#discussion_r3622418416
##########
airflow-core/src/airflow/api_fastapi/auth/middlewares/refresh_token.py:
##########
@@ -64,6 +68,18 @@ async def dispatch(self, request: Request, call_next):
# Receive a AuthManagerRefreshTokenExpiredException when
the potential underlying refresh
# token used by the auth manager is expired
new_token = ""
+ except Exception:
+ # This middleware runs outside the FastAPI exception
handlers, so any
+ # other exception raised while resolving/refreshing the
user - e.g. an
+ # OperationalError from a dropped database connection -
would surface
+ # as a bare 500 for a request that may not even need the
refresh.
+ # Proceed without injecting the user (the route's own auth
dependency
+ # revalidates the token) and leave the cookie untouched so
a later
+ # request can refresh once the backend recovers.
+ log.warning(
Review Comment:
Done — switched the unexpected arm to `log.exception()` in d450d2d, so the
swallowed error is logged at ERROR with its traceback instead of WARNING.
The catch stays broad for the reason in my earlier comment (the middleware
delegates the lookup to whatever auth manager is configured, so it can't
enumerate which exceptions are transient for that backend without baking in a
DB assumption). What your point does change is visibility, and that part was
fair: at ERROR, a genuine bug is at least as loud as the bare 500 it replaces,
rather than hiding at WARNING. The test now asserts both the level and that
`exc_info` is attached.
--
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]
