fix(runtime-sdk): chain the app's exception onto a reported lifespan failure - #237
fix(runtime-sdk): chain the app's exception onto a reported lifespan failure#237whitphx wants to merge 1 commit into
Conversation
…failure A reported startup or shutdown failure surfaced as a bare RuntimeError carrying only the message text, so the app's own exception reached callers nowhere. Frameworks re-raise it right after sending the failed event, and in that same-tick case the awaiter has not resumed, so attaching it there delivers it as the reported error's cause.
1d73c14 to
f7783dd
Compare
|
Sounds reasonable to me, but let me check if this behavior matches with other packages |
|
@ask-bonk review |
| # and the attachment still reaches it as the cause: | ||
| # https://github.com/encode/starlette/blob/1.3.1/starlette/routing.py | ||
| for reported in (startup, shutdown_complete): | ||
| if reported.done() and not reported.cancelled(): |
There was a problem hiding this comment.
Future.done() remains true after await startup or await shutdown_complete has already resumed. If an app yields after sending lifespan.*.failed and then raises, the caller can observe the RuntimeError with no cause before this code retroactively mutates it; this branch also returns instead of logging that later exception, which regresses the previous behavior. Please limit attachment to the same event-loop turn as the failed event, or otherwise fall through to the existing logging path.
There was a problem hiding this comment.
Future.done() remains true after await startup or await shutdown_complete has already resumed. If an app yields after sending lifespan.*.failed and then raises, the caller can observe the RuntimeError with no cause before this code retroactively mutates it;
Not sure this matters.
this branch also returns instead of logging that later exception, which regresses the previous behavior.
This part makes sense to me.
|
Submitted 1 actionable inline finding. |
When an app reports
lifespan.startup.failed, the caller gets aRuntimeErrorcarrying only the message text, so the app's own exception object reaches nobody. This attaches it as the reported error's__cause__, on the startup and shutdown paths alike.Test Plan
(run from
packages/runtime-sdk)