Add throwPluginError to Plugin Utilities
#2924
Conversation
Add the function `throwPluginError`. This function is intended to provide a common `ResponseError` message for use in logging. Renamed `response` to `pluginResponse` for more clarity.
drsooch
requested review from
kokobd,
pepeiborra,
OliverMadine,
July541,
jneira and
berberman
as code owners
| @@ -144,14 +146,14 @@ p `isInsideRealSrcSpan` r = let (Range sp ep) = realSrcSpanToRange r in sp <= p | |||
|
|
|||
| getFirstPragma :: MonadIO m => IdeState -> NormalizedFilePath -> ExceptT String m NextPragmaInfo | |||
| getFirstPragma state nfp = handleMaybeM "Error: Could not get NextPragmaInfo" $ do | |||
| ghcSession <- liftIO $ runAction "AlternateNumberFormat.GhcSession" state $ useWithStale GhcSession nfp | |||
| (_, fileContents) <- liftIO $ runAction "AlternateNumberFormat.GetFileContents" state $ getFileContents nfp | |||
| ghcSession <- liftIO $ runAction (alternateNumberFormatId <> ".GhcSession") state $ useWithStale GhcSession nfp | |||
There was a problem hiding this comment.
This does make me feel like we need some kind of "component context" for our logs and errors.... seems to be one of those things that everyone invents eventually.
There was a problem hiding this comment.
Yeah #2919 I'm going to look to enhance the pluginRecorder to hopefully be able to decorate it with the plugin id. The other question is how to integrate that with runAction logging.
This is one of the big things I want to look at considering all plugins have this same model of id.Action.
There was a problem hiding this comment.
Yeah, I guess what I'm wondering is whether we want some more generic "log context" stack in our recorders or something. It comes up a lot.
There was a problem hiding this comment.
I strongly want something that can wrap runAction and return ExceptT String m ActionResult, then we can clean much boilerplate code.
|
@michaelpj I've been thinking about this more. Would it make sense to just log the error within Something like this: throwPluginError' :: (HasCallStack, MonadIO m) => Recorder (WithPriority String) -> PluginId -> String -> String -> ExceptT String m b
throwPluginError' r (PluginId who) what _where' = logWith r Warning msg >> throwE msg |
Now that I actually played around with it, I forgot we'd need to depend on |
That's basically what I was thinking about. The |
* Add new PluginUtility function. Add the function `throwPluginError`. This function is intended to provide a common `ResponseError` message for use in logging. Renamed `response` to `pluginResponse` for more clarity. * Call hierarchy clean up * Make Descriptor usable as String/Text or PluginID * Update reference to ChangeTypeSignature descriptor * Use unpack rather than show * Import cleanup * Merge cleanup * Fix test suites for effected plugins * forgot to change the CodeAction kind in the test suite... * Update new plugin
Add the function
throwPluginError. This function is intended toprovide a common
ResponseErrormessage for use in logging.Renamed
responsetopluginResponseinPluginUtilsfor more clarity.Shifting away from using
descriptor :: PluginID -> PluginDescriptor. Instead the plugin ID is pulled out and defined as part of a plugin directly. This provides easy access to the PluginID rather than having to feed it through functions.I used the
callHierarchyPluginas a pseudo-testing ground for how it would look in practice. There will be a follow up PR that actually makes use of the error message (see #2919).I've also updated plugins I wrote to use the new descriptor model.