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

don't crash lsp server on graphql config errors #2470

Merged
merged 1 commit into from
Jun 3, 2022
Merged

don't crash lsp server on graphql config errors #2470

merged 1 commit into from
Jun 3, 2022

Conversation

acao
Copy link
Member

@acao acao commented Jun 3, 2022

Aims to resolve #2421

  • graphql config errors only log to output channel, no longer crash the LSP
  • improve readability, add a helpful link for users
  • more performant LSP request no-ops for failing/missing config

this used to fail silently in the output channel, but vscode introduced a new retry and notification for this. in previous versions of vscode, the server exited non-0, but this didn't cause notifications for the user. this should introduce bugfixes and improvements for non-vscode users of the LSP server as well.

would like to provide more helpful graphql config DX in the future but this should be better for now.

if your LSP server cli implementation needs a non-zero exit for missing/invalid lsp config, let me know and we can add a feature to enable a non-zero failure on config errors for LSP and/or the CLI service wrapper

@changeset-bot
Copy link

changeset-bot bot commented Jun 3, 2022

🦋 Changeset detected

Latest commit: e89be6a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
graphql-language-service-server Patch
vscode-graphql Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@acao acao changed the title fix bug with lsp server crashing on graphql config errors don't crash lsp server on graphql config errors Jun 3, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2022

The latest changes of this PR are available as canary in npm (based on the declared changesets):

graphql-language-service-cli@3.2.25-canary-6116786d.0
graphql-language-service-server@2.7.24-canary-6116786d.0

@codecov
Copy link

codecov bot commented Jun 3, 2022

Codecov Report

Merging #2470 (e89be6a) into main (2d91916) will increase coverage by 3.05%.
The diff coverage is 24.53%.

@@            Coverage Diff             @@
##             main    #2470      +/-   ##
==========================================
+ Coverage   65.70%   68.76%   +3.05%     
==========================================
  Files          85       71      -14     
  Lines        5106     4223     -883     
  Branches     1631     1410     -221     
==========================================
- Hits         3355     2904     -451     
+ Misses       1747     1315     -432     
  Partials        4        4              
Impacted Files Coverage Δ
packages/codemirror-graphql/src/hint.ts 94.73% <ø> (ø)
packages/codemirror-graphql/src/lint.ts 100.00% <ø> (ø)
packages/codemirror-graphql/src/results/mode.ts 47.05% <ø> (ø)
...kages/codemirror-graphql/src/utils/forEachState.ts 100.00% <ø> (ø)
packages/codemirror-graphql/src/utils/hintList.ts 95.65% <ø> (ø)
...ckages/codemirror-graphql/src/utils/mode-indent.ts 0.00% <0.00%> (ø)
packages/codemirror-graphql/src/variables/hint.ts 89.70% <ø> (ø)
packages/codemirror-graphql/src/variables/mode.ts 79.48% <ø> (ø)
packages/graphiql-react/src/editor/whitespace.ts 100.00% <ø> (ø)
packages/graphiql-react/src/utility/debounce.ts 0.00% <0.00%> (ø)
... and 91 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8cda51f...e89be6a. Read the comment docs.

@acao acao added lsp-server graphql-language-service-server vscode-graphql labels Jun 3, 2022
Aims to resolve #2421

- graphql config errors only log to output channel, no longer crash the LSP
- more performant LSP request no-ops for failing/missing config

this used to fail silently in the output channel, but vscode introduced a new retry and notification for this

would like to provide more helpful graphql config DX in the future but this should be better for now
@acao acao merged commit d0017a9 into main Jun 3, 2022
@acao acao deleted the fix-lsp-crash branch June 3, 2022 14:40
@acao acao mentioned this pull request Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lsp-server graphql-language-service-server vscode-graphql
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VSCode extension crashing.
1 participant