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

LSP server triggers unhandled rejection when schema endpoint is unavailable #1711

Open
rchl opened this issue Nov 5, 2020 · 3 comments
Open
Labels
bug lsp-server graphql-language-service-server

Comments

@rchl
Copy link
Contributor

rchl commented Nov 5, 2020

With a config file like:

module.exports = {
    schema: 'http://localhost:5000/browse/api'
}

And the graphql server not actually running, the LSP server will try to connect to the endpoint and trigger promise rejection that is not unhandled:

internal/process/warning.js:40 (node:8940) UnhandledPromiseRejectionWarning: FetchError: request to http://localhost:5000/browse/api failed, reason: connect ECONNREFUSED 127.0.0.1:5000
    at ClientRequest.<anonymous> (/Users/me/github/graphiql/node_modules/node-fetch/lib/index.js:1455:11)
    at ClientRequest.emit (events.js:314:20)
    at Socket.socketErrorListener (_http_client.js:428:9)
    at Socket.emit (events.js:314:20)
    at emitErrorNT (internal/streams/destroy.js:92:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
    at processTicksAndRejections (internal/process/task_queues.js:84:21)

Unhandled rejections are bad as with Node 14 those will just crash the process.

@acao
Copy link
Member

acao commented Nov 14, 2020

thanks for the bug report! hmmm... which version of the LSP server are you using? are you using the CLI or the server directly or the vscode or vim extensions? seems this is coming from graphql-config since we don't invoke node-fetch ourselves. this error was being trapped before, not sure why it isn't now. Should probably send a client notification instead, so IDE's can push useful warnings about this?

the thing to note here is that without a valid schema, the server can't really do anything, as all language server features require the schema (except formatting which Prettier plugin provides, thus why we don't ship our own formatting handler), so it will need to exit when schema is not present no matter what.

although... i guess because I just implemented auto-reloading when changing the graphql config, it could instead disable all the language server features until the config is changed and a valid schema is present. is that the experience you are expecting?

@rchl
Copy link
Contributor Author

rchl commented Nov 14, 2020

Starting the server with:

node graphql-language-service-cli/bin/graphql.js server --method stream`

with those dependencies:

    "graphql": "^15.0.0",
    "graphql-language-service-cli": "^3.0.1"

(also tried the latest version from master)

It looks like the problem is indeed in graphql-config and looks like you can't catch that error in here even if you tried.

I suppose I should file an issue on graphql-config repo.

although... i guess because I just implemented auto-reloading when changing the graphql config, it could instead disable all the language server features until the config is changed and a valid schema is present. is that the experience you are expecting?

That sounds better to me then exiting the server as exiting would be hard to understand for the user.
To notify the user about the problem with loading schema, you could probably use window/showMessage. Or create diagnostic in opened gql file, I guess.

@acao
Copy link
Member

acao commented Nov 26, 2021

I was wrong, we can trap any exception we need to and handle this from the server and send a client notification as we should

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug lsp-server graphql-language-service-server
Projects
None yet
Development

No branches or pull requests

2 participants