Skip to content
This repository was archived by the owner on Oct 7, 2020. It is now read-only.

Prevent hie crash if hlint crashes #1153

Merged
merged 1 commit into from
Apr 4, 2019

Conversation

fendor
Copy link
Collaborator

@fendor fendor commented Mar 27, 2019

Related to #1143

Also improves stability of hie, thus related to #1146
If hlint chokes on anything, dont kill hie.
An error message might be desirable but I dont know how to do that.

@fendor fendor requested review from lukel97 and alanz March 27, 2019 16:27
@fendor fendor force-pushed the hlint-prevent-crash branch from 4fe7097 to a0f56d0 Compare March 27, 2019 16:31
@mpickering
Copy link
Collaborator

There are already other functions which catch exceptions in HIE. Did you look at using one of those rather than rolling your own using try? In particular catchingSomeException seems like quite a broad brush.

@fendor
Copy link
Collaborator Author

fendor commented Mar 27, 2019

Indeed, catching SomeException is terrible.

Catch all IOExceptions that hlint might throw
@fendor fendor force-pushed the hlint-prevent-crash branch from a0f56d0 to 6751e45 Compare March 27, 2019 17:56
@fendor
Copy link
Collaborator Author

fendor commented Mar 27, 2019

Now we catch IOExceptions!

@samuelpilz
Copy link
Contributor

@mpickering I am actually in favor of aggressive error-catching from hlint. I think the hie language server should not be brought down by a internal error in any plugin like hlint.

It is the best thing for hie to do is to stop the plugin hlint and possibly restart it since hie is still quite useful if one plugin is removed.

@mpickering
Copy link
Collaborator

@power-fungus The problem is catching async exceptions such as interrupts.

@samuelpilz
Copy link
Contributor

Indeed, async exceptions should not be caught

@lukel97
Copy link
Collaborator

lukel97 commented Apr 3, 2019

I would argue that language servers should be a bit overdramatic and crash frequently: VS Code automatically restarts the server if it dies, and from what I recall with the ghc-mod plugin, the design is to propagate as many fatal errors as possible

@fendor
Copy link
Collaborator Author

fendor commented Apr 4, 2019

Hlint errors dont have to be fatal ones though. In that case, hlint would just not work, while other functionality is still working.

@Anrock
Copy link
Collaborator

Anrock commented Apr 4, 2019

I'm against being overdramatic, too. While VSCode may handle this well, LanguageClient-neovim stumbles on server crashes and i suspect other client may do this too.

As a side note: LSP should be general overall, so coding against behaviour of specific client isn't right thing to do - remember embrace&extend&extuinguish. There are plenty of VSCode-specific quirks in other clients/servers already.

@Avi-D-coder
Copy link
Collaborator

Avi-D-coder commented Apr 4, 2019

The problem with crashing frequently is many language clients, I have used don't restart servers. If the server can recover it should try to. If possible plugins should be restarted.
A message on stderr might also be a good idea. Does anyone know how many clients handle stderr?

@alanz
Copy link
Collaborator

alanz commented Apr 4, 2019

LGTM, now it is catching IOException.

@fendor fendor merged commit e6d249e into haskell:master Apr 4, 2019
@fendor fendor deleted the hlint-prevent-crash branch April 13, 2019 16:47
@alanz alanz added this to the 2019-04 milestone May 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants