fix: hover log #1481
fix: hover log #1481
Conversation
If there was an error in schema parsing, the hover handler silently fails. The aim of this PR is to raise the issue, the fix might be added at another level (and maybe even needed at other places). For this PR, we re-throw the error. Co-authored-by: huv1k <[email protected]>
|
@divyenduz we should wrap all the errors at the top, then! the schema error being trapped was there from before so I kept it, assumed there was a reason it would make so much sense to remove, but i noticed it during a refactor, so I wasn't able to remove it then. this is why i added a try/catch to startServer() but i removed it. let's remove the try/catch from hover, and bubble it up to where this function is called, so we can trap all the errors from LSP methods in one place. How is the behavior without a graphql config? Does this throw lots of errors? |
|
@divyenduz thanks for opening this and showing more initiative on this! this approach should work across the board: |
|
closing for #1482! confirmed it fixes this bug and many others. thanks @divyenduz ! |
If there was an error in schema parsing, the hover handler silently
fails.
The aim of this PR is to raise the issue, the fix might be added at
another level (and maybe even needed at other places).
For this PR, we re-throw the error.
After this PR, invalid schema yields:
P.S. the tests are much faster now! All of them took like 18s on my machine 🙌