airflow
airflow copied to clipboard
Improve dag error handling
Improve error handling for DAG access in API endpoints
This PR improves error handling when accessing DAGs via API endpoints:
- Add consistent error handling for DAG access across API endpoints
- Handle ImportError and SyntaxError with 422 status code and clear error messages
- Handle generic exceptions with 500 status code
- Provide clear error messages for different failure scenarios
- Update API documentation to include new error response codes
- Add comprehensive tests for error handling scenarios
This change ensures users get clear feedback when DAGs fail to load due to import errors, syntax errors, or other unexpected issues.
closes: #48960
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst) Here are some useful points:
- Pay attention to the quality of your code (ruff, mypy and type annotations). Our pre-commits will help you with that.
- In case of a new feature add useful documentation (in docstrings or in
docs/directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it. - Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the Airflow Coding style.
- Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits. Apache Airflow is a community-driven project and together we are making it better 🚀. In case of doubts contact the developers at: Mailing List: [email protected] Slack: https://s.apache.org/airflow-slack
Don't hesitate to directly resolve comments you've addressed so we know easily what's left to focus on.
The main problem of this is it will really handle only a precise deserialization errror => dag_id is missing from the encoded dag.
Any other undexpected error in the deserialization process, will just crash and not provide any specific feedback.
I think we shoul catch everything in
SerializedDAG.deserialize_dagand raise based on the exception a new customDeserializationError.Then all those deserialization errors can be catch on the API side and provide a relevant feedback.
Hi @pierrejeambrun, Thanks for the review. I’ve raised a custom DeserializationError and handled it in SerializedDAG.deserialize_dag. I also noticed the new dependency DagBagDep, and I’ve merged it into get_dag_from_dag_bag.
Hi @pierrejeambrun, Thanks for the review and suggestion. I have catched all the Runtime error in the DeserializationError and simplify the dag_bag.get_dag handle.
@Yusin0903 just checking to see if you got a chance to work on the review comments?
@phanikumv Thanks for the reminder! I’ve been a bit tied up, but I’ll make sure to complete the review comments shortly.
Looks good to me except for one question above
mostly good, a few minor improvements left
some changes have been made in the main branches. During fixing those, I also rewrote the test cases. no need for a new PR
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.