From d0017a93b818cf3119e51c2b6c4a19004f98e29b Mon Sep 17 00:00:00 2001 From: Rikki Schulte Date: Fri, 3 Jun 2022 15:10:54 +0200 Subject: [PATCH] fix bug with lsp server crashing on graphql config errors 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 --- .changeset/tidy-ghosts-happen.md | 13 ++ .vscode/settings.json | 3 +- package.json | 4 +- .../src/GraphQLCache.ts | 18 +- .../src/MessageProcessor.ts | 166 +++++++++++++----- .../graphql-language-service/src/types.ts | 2 +- scripts/prepublish.sh | 4 + 7 files changed, 160 insertions(+), 50 deletions(-) create mode 100644 .changeset/tidy-ghosts-happen.md diff --git a/.changeset/tidy-ghosts-happen.md b/.changeset/tidy-ghosts-happen.md new file mode 100644 index 00000000000..1b476abc7af --- /dev/null +++ b/.changeset/tidy-ghosts-happen.md @@ -0,0 +1,13 @@ +--- +'graphql-language-service-server': patch +'vscode-graphql': patch +--- + +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 diff --git a/.vscode/settings.json b/.vscode/settings.json index ad94bc76410..1b034a08ffd 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,3 +1,4 @@ { - "npm.packageManager": "yarn" + "npm.packageManager": "yarn", + "cSpell.words": ["graphqlrc"] } diff --git a/package.json b/package.json index cf70fe12748..62db0bd7c85 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ }, "scripts": { "build": "yarn build:clean && yarn build:packages && yarn build:graphiql-react && yarn build:graphiql", - "build-bundles": "yarn prebuild-bundles && yarn workspace graphiql run build-bundles", + "build-bundles": "yarn prebuild-bundles && wsrun -p -m -s build-bundles", "build-bundles-clean": "rimraf '{packages,examples,plugins}/**/{bundle,cdn,webpack}' && yarn workspace graphiql run build-bundles-clean", "build-clean": "wsrun -m build-clean ", "build-demo": "wsrun -m -s build-demo", @@ -60,7 +60,7 @@ "prepublishOnly": "./scripts/prepublish.sh", "pretty": "node scripts/pretty.js", "pretty-check": "node scripts/pretty.js --check", - "release": "yarn build && yarn build-bundles && (wsrun -m -s release --changedSince main || echo Some releases failed) && yarn changeset publish", + "release": "yarn build && yarn build-bundles && yarn changeset publish", "release:canary": "(node scripts/canary-release.js && yarn build && yarn build-bundles && yarn changeset publish --tag canary) || echo Skipping Canary...", "repo:lint": "manypkg check", "repo:fix": "manypkg fix", diff --git a/packages/graphql-language-service-server/src/GraphQLCache.ts b/packages/graphql-language-service-server/src/GraphQLCache.ts index e44ce4acd6c..6d65d9df375 100644 --- a/packages/graphql-language-service-server/src/GraphQLCache.ts +++ b/packages/graphql-language-service-server/src/GraphQLCache.ts @@ -35,6 +35,7 @@ import stringToHash from './stringToHash'; import glob from 'glob'; import { LoadConfigOptions } from './types'; import { URI } from 'vscode-uri'; +import { Logger } from './Logger'; // Maximum files to read when processing GraphQL files. const MAX_READS = 200; @@ -59,10 +60,12 @@ const { export async function getGraphQLCache({ parser, + logger, loadConfigOptions, config, }: { parser: typeof parseDocument; + logger: Logger; loadConfigOptions: LoadConfigOptions; config?: GraphQLConfig; }): Promise { @@ -74,6 +77,7 @@ export async function getGraphQLCache({ configDir: loadConfigOptions.rootDir as string, config: graphQLConfig as GraphQLConfig, parser, + logger, }); } @@ -86,15 +90,18 @@ export class GraphQLCache implements GraphQLCacheInterface { _fragmentDefinitionsCache: Map>; _typeDefinitionsCache: Map>; _parser: typeof parseDocument; + _logger: Logger; constructor({ configDir, config, parser, + logger, }: { configDir: Uri; config: GraphQLConfig; parser: typeof parseDocument; + logger: Logger; }) { this._configDir = configDir; this._graphQLConfig = config; @@ -104,12 +111,21 @@ export class GraphQLCache implements GraphQLCacheInterface { this._typeDefinitionsCache = new Map(); this._typeExtensionMap = new Map(); this._parser = parser; + this._logger = logger; } getGraphQLConfig = (): GraphQLConfig => this._graphQLConfig; getProjectForFile = (uri: string): GraphQLProjectConfig => { - return this._graphQLConfig.getProjectForFile(URI.parse(uri).fsPath); + try { + return this._graphQLConfig.getProjectForFile(URI.parse(uri).fsPath); + } catch (err) { + this._logger.error( + `there was an error loading the project config for this file ${err}`, + ); + // @ts-expect-error + return null; + } }; getFragmentDependencies = async ( diff --git a/packages/graphql-language-service-server/src/MessageProcessor.ts b/packages/graphql-language-service-server/src/MessageProcessor.ts index dd0a4ee9128..8188d81afc1 100644 --- a/packages/graphql-language-service-server/src/MessageProcessor.ts +++ b/packages/graphql-language-service-server/src/MessageProcessor.ts @@ -65,12 +65,22 @@ import { parseDocument, DEFAULT_SUPPORTED_EXTENSIONS } from './parseDocument'; import { Logger } from './Logger'; import { printSchema, visit, parse, FragmentDefinitionNode } from 'graphql'; import { tmpdir } from 'os'; -import { GraphQLExtensionDeclaration } from 'graphql-config'; +import { + ConfigEmptyError, + ConfigInvalidError, + ConfigNotFoundError, + GraphQLExtensionDeclaration, + LoaderNoResultError, + ProjectNotFoundError, +} from 'graphql-config'; import type { LoadConfigOptions } from './types'; import { promisify } from 'util'; const writeFileAsync = promisify(writeFile); +const configDocLink = + 'https://www.npmjs.com/package/graphql-language-service-server#user-content-graphql-configuration-file'; + // import dotenv config as early as possible for graphql-config cjs pattern require('dotenv').config(); @@ -89,6 +99,7 @@ export class MessageProcessor { _languageService!: GraphQLLanguageService; _textDocumentCache: Map; _isInitialized: boolean; + _isGraphQLConfigMissing: boolean | null = null; _willShutdown: boolean; _logger: Logger; _extensions?: GraphQLExtensionDeclaration[]; @@ -226,17 +237,70 @@ export class MessageProcessor { }, this._settings.load ?? {}), rootDir, }; - // reload the graphql cache - this._graphQLCache = await getGraphQLCache({ - parser: this._parser, - loadConfigOptions: this._loadConfigOptions, - }); - this._languageService = new GraphQLLanguageService(this._graphQLCache); - if (this._graphQLCache?.getGraphQLConfig) { - const config = this._graphQLCache.getGraphQLConfig(); - await this._cacheAllProjectFiles(config); + try { + // reload the graphql cache + this._graphQLCache = await getGraphQLCache({ + parser: this._parser, + loadConfigOptions: this._loadConfigOptions, + + logger: this._logger, + }); + this._languageService = new GraphQLLanguageService(this._graphQLCache); + if (this._graphQLConfig || this._graphQLCache?.getGraphQLConfig) { + const config = + this._graphQLConfig ?? this._graphQLCache.getGraphQLConfig(); + await this._cacheAllProjectFiles(config); + } + this._isInitialized = true; + } catch (err) { + this._handleConfigError({ err }); } } + _handleConfigError({ err }: { err: unknown; uri?: string }) { + if (err instanceof ConfigNotFoundError) { + // TODO: obviously this needs to become a map by workspace from uri + // for workspaces support + this._isGraphQLConfigMissing = true; + + this._logConfigError(err.message); + return; + } else if (err instanceof ProjectNotFoundError) { + this._logConfigError( + 'Project not found for this file - make sure that a schema is present', + ); + } else if (err instanceof ConfigInvalidError) { + this._logConfigError(`Invalid configuration\n${err.message}`); + } else if (err instanceof ConfigEmptyError) { + this._logConfigError(err.message); + } else if (err instanceof LoaderNoResultError) { + this._logConfigError(err.message); + } else { + this._logConfigError( + // @ts-expect-error + err?.message ?? err?.toString(), + ); + } + + // force a no-op for all other language feature requests. + // + // TODO: contextually disable language features based on whether config is present + // Ideally could do this when sending initialization message, but extension settings are not available + // then, which are needed in order to initialize the language server (graphql-config loadConfig settings, for example) + this._isInitialized = false; + + // set this to false here so that if we don't have a missing config file issue anymore + // we can keep re-trying to load the config, so that on the next add or save event, + // it can keep retrying the language service + this._isGraphQLConfigMissing = false; + } + + _logConfigError(errorMessage: string) { + this._logger.error( + `WARNING: graphql-config error, only highlighting is enabled:\n` + + errorMessage + + `\nfor more information on using 'graphql-config' with 'graphql-language-service-server', \nsee the documentation at ${configDocLink}`, + ); + } async handleDidOpenOrSaveNotification( params: DidSaveTextDocumentParams | DidOpenTextDocumentParams, @@ -247,16 +311,17 @@ export class MessageProcessor { */ try { if (!this._isInitialized || !this._graphQLCache) { - if (!this._settings) { + // don't try to initialize again if we've already tried + // and the graphql config file or package.json entry isn't even there + if (this._isGraphQLConfigMissing !== true) { // then initial call to update graphql config await this._updateGraphQLConfig(); - this._isInitialized = true; } else { return null; } } } catch (err) { - this._logger.warn(err); + this._logger.error(err); } // Here, we set the workspace settings in memory, @@ -301,36 +366,43 @@ export class MessageProcessor { contents = cachedDocument.contents; } } - const project = this._graphQLCache.getProjectForFile(uri); - - if ( - this._isInitialized && - this._graphQLCache && - project.extensions?.languageService?.enableValidation !== false - ) { - await Promise.all( - contents.map(async ({ query, range }) => { - const results = await this._languageService.getDiagnostics( - query, - uri, - this._isRelayCompatMode(query), + if (!this._graphQLCache) { + return { uri, diagnostics }; + } else { + try { + const project = this._graphQLCache.getProjectForFile(uri); + if ( + this._isInitialized && + this._graphQLCache && + project?.extensions?.languageService?.enableValidation !== false + ) { + await Promise.all( + contents.map(async ({ query, range }) => { + const results = await this._languageService.getDiagnostics( + query, + uri, + this._isRelayCompatMode(query), + ); + if (results && results.length > 0) { + diagnostics.push( + ...processDiagnosticsMessage(results, query, range), + ); + } + }), ); - if (results && results.length > 0) { - diagnostics.push( - ...processDiagnosticsMessage(results, query, range), - ); - } - }), - ); + } - this._logger.log( - JSON.stringify({ - type: 'usage', - messageType: 'textDocument/didOpen', - projectName: project && project.name, - fileName: uri, - }), - ); + this._logger.log( + JSON.stringify({ + type: 'usage', + messageType: 'textDocument/didOpenOrSave', + projectName: project?.name, + fileName: uri, + }), + ); + } catch (err) { + this._handleConfigError({ err, uri }); + } } return { uri, diagnostics }; @@ -383,7 +455,7 @@ export class MessageProcessor { const project = this._graphQLCache.getProjectForFile(uri); const diagnostics: Diagnostic[] = []; - if (project.extensions?.languageService?.enableValidation !== false) { + if (project?.extensions?.languageService?.enableValidation !== false) { // Send the diagnostics onChange as well await Promise.all( contents.map(async ({ query, range }) => { @@ -441,7 +513,6 @@ export class MessageProcessor { if (this._textDocumentCache.has(uri)) { this._textDocumentCache.delete(uri); } - const project = this._graphQLCache.getProjectForFile(uri); this._logger.log( @@ -605,7 +676,9 @@ export class MessageProcessor { const project = this._graphQLCache.getProjectForFile(uri); let diagnostics: Diagnostic[] = []; - if (project.extensions?.languageService?.enableValidation !== false) { + if ( + project?.extensions?.languageService?.enableValidation !== false + ) { diagnostics = ( await Promise.all( contents.map(async ({ query, range }) => { @@ -751,7 +824,7 @@ export class MessageProcessor { async handleDocumentSymbolRequest( params: DocumentSymbolParams, ): Promise> { - if (!this._isInitialized) { + if (!this._isInitialized || !this._graphQLCache) { return []; } @@ -795,6 +868,9 @@ export class MessageProcessor { async handleWorkspaceSymbolRequest( params: WorkspaceSymbolParams, ): Promise> { + if (!this._isInitialized || !this._graphQLCache) { + return []; + } // const config = await this._graphQLCache.getGraphQLConfig(); // await this._cacheAllProjectFiles(config); diff --git a/packages/graphql-language-service/src/types.ts b/packages/graphql-language-service/src/types.ts index 4bb4b2f2feb..f1ad45745e3 100644 --- a/packages/graphql-language-service/src/types.ts +++ b/packages/graphql-language-service/src/types.ts @@ -49,7 +49,7 @@ export type { export interface GraphQLCache { getGraphQLConfig: () => GraphQLConfig; - getProjectForFile: (uri: string) => GraphQLProjectConfig; + getProjectForFile: (uri: string) => GraphQLProjectConfig | void; getObjectTypeDependencies: ( query: string, diff --git a/scripts/prepublish.sh b/scripts/prepublish.sh index 0fb989e027c..7e135242fb3 100755 --- a/scripts/prepublish.sh +++ b/scripts/prepublish.sh @@ -17,3 +17,7 @@ if [ "$CI" != true ]; then fi; yarn lint && yarn build && yarn build-bundles && yarn test && yarn e2e; + +# attempt a release against vscode-graphql and any workspace that specifies 'release' +# if semantic release incremented the version it will publish +(wsrun -m -s release --changedSince main || true);