ICRAR / daliuge

The DALiuGE Execution Engine
GNU Lesser General Public License v2.1
24 stars 7 forks source link

LIU-322: Catch async exceptions #215

Closed juliancarrivick closed 1 year ago

juliancarrivick commented 1 year ago

Alright, attempt no. 2: now with correct ticket numbers. This supersedes #214.

I've retained the type-hints and one of the tests I originally added in #214 as there were no functional changes there and there's no point throwing away those enhancements (plus it was useful for adding further tests below).

Exception handling is now encapsulated around the invocation of execute() in async_execute() with a helper method which wraps the call in try/except.

I didn't realise this until I went to write the tests, but this logic only occurs in a InputFiredAppDROP which means I've only fixed this case for drops of that type. I expected this logic to be on AbstractDrop which is why I suspect I may have originally led myself to do this handling in the EventFirer. Though now I think about it this makes sense since not all drops execute code (just AppDrops if I'm not mistaken), so I think we're OK there. Hopefully you can follow my though process there :sweat_smile:

juliancarrivick commented 1 year ago

Need to debug why build is timing out before this can be looked at!

coveralls commented 1 year ago

Coverage Status

Coverage increased (+0.05%) to 82.143% when pulling b71eae025711969ec54e9b17e2fdecdd4e879f58 on liu-322-top-level-catch into e383be766d9bd416563a9940d986700cac526fc6 on master.