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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/tidy-ghosts-happen.md
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
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
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
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
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
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
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);