SameerMesiah97 commented on code in PR #70057:
URL: https://github.com/apache/airflow/pull/70057#discussion_r3609594870
##########
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:
is` except Exception` intentionally this broad? It seems like this would
also swallow programming errors and other non-transient failures. Would it make
sense to catch the specific infrastructure exceptions we're expecting (e.g.
database/connection errors), while allowing genuinely unexpected exceptions to
surface?
--
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]
