-
-
Notifications
You must be signed in to change notification settings - Fork 347
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Various PluginError PR suggestions I missed earlier #3737
Changes from 32 commits
8adb03e
4e65cbe
d4a9d4e
e1b62cb
e34da52
ca81317
1776bd8
bc093c2
9bc837a
40d589c
fd11649
15cf638
cdc0364
d78f2bc
9b539d9
d415379
e618d7e
6fee73f
6cbe6d9
e0bbec1
6102e6b
953003a
9bbf421
afcf19d
77d264f
1a22bcc
28bdecc
1dd12d4
ba4fa79
89bba81
82e3ff5
a215904
f15ea45
8329b90
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
module Development.IDE.Plugin.HLS | ||
( | ||
asGhcIdePlugin | ||
, toResponseError | ||
, Log(..) | ||
) where | ||
|
||
|
@@ -80,10 +81,17 @@ prettyResponseError err = errorCode <> ":" <+> errorBody | |
errorCode = pretty $ show $ err ^. L.code | ||
errorBody = pretty $ err ^. L.message | ||
|
||
pluginNotEnabled :: SMethod m -> [(PluginId, b, a)] -> Text | ||
pluginNotEnabled :: SMethod m -> [PluginId] -> Text | ||
pluginNotEnabled method availPlugins = | ||
"No plugin enabled for " <> T.pack (show method) <> ", potentially available: " | ||
<> (T.intercalate ", " $ map (\(PluginId plid, _, _) -> plid) availPlugins) | ||
<> (T.intercalate ", " $ map (\(PluginId plid) -> plid) availPlugins) | ||
|
||
noPluginEnabled :: Recorder (WithPriority Log) -> SMethod m -> [PluginId] -> IO (Either ResponseError c) | ||
noPluginEnabled recorder m fs' = do | ||
logWith recorder Warning (LogNoPluginForMethod $ Some m) | ||
let err = ResponseError (InR ErrorCodes_MethodNotFound) msg Nothing | ||
msg = pluginNotEnabled m fs' | ||
return $ Left err | ||
|
||
pluginDoesntExist :: PluginId -> Text | ||
pluginDoesntExist (PluginId pid) = "Plugin " <> pid <> " doesn't exist" | ||
|
@@ -113,13 +121,6 @@ logAndReturnError recorder p errCode msg = do | |
logWith recorder Warning $ LogResponseError p err | ||
pure $ Left err | ||
|
||
-- | Logs the provider error before returning it to the caller | ||
logAndReturnError' :: Recorder (WithPriority Log) -> (LSPErrorCodes |? ErrorCodes) -> Log -> LSP.LspT Config IO (Either ResponseError a) | ||
logAndReturnError' recorder errCode msg = do | ||
let err = ResponseError errCode (fromString $ show msg) Nothing | ||
logWith recorder Warning $ msg | ||
pure $ Left err | ||
|
||
-- | Map a set of plugins to the underlying ghcide engine. | ||
asGhcIdePlugin :: Recorder (WithPriority Log) -> IdePlugins IdeState -> Plugin Config | ||
asGhcIdePlugin recorder (IdePlugins ls) = | ||
|
@@ -219,8 +220,15 @@ executeCommandHandlers recorder ecs = requestHandler SMethod_WorkspaceExecuteCom | |
Just (PluginCommand _ _ f) -> case A.fromJSON arg of | ||
A.Error err -> logAndReturnError recorder p (InR ErrorCodes_InvalidParams) (failedToParseArgs com p err arg) | ||
A.Success a -> do | ||
(first (toResponseError . (p,)) <$> runExceptT (f ide a)) `catchAny` -- See Note [Exception handling in plugins] | ||
(\e -> logAndReturnError' recorder (InR ErrorCodes_InternalError) (ExceptionInPlugin p (Some SMethod_WorkspaceApplyEdit) e)) | ||
res <- runExceptT (f ide a) `catchAny` -- See Note [Exception handling in plugins] | ||
(\e -> pure $ Left $ PluginInternalError (exceptionInPlugin p SMethod_WorkspaceExecuteCommand e)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this method? And it was different before?? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because that's it's actually method, see line 171. WorkspaceApplyEdit before was an error, either autocomplete typo, brain fog, or sloppy mistake on my part. |
||
case res of | ||
(Left (PluginRequestRefused _)) -> | ||
liftIO $ noPluginEnabled recorder SMethod_WorkspaceExecuteCommand (fst <$> ecs) | ||
(Left pluginErr) -> do | ||
liftIO $ logErrors recorder [(p, pluginErr)] | ||
pure $ Left $ toResponseError (p, pluginErr) | ||
(Right result) -> pure $ Right result | ||
|
||
-- --------------------------------------------------------------------- | ||
|
||
|
@@ -242,7 +250,7 @@ extensiblePlugins recorder xs = mempty { P.pluginHandlers = handlers } | |
let fs = filter (\(_, desc, _) -> pluginEnabled m params desc config) fs' | ||
-- Clients generally don't display ResponseErrors so instead we log any that we come across | ||
case nonEmpty fs of | ||
Nothing -> liftIO $ noPluginEnabled m fs' | ||
Nothing -> liftIO $ noPluginEnabled recorder m ((\(x, _, _) -> x) <$> fs') | ||
Just fs -> do | ||
let handlers = fmap (\(plid,_,handler) -> (plid,handler)) fs | ||
es <- runConcurrently exceptionInPlugin m handlers ide params | ||
|
@@ -255,16 +263,11 @@ extensiblePlugins recorder xs = mempty { P.pluginHandlers = handlers } | |
noRefused (_, _) = True | ||
filteredErrs = filter noRefused errs | ||
case nonEmpty filteredErrs of | ||
Nothing -> liftIO $ noPluginEnabled m fs' | ||
Nothing -> liftIO $ noPluginEnabled recorder m ((\(x, _, _) -> x) <$> fs') | ||
Just xs -> pure $ Left $ combineErrors xs | ||
Just xs -> do | ||
pure $ Right $ combineResponses m config caps params xs | ||
noPluginEnabled :: SMethod m -> [(PluginId, b, a)] -> IO (Either ResponseError c) | ||
noPluginEnabled m fs' = do | ||
logWith recorder Warning (LogNoPluginForMethod $ Some m) | ||
let err = ResponseError (InR ErrorCodes_MethodNotFound) msg Nothing | ||
msg = pluginNotEnabled m fs' | ||
return $ Left err | ||
|
||
|
||
-- --------------------------------------------------------------------- | ||
|
||
|
@@ -313,7 +316,6 @@ combineErrors :: NonEmpty (PluginId, PluginError) -> ResponseError | |
combineErrors (x NE.:| []) = toResponseError x | ||
combineErrors xs = toResponseError $ NE.last $ NE.sortWith (toPriority . snd) xs | ||
|
||
|
||
toResponseError :: (PluginId, PluginError) -> ResponseError | ||
toResponseError (PluginId plId, err) = | ||
ResponseError (toErrorCode err) (plId <> ": " <> tPretty err) Nothing | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this just be inlined into the function below now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good catch!