Skip to content

Plugin failures should not take down or make hang HLS #1231

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

Open
marcin-rzeznicki opened this issue Jan 19, 2021 · 7 comments
Open

Plugin failures should not take down or make hang HLS #1231

marcin-rzeznicki opened this issue Jan 19, 2021 · 7 comments
Labels
component: hls-eval-plugin component: plugins type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..

Comments

@marcin-rzeznicki
Copy link
Contributor

I encounter a case where entire server is crashed by a failed parse in Eval plugin:

haskell-language-server: Tokens.next failed to parse               
{- else -} (structLit Parser did not consume entire stream, left:>
CallStack (from HasCallStack):
  error, called at src/Ide/Plugin/Eval/Parse/Token.hs:134:19 in hls-eval-plugin-0.1.0.0-2Df7iIOQIttFUpUdbeyzFM:Ide.Plugin.Eval.Parse>

The code that fails to parse looks more or less like this:

        expr =
          let_
            1 {- = -} (setField "foo" (stringLit "hello") (v 0)) {- in -} $
          let_
            2 {- = -} (setField "bar" (stringLit "world") (v 1)) {- in -} $
          let_
            3 {- = -} (AST.ExprIf (AST.ExprFunction (AST.StringIsEmpty (getField "foo" (v 2))))
               {- then -} (setField "bar" (stringLit "hello world") (v 2))
               {- else -} (structLit
                [ "foo" .= StringValue "foo"
                , "bar" .= StringValue "bar"
                , "baz" .= StringValue "baz"
                ])) {- in -} $
          v 3

GHC has no problems with it.

In my humble opinion, the problem is not that this piece fails to parse for whatever reason but rather in that it crashes HLS.
Hope it helps! Thank you in advance

Your environment

Output of haskell-language-server --probe-tools or haskell-language-server-wrapper --probe-tools:

haskell-language-server version: 0.8.0.0 (GHC: 8.8.4) (PATH: /nix/store/3dkwi0xnifx6882f1czw9x7hsv63qj0x-haskell-language-server-0.8.0.0/bin/haskell-language-server)
Tool versions found on the $PATH
cabal:		Not found
stack:		2.5.1
ghc:		8.10.3

Which lsp-client do you use:

VS Codium

Describe your project (alternative: link to the project):

N/A

Contents of hie.yaml: N/A

Steps to reproduce

In general, you have to find a piece that triggers the above behavior in Eval plugin and include it in a project. Judging by the logs, I'd say that it's caused by keywords in comments (but that does not seem right). You can, of course use, the one I provided (in case it didn't reproduce, I could try to find a minimal reproducer)

Expected behaviour

HLS should not crash (regardless of whether a piece of code can be parsed by the plugin or not)

Actual behaviour

HLS crashes - VS displays pop-up that it crashed more than x times and won't be restarted

Include debug information

Execute in the root of your project the command haskell-language-server --debug . and paste the logs here:

Debug output:
<paste your logs here>

Paste the logs from the lsp-client, e.g. for VS Code

LSP logs:
haskell-language-server version: 0.8.0.0 (GHC: 8.8.4) (PATH: /nix/store/3dkwi0xnifx6882f1czw9x7hsv63qj0x-haskell-language-server-0.8.0.0/bin/haskell-language-server)
Starting (haskell-language-server)LSP server...
  with arguments: LspArguments {argLSP = True, argsCwd = Nothing, argFiles = [], argsShakeProfiling = Nothing, argsTesting = False, argsExamplePlugin = False, argsDebugOn = False, argsLogFile = Nothing, argsThreads = 0, argsProjectGhcVersion = False}
  with plugins: [PluginId "brittany",PluginId "class",PluginId "eval",PluginId "floskell",PluginId "ghcide",PluginId "hlint",PluginId "importLens",PluginId "moduleName",PluginId "ormolu",PluginId "pragmas",PluginId "retrie",PluginId "stylish-haskell",PluginId "tactic"]
  in directory: /home/marcin/channable/sharkmachine
If you are seeing this in a terminal, you probably should have run ghcide WITHOUT the --lsp option!
 Started LSP server in 0.00s
2021-01-19 14:37:26.487612469 [ThreadId 25] - Registering ide configuration: IdeConfiguration {workspaceFolders = fromList [NormalizedUri (-8084739005052116551) "file:///home/marcin/channable/sharkmachine"], clientSettings = hashed Nothing}
2021-01-19 14:37:26.48896058 [ThreadId 25] - Configuration changed: Object (fromList [("haskell",Object (fromList [("logFile",String ""),("updateBehavior",String "never-check"),("hlintOn",Bool True),("formatOnImportOn",Bool True),("indentationRules",Object (fromList [("enabled",Bool True)])),("liquidOn",Bool False),("languageServerVariant",String "haskell-language-server"),("serverExecutablePath",String "${workspaceFolder}/contrib/haskell-language-server.sh"),("diagnosticsOnChange",Bool True),("completionSnippetsOn",Bool True),("maxNumberOfProblems",Number 30.0),("formattingProvider",String "stylish-haskell"),("trace",Object (fromList [("server",String "off")]))]))])

...

2021-01-19 14:37:44.144813922 [ThreadId 113] - Using interface files cache dir: ghcide
2021-01-19 14:37:44.144874525 [ThreadId 113] - Making new HscEnv[main]

...

2021-01-19 14:37:44.992944709 [ThreadId 1992] - finish: CodeAction (took 0.00s)
2021-01-19 14:37:45.73748366 [ThreadId 2427] - finish: codeLens (took 1.28s)
2021-01-19 14:37:45.738428253 [ThreadId 2433] - finish:  (took 0.00s)
2021-01-19 14:37:45.739461973 [ThreadId 2434] - finish: ModuleName.ghcSession (took 0.00s)
2021-01-19 14:37:45.739877384 [ThreadId 2436] - finish: ModuleName.GetParsedModule (took 0.00s)
haskell-language-server: Tokens.next failed to parse               {- else -} (structLit Parser did not consume entire stream, left: "(structLit"
CallStack (from HasCallStack):
  error, called at src/Ide/Plugin/Eval/Parse/Token.hs:134:19 in hls-eval-plugin-0.1.0.0-2Df7iIOQIttFUpUdbeyzFM:Ide.Plugin.Eval.Parse.Token
2021-01-19 14:37:45.925001278 [ThreadId 2460] - finish: CodeAction:PackageExports (took 0.93s)
2021-01-19 14:37:45.925005508 [ThreadId 2459] - finish: CodeAction:PackageExports (took 1.36s)
2021-01-19 14:37:45.92524896 [ThreadId 2461] - finish: importLens (took 0.00s)
2021-01-19 14:37:45.925462936 [ThreadId 2463] - finish: addPragma (took 0.00s)
2021-01-19 14:37:45.925708555 [ThreadId 2465] - finish: retrie (took 0.00s)
2021-01-19 14:37:45.925910353 [ThreadId 2467] - finish: tactic (took 0.00s)
2021-01-19 14:37:45.926165106 [ThreadId 2472] - finish: tactic (took 0.00s)
2021-01-19 14:37:45.926210695 [ThreadId 2473] - finish: tactic (took 0.00s)
[Info  - 2:37:46 PM] Connection to server got closed. Server will restart.
[Error - 2:37:46 PM] Request textDocument/codeLens failed.
Error: Connection got disposed.
	at Object.dispose (/home/marcin/.vscode/extensions/haskell.haskell-1.2.0/dist/extension.js:1:90964)

@konn
Copy link
Collaborator

konn commented Jan 19, 2021

It seems somewhat similar to #1214. I'm addressing the root cause in #1232.

@marcin-rzeznicki
Copy link
Contributor Author

Thanks a lot! Indeed, feels similar (and, in fact, removing all these comments helped). Do you want me to close this one then, or would you like to keep it?

@ndmitchell
Copy link
Collaborator

It seems a bug that a dodgy plugin can take down anything - we should have approximately everything guarded by a catch, and always recover. So I think this bug report points to a lack of catch, and is thus useful separately.

@marcin-rzeznicki
Copy link
Contributor Author

👌 Makes sense and, in fact, this has been my biggest concern:

In my humble opinion, the problem is not that this piece fails to parse for whatever reason but rather in that it crashes HLS.

@jneira jneira added component: hls-eval-plugin type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc.. labels Jan 20, 2021
@ndmitchell ndmitchell changed the title Eval Plugin: Failed parse takes down entire server Plugin failures should not take down HLS Jan 23, 2021
@avh4
Copy link

avh4 commented Feb 20, 2021

I think I'm seeing this crash as well:

haskell-language-server-0.9.0-linux-8.10.3: Tokens.next failed to parse   ( {- nothing -} ) Parser did not consume entire stream, left: ")"
CallStack (from HasCallStack):
  error, called at src/Ide/Plugin/Eval/Parse/Token.hs:146:19 in hls-eval-plugin-0.1.0.1-inplace:Ide.Plugin.Eval.Parse.Token

The input it seems to be complaining about "( {- nothing -} )" is actually text that's inside of a documentation comment in one of my hs files.

@jneira jneira changed the title Plugin failures should not take down HLS Plugin failures should not take down or make hang HLS Jan 31, 2022
@michaelpj
Copy link
Collaborator

Well, plugins certainly shouldn't be failing with errors from partial functions (#2514).

@pepeiborra
Copy link
Collaborator

Well, plugins certainly shouldn't be failing with errors from partial functions (#2514).

I think the key take away here is better error handling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: hls-eval-plugin component: plugins type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..
Projects
None yet
Development

No branches or pull requests

8 participants