Skip to content

Replace bare except with except Exception in task-sdk supervisor - #69544

Open
bramhanandlingala wants to merge 1 commit into
apache:mainfrom
bramhanandlingala:fix/#69441
Open

Replace bare except with except Exception in task-sdk supervisor#69544
bramhanandlingala wants to merge 1 commit into
apache:mainfrom
bramhanandlingala:fix/#69441

Conversation

@bramhanandlingala

@bramhanandlingala bramhanandlingala commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Description

Issue #69441 flags exception handlers that catch overly broad types,
which can hide real errors or interfere with signal handling. I looked
for bare except: clauses specifically, since those catch everything
including KeyboardInterrupt/SystemExit. There were two left in the
repo outside of tests/docs.

1. Task-SDK supervisor — the bare except around coordinator init
always re-raised afterward, so signals weren't actually swallowed, just
logged with a misleading traceback before exiting. Narrowed to
except Exception: so real errors are still logged the same way, but
Ctrl+C/shutdown no longer produce that spurious log line.

2. Pytest plugin — a bare except: raise that did nothing (pure
re-raise, no logging). Removed it; the finally below already handles
cleanup.

Didn't touch the other except Exception: blocks in core — those don't
have this bare-except issue, and a full rewrite is a separate effort.

related: #69441

Gen-AI disclosure: I used a generative AI tool to help identify the root
cause, write tests, and draft the PR description. I reviewed, tested, and
verified all changes locally before submitting.

Was generative AI tooling used to co-author this PR?
  • Yes — Claude

Generated-by: Claude following the guidelines

@Vamsi-klu

Copy link
Copy Markdown
Contributor

Verified both hunks against head. In supervisor.py the handler is log.exception(...) then raise, and for_queue only raises Exception subclasses (InvalidCoordinatorError/ValueError, plus any failure from the dynamically import_string-loaded coordinator class), so narrowing bare except: to except Exception: is behavior-preserving for every real error. The only runtime effect is that KeyboardInterrupt/SystemExit no longer get an extra traceback logged on their way out -- note they were already re-raised by the raise under the old bare except, so this suppresses a spurious log line rather than fixing swallowed signals. In pytest_plugin.py the removed except: raise was a pure no-op and the finally restoring AIRFLOW_CONFIG runs regardless, so removal is identical. Correct and cherry-pickable, just low impact. LGTM.

@potiuk potiuk added the ready for maintainer review Set after triaging when all criteria pass. label Jul 8, 2026
@bramhanandlingala

bramhanandlingala commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Hi @Vamsi-klu

Could you please review the code and approve the PR

@shahar1 shahar1 removed the ready for maintainer review Set after triaging when all criteria pass. label Jul 25, 2026

@shahar1 shahar1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants