Document exception handling in Actions/Rules #3407
Conversation
I also took the liberty of removing the `MonadFail` instance for `Rules`, since failing in `Rules` seems much worse as it won't automatically be caught. There was only one use of that, which now has an appropriately threatening comment.
| -- | A computation that defines all the rules that form part of the computation graph. | ||
| -- | ||
| -- 'Rules' has access to 'IO' through 'MonadIO'. Use of 'IO' is at your own risk: if | ||
| -- you write 'Rules' that thow exceptions, then you need to make sure to handle them |
There was a problem hiding this comment.
| -- you write 'Rules' that thow exceptions, then you need to make sure to handle them | |
| -- you write 'Rules' that throw exceptions, then you need to make sure to handle them |
| -- | An action representing something that can be run as part of a 'Rule'. | ||
| -- | ||
| -- 'Action's can be pure functions but also have access to 'IO' via 'MonadIO' and 'MonadUnliftIO. | ||
| -- It should be assumed that actions can throw exceptions, these can be caught with |
There was a problem hiding this comment.
Sorry for nitpicking, but who assumes these actions can throw exceptions? The user, or the Rules engine? Or both? E.g., are exceptions always caught or may they bubble up and eventually crash HLS?
There was a problem hiding this comment.
It's unclear! I'm writing this for the "user of hls-graph", which in this case is HLS itself, the point being that hls-graph does not do anything about the exceptions, but the user can. Now HLS does use actionCatch to catch such exceptions and turn them into diagnostics, but it's a bit of a layering violation to write that here. Perhaps I should say it anyway?
Conceivably some other user of hls-graph could choose to just let them bubble up and crash the system...
There was a problem hiding this comment.
Ah I see, then slightly adapting the beginning of the sentence seems fine by me.
| -- It should be assumed that actions can throw exceptions, these can be caught with | |
| -- It should be assumed that actions throw exceptions, these can be caught with |
Maybe this is better? Or maybe just go with what you think is better, I am sure your English is better than mine.
|
I think one relevant difference to Shake broadly assumes that it's the top-level process, and so e.g. failing during |
I also took the liberty of removing the
MonadFailinstance forRules, since failing inRulesseems much worse as it won't automatically be caught. There was only one use of that, which now has an appropriately threatening comment.