Uh oh!
There was an error while loading. Please reload this page.
Don't crash supervisor IPC loop on transient network errors - #66572
Don't crash supervisor IPC loop on transient network errors#66572vatsrahul1001 merged 2 commits into
Conversation
handle_requests in the supervisor only caught ServerResponseError. Any non-HTTP exception (httpx.ConnectError, httpx.TimeoutException, socket timeouts, etc.) would propagate, terminate the generator, and permanently break the supervisor-to-task IPC channel. The task subprocess would then get EOFError on every subsequent send, and the worker would be stuck waiting for replies that never come. Add a catch-all except Exception after the ServerResponseError handler that logs the unhandled exception with type info, sends a best-effort ErrorResponse(API_SERVER_ERROR, ...) back to the task so the failure surfaces in task logs (wrapped in suppress(Exception) because if we can't reach the task subprocess via stdin we shouldn't double-fault), and lets the request loop continue to the next request. Test added: a fake httpx.ConnectError on the first call produces an ErrorResponse, the generator stays alive, and a second request is processed normally (the loop is not dead). Reported by the L3 ASVS sweep at apache/tooling-agents#24 (FINDING-005).
0f0a6ff to
067a4eeComparepotiuk
commented
May 17, 2026
I'd love to get this one merged — and would love it in 3.2.2 if it's not too late. cc @vatsrahul1001 (3.2.2 RM) Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting |
Uh oh!
There was an error while loading. Please reload this page.
vatsrahul1001
commented
May 18, 2026
@potiuk can you address comments? |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Shorten the catch-all comment per amoghrajesh's suggestion. - Use exc_info=e in log.exception instead of exception_type field per jason810496's suggestion (exception type is redundant since the exception itself is logged with full type info and traceback).
Uh oh!
There was an error while loading. Please reload this page.
Backport failed to create: v3-2-test. View the failure log Run detailsNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
You can attempt to backport this manually by running: cherry_picker 1e5d799 v3-2-testThis should apply the commit to the v3-2-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continueIf you don't have cherry-picker installed, see the installation guide. |
…67177) * Don't crash supervisor IPC loop on transient network errors handle_requests in the supervisor only caught ServerResponseError. Any non-HTTP exception (httpx.ConnectError, httpx.TimeoutException, socket timeouts, etc.) would propagate, terminate the generator, and permanently break the supervisor-to-task IPC channel. The task subprocess would then get EOFError on every subsequent send, and the worker would be stuck waiting for replies that never come. Add a catch-all except Exception after the ServerResponseError handler that logs the unhandled exception with type info, sends a best-effort ErrorResponse(API_SERVER_ERROR, ...) back to the task so the failure surfaces in task logs (wrapped in suppress(Exception) because if we can't reach the task subprocess via stdin we shouldn't double-fault), and lets the request loop continue to the next request. Test added: a fake httpx.ConnectError on the first call produces an ErrorResponse, the generator stays alive, and a second request is processed normally (the loop is not dead). Reported by the L3 ASVS sweep at apache/tooling-agents#24 (FINDING-005). * Address review comments: shorten comment and use exc_info - Shorten the catch-all comment per amoghrajesh's suggestion. - Use exc_info=e in log.exception instead of exception_type field per jason810496's suggestion (exception type is redundant since the exception itself is logged with full type info and traceback). (cherry picked from commit 1e5d799) Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
…67177) * Don't crash supervisor IPC loop on transient network errors handle_requests in the supervisor only caught ServerResponseError. Any non-HTTP exception (httpx.ConnectError, httpx.TimeoutException, socket timeouts, etc.) would propagate, terminate the generator, and permanently break the supervisor-to-task IPC channel. The task subprocess would then get EOFError on every subsequent send, and the worker would be stuck waiting for replies that never come. Add a catch-all except Exception after the ServerResponseError handler that logs the unhandled exception with type info, sends a best-effort ErrorResponse(API_SERVER_ERROR, ...) back to the task so the failure surfaces in task logs (wrapped in suppress(Exception) because if we can't reach the task subprocess via stdin we shouldn't double-fault), and lets the request loop continue to the next request. Test added: a fake httpx.ConnectError on the first call produces an ErrorResponse, the generator stays alive, and a second request is processed normally (the loop is not dead). Reported by the L3 ASVS sweep at apache/tooling-agents#24 (FINDING-005). * Address review comments: shorten comment and use exc_info - Shorten the catch-all comment per amoghrajesh's suggestion. - Use exc_info=e in log.exception instead of exception_type field per jason810496's suggestion (exception type is redundant since the exception itself is logged with full type info and traceback). (cherry picked from commit 1e5d799) Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
…67177) * Don't crash supervisor IPC loop on transient network errors handle_requests in the supervisor only caught ServerResponseError. Any non-HTTP exception (httpx.ConnectError, httpx.TimeoutException, socket timeouts, etc.) would propagate, terminate the generator, and permanently break the supervisor-to-task IPC channel. The task subprocess would then get EOFError on every subsequent send, and the worker would be stuck waiting for replies that never come. Add a catch-all except Exception after the ServerResponseError handler that logs the unhandled exception with type info, sends a best-effort ErrorResponse(API_SERVER_ERROR, ...) back to the task so the failure surfaces in task logs (wrapped in suppress(Exception) because if we can't reach the task subprocess via stdin we shouldn't double-fault), and lets the request loop continue to the next request. Test added: a fake httpx.ConnectError on the first call produces an ErrorResponse, the generator stays alive, and a second request is processed normally (the loop is not dead). Reported by the L3 ASVS sweep at apache/tooling-agents#24 (FINDING-005). * Address review comments: shorten comment and use exc_info - Shorten the catch-all comment per amoghrajesh's suggestion. - Use exc_info=e in log.exception instead of exception_type field per jason810496's suggestion (exception type is redundant since the exception itself is logged with full type info and traceback). (cherry picked from commit 1e5d799) Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
…67177) * Don't crash supervisor IPC loop on transient network errors handle_requests in the supervisor only caught ServerResponseError. Any non-HTTP exception (httpx.ConnectError, httpx.TimeoutException, socket timeouts, etc.) would propagate, terminate the generator, and permanently break the supervisor-to-task IPC channel. The task subprocess would then get EOFError on every subsequent send, and the worker would be stuck waiting for replies that never come. Add a catch-all except Exception after the ServerResponseError handler that logs the unhandled exception with type info, sends a best-effort ErrorResponse(API_SERVER_ERROR, ...) back to the task so the failure surfaces in task logs (wrapped in suppress(Exception) because if we can't reach the task subprocess via stdin we shouldn't double-fault), and lets the request loop continue to the next request. Test added: a fake httpx.ConnectError on the first call produces an ErrorResponse, the generator stays alive, and a second request is processed normally (the loop is not dead). Reported by the L3 ASVS sweep at apache/tooling-agents#24 (FINDING-005). * Address review comments: shorten comment and use exc_info - Shorten the catch-all comment per amoghrajesh's suggestion. - Use exc_info=e in log.exception instead of exception_type field per jason810496's suggestion (exception type is redundant since the exception itself is logged with full type info and traceback). (cherry picked from commit 1e5d799) Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Summary
handle_requestsin the supervisor only caughtServerResponseError. Any non-HTTP exception (httpx.ConnectError,httpx.TimeoutException, socket timeouts, etc.) would propagate, terminate the generator, and permanently break the supervisor↔task IPC channel. The task subprocess would then getEOFErroron every subsequent send, and the worker would be stuck waiting for replies that never come.Fix
Add a catch-all
except Exceptionafter theServerResponseErrorhandler that:ErrorResponse(API_SERVER_ERROR, …)back to the task so the failure surfaces in task logs (wrapped insuppress(Exception)because if we can't reach the task subprocess via stdin we shouldn't double-fault).Test added: a fake
httpx.ConnectErroron the first call produces anErrorResponse, the generator stays alive, and a second request is processed normally (the loop is not dead).Reported by
L3 ASVS sweep — apache/tooling-agents#24 (FINDING-005).
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 4.7) following the guidelines