Skip to content

Commit 8f0d940

Browse files
Document exception handling in Actions/Rules (#3407)
* Document exception handling in Actions/Rules 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. * Tweak Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
1 parent 90d17fe commit 8f0d940

File tree

2 files changed

+18
-3
lines changed
  • ghcide/src/Development/IDE/Core
  • hls-graph/src/Development/IDE/Graph/Internal

2 files changed

+18
-3
lines changed

ghcide/src/Development/IDE/Core/Shake.hs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,13 +307,17 @@ type GetStalePersistent = NormalizedFilePath -> IdeAction (Maybe (Dynamic,Positi
307307

308308
getShakeExtras :: Action ShakeExtras
309309
getShakeExtras = do
310+
-- Will fail the action with a pattern match failure, but be caught
310311
Just x <- getShakeExtra @ShakeExtras
311312
return x
312313

313314
getShakeExtrasRules :: Rules ShakeExtras
314315
getShakeExtrasRules = do
315-
Just x <- getShakeExtraRules @ShakeExtras
316-
return x
316+
mExtras <- getShakeExtraRules @ShakeExtras
317+
case mExtras of
318+
Just x -> return x
319+
-- This will actually crash HLS
320+
Nothing -> liftIO $ fail "missing ShakeExtras"
317321

318322
-- | Returns the client configuration, creating a build dependency.
319323
-- You should always use this function when accessing client configuration

hls-graph/src/Development/IDE/Graph/Internal/Types.hs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,13 @@ unwrapDynamic x = fromMaybe (error msg) $ fromDynamic x
5454

5555
type TheRules = Map.HashMap TypeRep Dynamic
5656

57+
-- | A computation that defines all the rules that form part of the computation graph.
58+
--
59+
-- 'Rules' has access to 'IO' through 'MonadIO'. Use of 'IO' is at your own risk: if
60+
-- you write 'Rules' that throw exceptions, then you need to make sure to handle them
61+
-- yourself when you run the resulting 'Rules'.
5762
newtype Rules a = Rules (ReaderT SRules IO a)
58-
deriving newtype (Monad, Applicative, Functor, MonadIO, MonadFail)
63+
deriving newtype (Monad, Applicative, Functor, MonadIO)
5964

6065
data SRules = SRules {
6166
rulesExtra :: !Dynamic,
@@ -67,6 +72,12 @@ data SRules = SRules {
6772
---------------------------------------------------------------------
6873
-- ACTIONS
6974

75+
-- | An action representing something that can be run as part of a 'Rule'.
76+
--
77+
-- 'Action's can be pure functions but also have access to 'IO' via 'MonadIO' and 'MonadUnliftIO.
78+
-- It should be assumed that actions throw exceptions, these can be caught with
79+
-- 'Development.IDE.Graph.Internal.Action.actionCatch'. In particular, it is
80+
-- permissible to use the 'MonadFail' instance, which will lead to an 'IOException'.
7081
newtype Action a = Action {fromAction :: ReaderT SAction IO a}
7182
deriving newtype (Monad, Applicative, Functor, MonadIO, MonadFail, MonadThrow, MonadCatch, MonadMask, MonadUnliftIO)
7283

0 commit comments

Comments
 (0)