Skip to content

Commit

Permalink
fix bug with lsp server crashing on graphql config errors
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@acao
acao committed Jun 3, 2022
1 parent 8cda51f commit d0017a9
Show file tree
Hide file tree
Showing 7 changed files with 160 additions and 50 deletions.
13 changes: 13 additions & 0 deletions .changeset/tidy-ghosts-happen.md
View file
Original file line number Diff line number Diff line change
@@ -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
3 changes: 2 additions & 1 deletion .vscode/settings.json
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{
"npm.packageManager": "yarn"
"npm.packageManager": "yarn",
"cSpell.words": ["graphqlrc"]
}
4 changes: 2 additions & 2 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
18 changes: 17 additions & 1 deletion packages/graphql-language-service-server/src/GraphQLCache.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -59,10 +60,12 @@ const {

export async function getGraphQLCache({
parser,
logger,
loadConfigOptions,
config,
}: {
parser: typeof parseDocument;
logger: Logger;
loadConfigOptions: LoadConfigOptions;
config?: GraphQLConfig;
}): Promise<GraphQLCache> {
Expand All @@ -74,6 +77,7 @@ export async function getGraphQLCache({
configDir: loadConfigOptions.rootDir as string,
config: graphQLConfig as GraphQLConfig,
parser,
logger,
});
}

Expand All @@ -86,15 +90,18 @@ export class GraphQLCache implements GraphQLCacheInterface {
_fragmentDefinitionsCache: Map<Uri, Map<string, FragmentInfo>>;
_typeDefinitionsCache: Map<Uri, Map<string, ObjectTypeInfo>>;
_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;
Expand All @@ -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 (
Expand Down
166 changes: 121 additions & 45 deletions packages/graphql-language-service-server/src/MessageProcessor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -89,6 +99,7 @@ export class MessageProcessor {
_languageService!: GraphQLLanguageService;
_textDocumentCache: Map<string, CachedDocumentType>;
_isInitialized: boolean;
_isGraphQLConfigMissing: boolean | null = null;
_willShutdown: boolean;
_logger: Logger;
_extensions?: GraphQLExtensionDeclaration[];
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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 };
Expand Down Expand Up @@ -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 }) => {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 }) => {
Expand Down Expand Up @@ -751,7 +824,7 @@ export class MessageProcessor {
async handleDocumentSymbolRequest(
params: DocumentSymbolParams,
): Promise<Array<SymbolInformation>> {
if (!this._isInitialized) {
if (!this._isInitialized || !this._graphQLCache) {
return [];
}

Expand Down Expand Up @@ -795,6 +868,9 @@ export class MessageProcessor {
async handleWorkspaceSymbolRequest(
params: WorkspaceSymbolParams,
): Promise<Array<SymbolInformation>> {
if (!this._isInitialized || !this._graphQLCache) {
return [];
}
// const config = await this._graphQLCache.getGraphQLConfig();
// await this._cacheAllProjectFiles(config);

Expand Down
2 changes: 1 addition & 1 deletion packages/graphql-language-service/src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export type {
export interface GraphQLCache {
getGraphQLConfig: () => GraphQLConfig;

getProjectForFile: (uri: string) => GraphQLProjectConfig;
getProjectForFile: (uri: string) => GraphQLProjectConfig | void;

getObjectTypeDependencies: (
query: string,
Expand Down
4 changes: 4 additions & 0 deletions scripts/prepublish.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

0 comments on commit d0017a9

Please sign in to comment.