Skip to content

Send unhandled exceptions to the user#2484

Merged
mergify[bot] merged 5 commits into
masterfrom
unhandled-exceptions
Dec 16, 2021
Merged

Send unhandled exceptions to the user#2484
mergify[bot] merged 5 commits into
masterfrom
unhandled-exceptions

Conversation

@pepeiborra

@pepeiborra pepeiborra commented Dec 13, 2021

Copy link
Copy Markdown
Collaborator

Closes #2390

Comment thread ghcide/src/Development/IDE/LSP/LanguageServer.hs Outdated
Comment thread ghcide/src/Development/IDE/LSP/LanguageServer.hs Outdated
Comment thread ghcide/src/Development/IDE/LSP/LanguageServer.hs
@pepeiborra pepeiborra added the merge me Label to trigger pull request merge label Dec 13, 2021
@jneira

jneira commented Dec 15, 2021

Copy link
Copy Markdown
Member

The test suite for tactics has failed twice several times for windows 8.10.7, with different test error cases, not sur if it will be transient

This cancelled job had 7 attempts and the tactics test suite failed in the 6 first ones

@Anton-Latukha

Anton-Latukha commented Dec 15, 2021

Copy link
Copy Markdown
Collaborator

Recent failed test workflows were:
pepeiborra - https://github.com/haskell/haskell-language-server/runs/4513326877?check_suite_focus=true
AntonLatukha - https://github.com/haskell/haskell-language-server/runs/4527885363?check_suite_focus=true (before that I & CI skipped my previous restart of a workflow, because right after it I decided not just restart, but also do a sync at the same time)
jneira - https://github.com/haskell/haskell-language-server/runs/4530467808?check_suite_focus=true

@pepeiborra

Copy link
Copy Markdown
Collaborator Author

The last failure is due to the old sqlite3 hiedb problem, not something specific to this change. We have a ticket for it somewhere, but I am on my phone right now and can't look.

We have a couple of sources of flakiness, and I would appreciate if someone would find the time to look into the hiedb one. Why is it getting triggered? Is the test suite running tests in parallel?

SQLite3 returned ErrorBusy while attempting to perform step: database is locked\n",

@pepeiborra

Copy link
Copy Markdown
Collaborator Author

Added code to stop the hiedb on shutdown. Might or might not help

@pepeiborra
pepeiborra requested a review from michaelpj December 16, 2021 10:50
Comment thread ghcide/src/Development/IDE/LSP/LanguageServer.hs
Comment thread ghcide/src/Development/IDE/LSP/LanguageServer.hs

-- | Runs the action until it ends or until the given MVar is put.
-- Rethrows any exceptions.
untilMVar :: MonadUnliftIO m => MVar () -> m () -> m ()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind of surprised these sorts of utilities aren't available elsewhere. I couldn't find them, anyway.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Especially the pattern of forking a thread with an MVar to control terminating it 🤔

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They may exist, I'm not sure either.

@michaelpj

Copy link
Copy Markdown
Collaborator

The windows jobs seemed to be hanging (stuck for 4h). I attempted to cancel them but it's unclear that it's worked.

@mergify
mergify Bot merged commit 0211f75 into master Dec 16, 2021
@michaelpj

Copy link
Copy Markdown
Collaborator

... how did that work?

@jneira

jneira commented Dec 16, 2021

Copy link
Copy Markdown
Member

... how did that work?

it should not 😕
the test post-jobs was supposed to be cancelled but it was not
https://github.com/haskell/haskell-language-server/runs/4549724494?check_suite_focus=true

@michaelpj

Copy link
Copy Markdown
Collaborator

I think this might be making the ghcide tests hang on windows. I think we should revert it until we can confirm.

@jneira

jneira commented Dec 17, 2021

Copy link
Copy Markdown
Member

I think this might be making the ghcide tests hang on windows. I think we should revert it until we can confirm.

well i have a pr fixing the cancelling thing and including this pr, if its tests are not stuck it would be fine

After reading your comment in #2493 it seems so
I am gonna revert it in my fork to check it, i have a pr in my fork with is also stuck: jneira#55 so the problem is reproduced there too

jneira added a commit to jneira/haskell-language-server that referenced this pull request Dec 17, 2021
@jneira

jneira commented Dec 17, 2021

Copy link
Copy Markdown
Member

pr in my fork with this reverted jneira#56

@jneira

jneira commented Dec 17, 2021

Copy link
Copy Markdown
Member

Ok the pr in my fork with this reverted has passed the ghcide tests: https://github.com/jneira/haskell-language-server/runs/4559275575?check_suite_focus=true
I am gonna open here

pepeiborra pushed a commit that referenced this pull request Dec 18, 2021
pepeiborra added a commit that referenced this pull request Dec 18, 2021
pepeiborra added a commit that referenced this pull request Dec 19, 2021
pepeiborra added a commit that referenced this pull request Dec 19, 2021
* Revert "Revert "Send unhandled exceptions to the user (#2484)" (#2497)"

This reverts commit 5d2189c.

* Log when reactor thread exits

* log shakeSessionInit

* Do not assume that the build has been initialized
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge me Label to trigger pull request merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Report error conditions via LSP message dialogs

4 participants