Use an error handler in the dispatcher

[alanz]

If there is an error in any of the plugin commands, catch the error and return it wrapped in IdeResponseError

This should be done in the main dispatcher loop:

doDispatch :: PluginId -> PluginDescriptor -> Dispatcher
doDispatch pn desc req = do
  plugins <- getPlugins
  debugm $ "doDispatch:desc=" ++ show desc
  debugm $ "doDispatch:req=" ++ show req
  case Map.lookup (pn,ideCommand req) (pluginCache plugins) of
    Nothing -> return (IdeResponseError (toJSON $ "No such command:" <> ideCommand req))
    Just cmd -> (cmdFunc cmd) req

The call to (cmdFunc cmd) req should be wrapped in an error handler.

[JPMoresmau]

I know this is probably a stupid question, but the doc for Control.Exception.catch says it's usually a bad idea to catch all exceptions, as it may impact the program behavior. But surely we don't know what type of exceptions plugins are susceptible to raise? Or is there some exceptions that we'll know we should throw again?

[alanz]

To be honest I am not sure. In my view the transports and dispatcher should provide a long-lived service, and if a specific plugin goes bad it should not bring the whole thing down. Or maybe it should, but with a good error message somewhere so it can be dealt with.

[cocreature]

An example of an exception which should not be catched is UserInterrupt. Otherwise typing Control+C won’t kill hie which is really annoying (in fact it looks like we already have problems with that). The real solution to this problem would probably involve forking off the plugins and so making everything async (which I want to do anyway, since otherwise plugins can cause hie to hang). Forking off will solve both problems at the same time, UserInterrupt will still cause hie to exit since it’s always the main thread that receives it and plugins can’t bring down the main thread. But for that we need to first figure out #78.

[alanz]

This can only work if there is a way of accessing the underlying GHC(ghc-mod) session, as it gets lost on a forkIO.

[cocreature]

Yep, that’s what I’m hoping #78 will do.

[cocreature]

Taking another look, the dispatcher is running in a separate thread so it should never receive things such as UserInterrupt so I would argue that in this case catching all exceptions is the correct thing to do.