Stop showing failed clojure-lsp requests in error popups

[bpringe]

From @ericdallo: https://clojurians.slack.com/archives/CBE668G4R/p1682006539680229?thread_ts=1681999380.506619&cid=CBE668G4R

I think internal errors from feature X in clojure-lsp should not show popups to user, it's a feature that shows only when user opens the tree window

I think this is being done by the vscode lsp client lib, but we can override it, I think. We did something similar here for a specific error request: https://github.com/BetterThanTomorrow/calva/pull/1904

[bpringe]

One option for a repro to test a solution is to use release 2023.04.19-12.43.29 of clojure-lsp and edit open/edit and edn file with a vector at the root that contains a map.

[skylize]

I would love to see a less spammy solution than the popup system Code implements by default.

However, I definitely do not like the idea of just indiscriminately swallowing any-and-all errors that LSP emits. If LSP has some kind of problem processing the document, and the Calva Team has has not specifically considered the situation(s) that cause some specific error, I want to be notified somehow.

Lacking any good suggestions to offer for a better system than the popups, I fully support the Calva Team carefully analyzing any new errors that arise, and, if appropriate, rapidly pushing updates to mute them asap, on a case-by-case basis.

---

I do have a suggestion to improve the user experience of any popups that still appear: rewrite the messages to include "Clojure LSP".

Not many users who should be expected to immediately recognize Request textDocument/whatever failed as LSP failing to parse something correctly in some particular context. So even though a message touch-up would not do anything to help with the spamminess, it would at least make the messages significantly less cryptic to the average user and offer a meaningful hint at the errors' scope.

Compare:

[Error - 4:10:17 PM] Request textDocument/documentSymbol failed.
  Message: Internal error
  Code: -32603 

[Error - 4:10:17 PM] Request textDocument/documentSymbol failed.
  Message: Clojure LSP failed to perform Code Action `textDocument/documentSymbol`.
  Code: -32603 

[bpringe]

Thanks for the input. I too am not totally sure about avoiding all popups. The messages can at least be seen if the server<->client logs are turned on, so they aren't totally swallowed - they just might not be seen immediately, if ever, by the user.

If we stopped popping up any errors, the process if clojure-lsp errors were related to something broken in Calva would be the users notices something's not working, they troubleshoot themselves or ask us for help, and we guide them to the logs.

@ericdallo What does lsp-mode regarding these errors? Does it show them in any obvious way to the user if they aren't looking at the client<->server logs? I don't think we definitely should do what lsp-mode does, but maybe we can get some inspiration from it.

Another thing to consider is these errors don't happen often, and when they do it's good to know, at least initially. I'm not sure if they provide enough context to ignore a certain class of errors, though (@ericdallo do they?). For example textDocument/documentSymbol isn't just about the edn tree feature that they're about in this recent case. So, making popups not happen for just textDocument/documentSymbol messages seems like a bad approach as we're categorically ignoring potentially many types of problems (assuming we still wanted popups for non edn-tree-specific issues).

@PEZ What are your thoughts here?

[PEZ]

I think we could consider suppressing all these pop-ups and log them in the dev console or in the Calva says output channel. Then if someone reports about troubles with clojure-lsp we can ask about any messages.

A lot of users do not know about clojure-lsp, or how it is something else than Calva. Which can make these messages a bit confusing.

[ericdallo]

Emacs prints the errors in its echo buffer but it's quite succinct and don't need user input to dismiss it.

My whole point here is that some LSP requests are done all the time, and if clojure-lsp has some corner case bug, it may print lots of popups even if user dismiss it, maybe have options to not require user input or have a way to "dismiss all popups from this error" would be nice.

[skylize]

I have the following in my keybindings.json. If there are any toasts visible, it hijacks Shift+Esc to clear them all, regardless if any toast still has focus.

  {
    "key": "shift+escape",
    "command": "notifications.clearAll",
    "when": "notificationToastsVisible || notificationCenterVisible"
  },

[PEZ]

Emacs prints the errors in its echo buffer but it's quite succinct and don't need user input to dismiss it.

Yes, something like that I think Calva should do too. Closest thing is probably to write in the Calva says output channel.

[bpringe]

Closest thing is probably to write in the Calva says output channel.

The errors are already printed in the client<->server logs channel. I don't think it's necessary to print them elsewhere too, but I guess it wouldn't hurt.

[PEZ]

I can't find a way to intercept the error dialog. I asked about it here:

• https://github.com/microsoft/vscode-discussions/discussions/579

[bpringe]

@PEZ I don't think this is happening with textDocument/publishDiagnostics. You mentioned that in the linked issue but this particular issue is about errors like Request textDocument/documentSymbol failed, which isn't related to textDocument/publishDiagnostics. I may have missed something, though.

[bpringe]

I tried adding the following middleware to the lsp client to see if we could catch the error before the popup is shown:

        async provideDocumentSymbols(document, token, next) {
          let result = undefined;
          try {
            result = await next(document, token);
          } catch (error) {
            console.log('error', error);
          }
          return result;
        },

I put a breakpoint in the catch block, but the popup is shown before the catch breakpoint is hit (and after next is called, as expected), so I'm not sure yet what we need to do to prevent those popups. We can probably ask on this repo, where the language client implementation lives. I'm done for today, though.

[bpringe]

We might be able to override vscode.window.showErrorMessage and filter out clojure-lsp errors.

[PEZ]

@PEZ I don't think this is happening with textDocument/publishDiagnostics.

You're quite right. I realized my error here yesterday when trying to figure out some way to intercept this popup.

@ericdallo , what is clojure-lsp sending that makes this error message show?

[lsolbach]

Seems like fixing the symtoms instead of fixing the root cause. An EDN file with a vector of maps is a valid file i the first place. So shouldn't this be fixed in LSP?

[PEZ]

Indeed. It's being fixed there. But Calva users need to be protected from the spam of this category of errors.

[ericdallo]

I released a new clojure-lsp version fixing all known edn tree issues, althoug I still think it's good to have this improvement on calva side for future regressions/errors as PEZ mentioned

[skylize]

Seems like fixing the symtoms instead of fixing the root cause. @lsolbach

We can almost certainly count on similar errors to pop up numerous times in the future. We should not just resign ourselves to getting spammed with messages continuously every time, while trying to get work done, until the problem can be resolved upstream.

[lsolbach]

Defensive programming is not a bad thing, of course. I really appreciate not to be bugged by the spam. ;-) I also really appreciate your work on making clojure programming such a nice experience.