airflow icon indicating copy to clipboard operation
airflow copied to clipboard

Improve dag error handling

Open Yusin0903 opened this issue 7 months ago • 4 comments

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.

Yusin0903 avatar Apr 12 '25 18:04 Yusin0903

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

boring-cyborg[bot] avatar Apr 12 '25 18:04 boring-cyborg[bot]

Don't hesitate to directly resolve comments you've addressed so we know easily what's left to focus on.

pierrejeambrun avatar May 05 '25 09:05 pierrejeambrun

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_dag and raise based on the exception a new custom DeserializationError.

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.

Yusin0903 avatar May 13 '25 18:05 Yusin0903

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 avatar May 25 '25 12:05 Yusin0903

@Yusin0903 just checking to see if you got a chance to work on the review comments?

phanikumv avatar Jul 31 '25 09:07 phanikumv

@phanikumv Thanks for the reminder! I’ve been a bit tied up, but I’ll make sure to complete the review comments shortly.

Yusin0903 avatar Aug 02 '25 09:08 Yusin0903

Looks good to me except for one question above

uranusjr avatar Aug 21 '25 06:08 uranusjr

mostly good, a few minor improvements left

Lee-W avatar Aug 26 '25 03:08 Lee-W

some changes have been made in the main branches. During fixing those, I also rewrote the test cases. no need for a new PR

Lee-W avatar Sep 02 '25 09:09 Lee-W

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

boring-cyborg[bot] avatar Sep 02 '25 10:09 boring-cyborg[bot]