Skip to content
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

Error popups when editing some invalid forms #1889

Open
skylize opened this issue Oct 10, 2022 · 6 comments
Open

Error popups when editing some invalid forms #1889

skylize opened this issue Oct 10, 2022 · 6 comments

Comments

@skylize
Copy link
Contributor

skylize commented Oct 10, 2022

All Calva's dependencies that use rewrite-clj, such as clojure-lsp, commonly propogate parsing exceptions. (I'm guessing Calva's direct use of rewrite-clj does the same?) Calva then fails to handle these exceptions or actively throws them at the user, causing a popup modal, which shows the opaque message of Request textDocument/codeAction failed., and does not auto-dismiss itself.

An example case that occurs frequently during normal (even if perfect) editing is caused by an unbalanced map anywhere between 2 parens.

;; The | represents the cursor.
;; Typing anything at all from this point (i.e. to type the first key)
;;  will cause and exception popup.
(foo-fn {|})

The user experience is severely degraded by these unavoidable popups that only go away with manual intervention. Calva should catch these exceptions and route them to a Diagnostic. They can be reported to the user as equivalent to a linting error.

One question here is where in the document the error should be reported. (Pretty sure we won't have meaningful line and column numbers from a failed parse?) The obvious choice would be either the start or end of the document. My personal opinion is it would be more helpful to use the start or end of whichever line the cursor happens to be on. This ensures the user receives visible feedback from anywhere in the document.

@skylize skylize changed the title Calva should route parsing exceptions into Diagnotic reporting Calva should not throw error popups because of rewrite-clj parsing errors Oct 10, 2022
@PEZ
Copy link
Collaborator

PEZ commented Oct 10, 2022

Thanks for the issue!

I can't reproduce the issue with your example there. Maybe there is something more in your file you are testing with?

@skylize
Copy link
Contributor Author

skylize commented Oct 11, 2022

I am getting intermittent results on that particular reduction of the problem as well. Typing into ({|}) will throw 100%.

With only ({}), we get a lint error but otherwise no problems:
[Trace - 10:22:54 AM] Sending notification 'textDocument/didChange'.
Params: {
    "textDocument": {
        "uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
        "version": 10
    },
    "contentChanges": [
        {
            "text": "({})"
        }
    ]
}

[Trace - 10:22:54 AM] Sending request 'textDocument/codeAction - (34)'.
Params: {
"textDocument": {
"uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj"
},
"range": {
"start": {
"line": 0,
"character": 2
},
"end": {
"line": 0,
"character": 2
}
},
"context": {
"diagnostics": [],
"triggerKind": 2
}
}

[Trace - 10:22:54 AM] Sending request 'textDocument/documentSymbol - (35)'.
Params: {
"textDocument": {
"uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj"
}
}

[Trace - 10:22:54 AM] Received response 'textDocument/documentSymbol - (35)' in 196ms.
Result: [
{
"name": "/home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
"kind": 21,
"range": {
"start": {
"line": 0,
"character": 0
},
"end": {
"line": 999999,
"character": 999999
}
},
"selectionRange": {
"start": {
"line": 0,
"character": 0
},
"end": {
"line": 999999,
"character": 999999
}
},
"children": []
}
]

[Trace - 10:22:54 AM] Received response 'textDocument/codeAction - (34)' in 294ms.
Result: [
{
"title": "Change coll to vector",
"kind": "refactor",
"command": {
"title": "Change coll",
"command": "change-coll",
"arguments": [
"file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
0,
2,
"vector"
]
}
},
{
"title": "Change coll to set",
"kind": "refactor",
"command": {
"title": "Change coll",
"command": "change-coll",
"arguments": [
"file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
0,
2,
"set"
]
}
},
{
"title": "Change coll to list",
"kind": "refactor",
"command": {
"title": "Change coll",
"command": "change-coll",
"arguments": [
"file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
0,
2,
"list"
]
}
},
{
"title": "Move to let",
"kind": "refactor.extract",
"command": {
"title": "Move to let",
"command": "move-to-let",
"arguments": [
"file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
0,
2,
"new-binding"
]
}
},
{
"title": "Extract to def",
"kind": "refactor.extract",
"command": {
"title": "Extract to def",
"command": "extract-to-def",
"arguments": [
"file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
0,
2,
null
]
}
},
{
"title": "Sort list",
"kind": "refactor.rewrite",
"command": {
"title": "Sort list",
"command": "sort-clauses",
"arguments": [
"file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
0,
2
]
}
},
{
"title": "Introduce let",
"kind": "refactor.extract",
"command": {
"title": "Introduce let",
"command": "introduce-let",
"arguments": [
"file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
0,
2,
"new-binding"
]
}
},
{
"title": "Clean namespace",
"kind": "source.organizeImports",
"command": {
"title": "Clean namespace",
"command": "clean-ns",
"arguments": [
"file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
0,
2
]
}
}
]

[Trace - 10:22:54 AM] Received notification 'textDocument/publishDiagnostics'.
Params: {
"uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
"diagnostics": [
{
"range": {
"start": {
"line": 0,
"character": 0
},
"end": {
"line": 0,
"character": 4
}
},
"tags": [],
"message": "map is called with 0 args but expects 1 or 2",
"code": "invalid-arity",
"severity": 1,
"source": "clj-kondo"
}
]
}

[Trace - 10:22:54 AM] Sending request 'textDocument/codeAction - (36)'.
Params: {
"textDocument": {
"uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj"
},
"range": {
"start": {
"line": 0,
"character": 2
},
"end": {
"line": 0,
"character": 2
}
},
"context": {
"diagnostics": [
{
"range": {
"start": {
"line": 0,
"character": 0
},
"end": {
"line": 0,
"character": 4
}
},
"message": "map is called with 0 args but expects 1 or 2",
"code": "invalid-arity",
"severity": 1,
"source": "clj-kondo"
}
],
"triggerKind": 2
}
}

[Trace - 10:22:54 AM] Received response 'textDocument/codeAction - (36)' in 3ms.
Result: [
{
"title": "Change coll to vector",
"kind": "refactor",
"command": {
"title": "Change coll",
"command": "change-coll",
"arguments": [
"file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
0,
2,
"vector"
]
}
},
{
"title": "Change coll to set",
"kind": "refactor",
"command": {
"title": "Change coll",
"command": "change-coll",
"arguments": [
"file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
0,
2,
"set"
]
}
},
{
"title": "Change coll to list",
"kind": "refactor",
"command": {
"title": "Change coll",
"command": "change-coll",
"arguments": [
"file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
0,
2,
"list"
]
}
},
{
"title": "Move to let",
"kind": "refactor.extract",
"command": {
"title": "Move to let",
"command": "move-to-let",
"arguments": [
"file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
0,
2,
"new-binding"
]
}
},
{
"title": "Extract to def",
"kind": "refactor.extract",
"command": {
"title": "Extract to def",
"command": "extract-to-def",
"arguments": [
"file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
0,
2,
null
]
}
},
{
"title": "Sort list",
"kind": "refactor.rewrite",
"command": {
"title": "Sort list",
"command": "sort-clauses",
"arguments": [
"file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
0,
2
]
}
},
{
"title": "Introduce let",
"kind": "refactor.extract",
"command": {
"title": "Introduce let",
"command": "introduce-let",
"arguments": [
"file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
0,
2,
"new-binding"
]
}
},
{
"title": "Suppress 'invalid-arity' diagnostic",
"kind": "quickfix",
"command": {
"title": "Suppress diagnostic",
"command": "suppress-diagnostic",
"arguments": [
"file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
0,
0,
"invalid-arity"
]
}
},
{
"title": "Clean namespace",
"kind": "source.organizeImports",
"command": {
"title": "Clean namespace",
"command": "clean-ns",
"arguments": [
"file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
0,
2
]
}
}
]

Typing a :, resulting in ({:}), triggers an exception. A codeAction request is sent, and clj-kondo sends an additional codeAction request for trying to call a map as a function with no arguments. Both requests come back as an "Internal error":
[Trace - 10:24:11 AM] Sending notification 'textDocument/didChange'.
Params: {
    "textDocument": {
        "uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
        "version": 11
    },
    "contentChanges": [
        {
            "text": "({:})"
        }
    ]
}

[Trace - 10:24:11 AM] Sending request 'textDocument/completion - (41)'.
Params: {
"textDocument": {
"uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj"
},
"position": {
"line": 0,
"character": 3
},
"context": {
"triggerKind": 1
}
}

[Trace - 10:24:11 AM] Received response 'textDocument/completion - (41)' in 3ms.
No result returned.

[Trace - 10:24:11 AM] Sending request 'textDocument/codeAction - (42)'.
Params: {
"textDocument": {
"uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj"
},
"range": {
"start": {
"line": 0,
"character": 3
},
"end": {
"line": 0,
"character": 3
}
},
"context": {
"diagnostics": [
{
"range": {
"start": {
"line": 0,
"character": 0
},
"end": {
"line": 0,
"character": 4
}
},
"message": "map is called with 0 args but expects 1 or 2",
"code": "invalid-arity",
"severity": 1,
"source": "clj-kondo"
}
],
"triggerKind": 2
}
}

[Trace - 10:24:11 AM] Received response 'textDocument/codeAction - (42)' in 83ms. Request failed: Internal error (-32603).
Error data: {
"id": 42,
"method": "textDocument/codeAction"
}

[Error - 10:24:11 AM] Request textDocument/codeAction failed.
Message: Internal error
Code: -32603
[object Object]
[Trace - 10:24:11 AM] Sending request 'textDocument/documentSymbol - (43)'.
Params: {
"textDocument": {
"uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj"
}
}

[Trace - 10:24:11 AM] Received response 'textDocument/documentSymbol - (43)' in 2ms.
Result: [
{
"name": "/home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
"kind": 21,
"range": {
"start": {
"line": 0,
"character": 0
},
"end": {
"line": 999999,
"character": 999999
}
},
"selectionRange": {
"start": {
"line": 0,
"character": 0
},
"end": {
"line": 999999,
"character": 999999
}
},
"children": []
}
]

[Trace - 10:24:11 AM] Received notification 'textDocument/publishDiagnostics'.
Params: {
"uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
"diagnostics": [
{
"range": {
"start": {
"line": 0,
"character": 0
},
"end": {
"line": 0,
"character": 5
}
},
"tags": [],
"message": "map is called with 0 args but expects 1 or 2",
"code": "invalid-arity",
"severity": 1,
"source": "clj-kondo"
},
{
"range": {
"start": {
"line": 0,
"character": 2
},
"end": {
"line": 0,
"character": 2
}
},
"tags": [],
"message": "Invalid keyword: .",
"code": "syntax",
"severity": 1,
"source": "clj-kondo"
}
]
}

[Trace - 10:24:12 AM] Sending request 'textDocument/codeAction - (44)'.
Params: {
"textDocument": {
"uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj"
},
"range": {
"start": {
"line": 0,
"character": 3
},
"end": {
"line": 0,
"character": 3
}
},
"context": {
"diagnostics": [
{
"range": {
"start": {
"line": 0,
"character": 0
},
"end": {
"line": 0,
"character": 5
}
},
"message": "map is called with 0 args but expects 1 or 2",
"code": "invalid-arity",
"severity": 1,
"source": "clj-kondo"
}
],
"triggerKind": 2
}
}

[Trace - 10:24:12 AM] Received response 'textDocument/codeAction - (44)' in 3ms. Request failed: Internal error (-32603).
Error data: {
"id": 44,
"method": "textDocument/codeAction"
}

[Error - 10:24:12 AM] Request textDocument/codeAction failed.
Message: Internal error
Code: -32603
[object Object]

If an argument was provided ahead of time, as in ({:} :foo), then clj-kondo is no longer in play. But we still reliably get an exception from a codeAction request:
[Trace - 10:25:41 AM] Sending notification 'textDocument/didChange'.
Params: {
    "textDocument": {
        "uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
        "version": 22
    },
    "contentChanges": [
        {
            "text": "({:} :foo)"
        }
    ]
}

[Trace - 10:25:41 AM] Sending request 'textDocument/completion - (77)'.
Params: {
"textDocument": {
"uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj"
},
"position": {
"line": 0,
"character": 3
},
"context": {
"triggerKind": 1
}
}

[Trace - 10:25:41 AM] Received response 'textDocument/completion - (77)' in 6ms.
Result: [
{
"label": ":foo",
"kind": 14,
"detail": "",
"data": {
"unresolved": [
[
"documentation",
{
"name": "foo",
"uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
"name-row": 1,
"name-col": 5
}
]
]
}
}
]

[Trace - 10:25:41 AM] Sending request 'completionItem/resolve - (78)'.
Params: {
"label": ":foo",
"insertTextFormat": 1,
"kind": 14,
"data": {
"unresolved": [
[
"documentation",
{
"name": "foo",
"uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
"name-row": 1,
"name-col": 5
}
]
]
}
}

[Trace - 10:25:41 AM] Received response 'completionItem/resolve - (78)' in 4ms.
Result: {
"label": ":foo",
"insertTextFormat": 1,
"kind": 14
}

[Trace - 10:25:41 AM] Sending request 'textDocument/codeAction - (79)'.
Params: {
"textDocument": {
"uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj"
},
"range": {
"start": {
"line": 0,
"character": 3
},
"end": {
"line": 0,
"character": 3
}
},
"context": {
"diagnostics": [],
"triggerKind": 2
}
}

[Trace - 10:25:41 AM] Received response 'textDocument/codeAction - (79)' in 83ms. Request failed: Internal error (-32603).
Error data: {
"id": 79,
"method": "textDocument/codeAction"
}

[Error - 10:25:41 AM] Request textDocument/codeAction failed.
Message: Internal error
Code: -32603
[object Object]
[Trace - 10:25:41 AM] Sending request 'textDocument/documentSymbol - (80)'.
Params: {
"textDocument": {
"uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj"
}
}

[Trace - 10:25:41 AM] Received response 'textDocument/documentSymbol - (80)' in 2ms.
Result: [
{
"name": "/home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
"kind": 21,
"range": {
"start": {
"line": 0,
"character": 0
},
"end": {
"line": 999999,
"character": 999999
}
},
"selectionRange": {
"start": {
"line": 0,
"character": 0
},
"end": {
"line": 999999,
"character": 999999
}
},
"children": []
}
]

[Trace - 10:25:41 AM] Received notification 'textDocument/publishDiagnostics'.
Params: {
"uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj",
"diagnostics": [
{
"range": {
"start": {
"line": 0,
"character": 2
},
"end": {
"line": 0,
"character": 2
}
},
"tags": [],
"message": "Invalid keyword: .",
"code": "syntax",
"severity": 1,
"source": "clj-kondo"
}
]
}

[Trace - 10:25:41 AM] Sending request 'textDocument/codeAction - (81)'.
Params: {
"textDocument": {
"uri": "file:///home/sky/edu/clojure/lsp-bug/lsp_bug.clj"
},
"range": {
"start": {
"line": 0,
"character": 3
},
"end": {
"line": 0,
"character": 3
}
},
"context": {
"diagnostics": [],
"triggerKind": 2
}
}

[Trace - 10:25:41 AM] Received response 'textDocument/codeAction - (81)' in 4ms. Request failed: Internal error (-32603).
Error data: {
"id": 81,
"method": "textDocument/codeAction"
}

[Error - 10:25:41 AM] Request textDocument/codeAction failed.
Message: Internal error
Code: -32603
[object Object]

Not totally clear to me what factors come into play for more extensive examples. I just know I get popups over and over and over, at times when the only problem with my code is that I just started typing the keyname for a new map entry, or otherwise caused some map to be temporarily out of balance.

@efraimmgon
Copy link

Also copying the form that throws the error when you're typing to a new file means it no longer throws this error when typing.

Haven't been able to figure out what's the issue too.

@skylize
Copy link
Contributor Author

skylize commented Oct 14, 2022

It seems Calva never has its hands directly on the event flow that leads to this bug. We tell clojure-lsp to handle codeActions when initializing the server, and then Code sends codeAction events directly to clojure-lsp.

As things stands now, we are probably stuck waiting for resolution to clojure-lsp #1268.

@PEZ PEZ changed the title Calva should not throw error popups because of rewrite-clj parsing errors Error popups when editing some invalid forms Oct 14, 2022
@skylize
Copy link
Contributor Author

skylize commented Oct 18, 2022

Here is a partial workaround until this can be solved:

Adding this to your keybindings.json provides a Shift+Esc shortcut to dismiss all notifications, whenever any are visible, regardless of focus.

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

bpringe added a commit that referenced this issue Oct 19, 2022
@bpringe
Copy link
Member

bpringe commented Oct 19, 2022

Once the above PR is merged and deployed, the workaround above won't be needed. 🎉

skylize pushed a commit to skylize/calva that referenced this issue Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants