From 06007498880528ed75dd4d705dcbcd7c9e775939 Mon Sep 17 00:00:00 2001 From: Mark Skelton Date: Tue, 9 May 2023 02:19:16 -0500 Subject: [PATCH] feat: Use native LSP logger (#3148) --- .changeset/stale-experts-explode.md | 7 ++ .../graphql-language-service-cli/src/cli.ts | 7 +- .../src/GraphQLCache.ts | 2 +- .../src/GraphQLLanguageService.ts | 3 +- .../src/Logger.ts | 85 +++---------------- .../src/MessageProcessor.ts | 2 +- .../__tests__/GraphQLLanguageService-test.ts | 4 +- .../src/__tests__/Logger-test.ts | 64 -------------- .../src/__tests__/MessageProcessor-test.ts | 5 +- .../src/__tests__/findGraphQLTags-test.ts | 26 ++---- .../src/findGraphQLTags.ts | 2 +- .../src/index.ts | 2 - .../src/parseDocument.ts | 5 +- .../src/startServer.ts | 29 ++----- 14 files changed, 47 insertions(+), 196 deletions(-) create mode 100644 .changeset/stale-experts-explode.md delete mode 100644 packages/graphql-language-service-server/src/__tests__/Logger-test.ts diff --git a/.changeset/stale-experts-explode.md b/.changeset/stale-experts-explode.md new file mode 100644 index 00000000000..7956a40b1ca --- /dev/null +++ b/.changeset/stale-experts-explode.md @@ -0,0 +1,7 @@ +--- +"graphql-language-service-cli": patch +"graphql-language-service-server": minor +"graphql-language-service": patch +--- + +Use native LSP logger instead of manual file based logging. This fixes errors in Neovim when using the GraphQL LSP. diff --git a/packages/graphql-language-service-cli/src/cli.ts b/packages/graphql-language-service-cli/src/cli.ts index 75647184aa7..ee3f84741bc 100644 --- a/packages/graphql-language-service-cli/src/cli.ts +++ b/packages/graphql-language-service-cli/src/cli.ts @@ -10,7 +10,7 @@ import yargs from 'yargs'; import client from './client'; -import { Logger, startServer } from 'graphql-language-service-server'; +import { startServer } from 'graphql-language-service-server'; const { argv } = yargs .usage( @@ -128,8 +128,9 @@ if (command === 'server') { } // eslint-disable-next-line promise/prefer-await-to-then -- don't know if I can use top level await here startServer(options).catch(error => { - const logger = new Logger(); - logger.error(String(error)); + process.stderr.write( + 'An error was thrown from GraphQL language service: ' + String(error), + ); }); } else { client(command as string, argv as Record); diff --git a/packages/graphql-language-service-server/src/GraphQLCache.ts b/packages/graphql-language-service-server/src/GraphQLCache.ts index 07166687740..3541176eea6 100644 --- a/packages/graphql-language-service-server/src/GraphQLCache.ts +++ b/packages/graphql-language-service-server/src/GraphQLCache.ts @@ -22,6 +22,7 @@ import type { ObjectTypeInfo, Uri, } from 'graphql-language-service'; +import type { Logger } from 'vscode-languageserver'; import * as fs from 'node:fs'; import { readFile } from 'node:fs/promises'; @@ -41,7 +42,6 @@ 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; diff --git a/packages/graphql-language-service-server/src/GraphQLLanguageService.ts b/packages/graphql-language-service-server/src/GraphQLLanguageService.ts index a86b149f0ef..e48b3fe8919 100644 --- a/packages/graphql-language-service-server/src/GraphQLLanguageService.ts +++ b/packages/graphql-language-service-server/src/GraphQLLanguageService.ts @@ -49,14 +49,13 @@ import { import { GraphQLConfig, GraphQLProjectConfig } from 'graphql-config'; +import type { Logger } from 'vscode-languageserver'; import { Hover, SymbolInformation, SymbolKind, } from 'vscode-languageserver-types'; -import { Logger } from './Logger'; - const KIND_TO_SYMBOL_KIND: { [key: string]: SymbolKind } = { [Kind.FIELD]: SymbolKind.Field, [Kind.OPERATION_DEFINITION]: SymbolKind.Class, diff --git a/packages/graphql-language-service-server/src/Logger.ts b/packages/graphql-language-service-server/src/Logger.ts index 6f1506c7991..ccc58defa81 100644 --- a/packages/graphql-language-service-server/src/Logger.ts +++ b/packages/graphql-language-service-server/src/Logger.ts @@ -8,94 +8,31 @@ */ import { Logger as VSCodeLogger } from 'vscode-jsonrpc'; -import { DiagnosticSeverity } from 'vscode-languageserver'; - -import * as fs from 'node:fs'; -import * as os from 'node:os'; -import { join } from 'node:path'; -import { Socket } from 'node:net'; - -import { - DIAGNOSTIC_SEVERITY, - SeverityEnum, - SEVERITY, -} from 'graphql-language-service'; +import { Connection } from 'vscode-languageserver'; export class Logger implements VSCodeLogger { - _logFilePath: string; - _stderrOnly: boolean; - - constructor(tmpDir?: string, stderrOnly?: boolean) { - const dir = join(tmpDir || os.tmpdir(), 'graphql-language-service-logs'); - try { - if (!fs.existsSync(dir)) { - fs.mkdirSync(dir); - } - } catch { - // intentionally no-op. Don't block the language server even if - // the necessary setup cannot be completed for logger. - } - - this._logFilePath = join( - dir, - `graphql-language-service-log-${ - os.userInfo().username - }-${getDateString()}.log`, - ); - - this._stderrOnly = stderrOnly || false; - } + constructor(private _connection: Connection) {} error(message: string): void { - this._log(message, SEVERITY.Error); + this._connection.console.error(message); } warn(message: string): void { - this._log(message, SEVERITY.Warning); + this._connection.console.warn(message); } info(message: string): void { - this._log(message, SEVERITY.Information); + this._connection.console.info(message); } log(message: string): void { - this._log(message, SEVERITY.Hint); - } - - _log(message: string, severityKey: SeverityEnum): void { - const timestamp = new Date().toLocaleString(); - const severity = DIAGNOSTIC_SEVERITY[severityKey]; - const { pid } = process; - - const stringMessage = String(message).trim(); - const logMessage = `${timestamp} [${severity}] (pid: ${pid}) graphql-language-service-usage-logs: ${stringMessage}\n`; - // write to the file in tmpdir - fs.appendFile(this._logFilePath, logMessage, _error => {}); - // @TODO: enable with debugging - if (severityKey !== SEVERITY.Hint) { - this._getOutputStream(severity).write(logMessage, err => { - if (err) { - // eslint-disable-next-line no-console - console.error(err); - } - }); - } - } - - _getOutputStream(severity: DiagnosticSeverity): Socket { - if (this._stderrOnly || severity === DIAGNOSTIC_SEVERITY.Error) { - return process.stderr; - } - - return process.stdout; + this._connection.console.log(message); } } -// function getUnixTime() { -// return new Date().getTime() / 1000; -// } - -function getDateString() { - const date = new Date(); - return `${date.getFullYear()}-${date.getMonth() + 1}-${date.getDate()}`; +export class NoopLogger implements VSCodeLogger { + error() {} + warn() {} + info() {} + log() {} } diff --git a/packages/graphql-language-service-server/src/MessageProcessor.ts b/packages/graphql-language-service-server/src/MessageProcessor.ts index 78ae60af40a..c44eb28efce 100644 --- a/packages/graphql-language-service-server/src/MessageProcessor.ts +++ b/packages/graphql-language-service-server/src/MessageProcessor.ts @@ -53,6 +53,7 @@ import type { WorkspaceSymbolParams, Connection, DidChangeConfigurationRegistrationOptions, + Logger, } from 'vscode-languageserver/node'; import type { UnnormalizedTypeDefPointer } from '@graphql-tools/load'; @@ -60,7 +61,6 @@ import type { UnnormalizedTypeDefPointer } from '@graphql-tools/load'; import { getGraphQLCache, GraphQLCache } from './GraphQLCache'; import { parseDocument, DEFAULT_SUPPORTED_EXTENSIONS } from './parseDocument'; -import { Logger } from './Logger'; import { printSchema, visit, parse, FragmentDefinitionNode } from 'graphql'; import { tmpdir } from 'node:os'; import { diff --git a/packages/graphql-language-service-server/src/__tests__/GraphQLLanguageService-test.ts b/packages/graphql-language-service-server/src/__tests__/GraphQLLanguageService-test.ts index eeaeba09bb2..0283c277174 100644 --- a/packages/graphql-language-service-server/src/__tests__/GraphQLLanguageService-test.ts +++ b/packages/graphql-language-service-server/src/__tests__/GraphQLLanguageService-test.ts @@ -13,7 +13,7 @@ import { GraphQLConfig } from 'graphql-config'; import { GraphQLLanguageService } from '../GraphQLLanguageService'; import { SymbolKind } from 'vscode-languageserver-protocol'; import { Position } from 'graphql-language-service'; -import { Logger } from '../Logger'; +import { NoopLogger } from '../Logger'; const MOCK_CONFIG = { filepath: join(__dirname, '.graphqlrc.yml'), @@ -122,7 +122,7 @@ describe('GraphQLLanguageService', () => { beforeEach(() => { languageService = new GraphQLLanguageService( mockCache as any, - new Logger(), + new NoopLogger(), ); }); diff --git a/packages/graphql-language-service-server/src/__tests__/Logger-test.ts b/packages/graphql-language-service-server/src/__tests__/Logger-test.ts deleted file mode 100644 index 3cfa3a89ee6..00000000000 --- a/packages/graphql-language-service-server/src/__tests__/Logger-test.ts +++ /dev/null @@ -1,64 +0,0 @@ -/** - * Copyright (c) 2021 GraphQL Contributors - * All rights reserved. - * - * This source code is licensed under the license found in the - * LICENSE file in the root directory of this source tree. - * - */ - -import { tmpdir } from 'node:os'; -import { Logger } from '../Logger'; - -describe('Logger', () => { - let mockedStdoutWrite: jest.SpyInstance = null; - let mockedStderrWrite: jest.SpyInstance = null; - - beforeEach(async () => { - mockedStdoutWrite = jest - .spyOn(process.stdout, 'write') - .mockImplementation(() => true); - mockedStderrWrite = jest - .spyOn(process.stderr, 'write') - .mockImplementation(() => true); - }); - - afterEach(() => { - mockedStdoutWrite.mockReset(); - mockedStderrWrite.mockReset(); - }); - - it('logs to stdout', () => { - const logger = new Logger(tmpdir()); - logger.info('log test'); - - expect(mockedStdoutWrite.mock.calls.length).toBe(1); - expect(mockedStdoutWrite.mock.calls[0][0]).toContain('log test'); - expect(mockedStderrWrite.mock.calls.length).toBe(0); - }); - - it('logs to stderr', () => { - const logger = new Logger(tmpdir()); - logger.error('error test'); - - expect(mockedStdoutWrite.mock.calls.length).toBe(0); - expect(mockedStderrWrite.mock.calls.length).toBe(1); - expect(mockedStderrWrite.mock.calls[0][0]).toContain('error test'); - }); - - it('only writes to stderr with "stderrOnly" enabled', () => { - const stderrOnly = true; - const logger = new Logger(tmpdir(), stderrOnly); - logger.info('info test'); - logger.warn('warn test'); - // log is only logged to file now :) - logger.log('log test'); - logger.error('error test'); - - expect(mockedStdoutWrite.mock.calls.length).toBe(0); - expect(mockedStderrWrite.mock.calls.length).toBe(3); - expect(mockedStderrWrite.mock.calls[0][0]).toContain('info test'); - expect(mockedStderrWrite.mock.calls[1][0]).toContain('warn test'); - expect(mockedStderrWrite.mock.calls[2][0]).toContain('error test'); - }); -}); diff --git a/packages/graphql-language-service-server/src/__tests__/MessageProcessor-test.ts b/packages/graphql-language-service-server/src/__tests__/MessageProcessor-test.ts index dd87770581d..c150d1007f6 100644 --- a/packages/graphql-language-service-server/src/__tests__/MessageProcessor-test.ts +++ b/packages/graphql-language-service-server/src/__tests__/MessageProcessor-test.ts @@ -6,7 +6,6 @@ * LICENSE file in the root directory of this source tree. * */ -import { tmpdir } from 'node:os'; import { SymbolKind } from 'vscode-languageserver'; import { FileChangeType } from 'vscode-languageserver-protocol'; import { Position, Range } from 'graphql-language-service'; @@ -22,7 +21,7 @@ import { loadConfig } from 'graphql-config'; import type { DefinitionQueryResult, Outline } from 'graphql-language-service'; -import { Logger } from '../Logger'; +import { NoopLogger } from '../Logger'; import { pathToFileURL } from 'node:url'; jest.mock('node:fs', () => ({ @@ -31,7 +30,7 @@ jest.mock('node:fs', () => ({ })); describe('MessageProcessor', () => { - const logger = new Logger(tmpdir()); + const logger = new NoopLogger(); const messageProcessor = new MessageProcessor({ // @ts-ignore connection: {}, diff --git a/packages/graphql-language-service-server/src/__tests__/findGraphQLTags-test.ts b/packages/graphql-language-service-server/src/__tests__/findGraphQLTags-test.ts index 15aac193bb1..fbc9966f79e 100644 --- a/packages/graphql-language-service-server/src/__tests__/findGraphQLTags-test.ts +++ b/packages/graphql-language-service-server/src/__tests__/findGraphQLTags-test.ts @@ -6,16 +6,15 @@ * LICENSE file in the root directory of this source tree. * */ -import { tmpdir } from 'node:os'; import { findGraphQLTags as baseFindGraphQLTags } from '../findGraphQLTags'; jest.mock('../Logger'); -import { Logger } from '../Logger'; +import { NoopLogger } from '../Logger'; describe('findGraphQLTags', () => { - const logger = new Logger(tmpdir()); + const logger = new NoopLogger(); const findGraphQLTags = (text: string, ext: string) => baseFindGraphQLTags(text, ext, '', logger); @@ -296,12 +295,7 @@ query {id}`); .spyOn(process.stderr, 'write') .mockImplementation(() => true); - const contents = baseFindGraphQLTags( - text, - '.svelte', - '', - new Logger(tmpdir(), false), - ); + const contents = baseFindGraphQLTags(text, '.svelte', '', new NoopLogger()); // We should have no contents expect(contents).toMatchObject([]); @@ -318,12 +312,7 @@ query {id}`); .spyOn(process.stderr, 'write') .mockImplementation(() => true); - const contents = baseFindGraphQLTags( - text, - '.svelte', - '', - new Logger(tmpdir(), false), - ); + const contents = baseFindGraphQLTags(text, '.svelte', '', new NoopLogger()); // We should have no contents expect(contents).toMatchObject([]); @@ -340,12 +329,7 @@ query {id}`); .spyOn(process.stderr, 'write') .mockImplementation(() => true); - const contents = baseFindGraphQLTags( - text, - '.svelte', - '', - new Logger(tmpdir(), false), - ); + const contents = baseFindGraphQLTags(text, '.svelte', '', new NoopLogger()); // We should have no contents expect(contents).toMatchObject([]); diff --git a/packages/graphql-language-service-server/src/findGraphQLTags.ts b/packages/graphql-language-service-server/src/findGraphQLTags.ts index 5dfd5db8894..7f738dbc19a 100644 --- a/packages/graphql-language-service-server/src/findGraphQLTags.ts +++ b/packages/graphql-language-service-server/src/findGraphQLTags.ts @@ -17,7 +17,7 @@ import { Position, Range } from 'graphql-language-service'; import { parse, ParserOptions, ParserPlugin } from '@babel/parser'; import * as VueParser from '@vue/compiler-sfc'; -import { Logger } from './Logger'; +import type { Logger } from 'vscode-languageserver'; // Attempt to be as inclusive as possible of source text. const PARSER_OPTIONS: ParserOptions = { diff --git a/packages/graphql-language-service-server/src/index.ts b/packages/graphql-language-service-server/src/index.ts index 3f764bdb635..a1be21b2e92 100644 --- a/packages/graphql-language-service-server/src/index.ts +++ b/packages/graphql-language-service-server/src/index.ts @@ -11,8 +11,6 @@ export { MessageProcessor } from './MessageProcessor'; export { default as startServer } from './startServer'; -export { Logger } from './Logger'; - export * from './GraphQLCache'; export * from './parseDocument'; export * from './findGraphQLTags'; diff --git a/packages/graphql-language-service-server/src/parseDocument.ts b/packages/graphql-language-service-server/src/parseDocument.ts index a9b809af1c7..886d8d8705c 100644 --- a/packages/graphql-language-service-server/src/parseDocument.ts +++ b/packages/graphql-language-service-server/src/parseDocument.ts @@ -1,9 +1,10 @@ import { extname } from 'node:path'; import type { CachedContent } from 'graphql-language-service'; import { Range, Position } from 'graphql-language-service'; +import type { Logger } from 'vscode-languageserver'; import { findGraphQLTags, DEFAULT_TAGS } from './findGraphQLTags'; -import { Logger } from './Logger'; +import { NoopLogger } from './Logger'; export const DEFAULT_SUPPORTED_EXTENSIONS = [ '.js', @@ -48,7 +49,7 @@ export function parseDocument( uri: string, fileExtensions: string[] = DEFAULT_SUPPORTED_EXTENSIONS, graphQLFileExtensions: string[] = DEFAULT_SUPPORTED_GRAPHQL_EXTENSIONS, - logger: Logger = new Logger(), + logger: Logger = new NoopLogger(), ): CachedContent[] { // Check if the text content includes a GraphQLV query. // If the text doesn't include GraphQL queries, do not proceed. diff --git a/packages/graphql-language-service-server/src/startServer.ts b/packages/graphql-language-service-server/src/startServer.ts index c0cfd006909..0f9cf102f0d 100644 --- a/packages/graphql-language-service-server/src/startServer.ts +++ b/packages/graphql-language-service-server/src/startServer.ts @@ -150,9 +150,6 @@ export default async function startServer( options: ServerOptions, ): Promise { if (options?.method) { - const stderrOnly = options.method === 'stream'; - const logger = new Logger(options.tmpDir, stderrOnly); - const finalOptions = buildOptions(options); let reader; let writer; @@ -181,7 +178,6 @@ export default async function startServer( const s = await initializeHandlers({ reader, writer, - logger, options: finalOptions, }); s.listen(); @@ -198,37 +194,30 @@ export default async function startServer( break; } - try { - const serverWithHandlers = await initializeHandlers({ - reader, - writer, - logger, - options: finalOptions, - }); - serverWithHandlers.listen(); - } catch (err) { - logger.error('There was a Graphql LSP handler exception:'); - logger.error(String(err)); - } + const serverWithHandlers = await initializeHandlers({ + reader, + writer, + options: finalOptions, + }); + serverWithHandlers.listen(); } } type InitializerParams = { reader: SocketMessageReader | StreamMessageReader | IPCMessageReader; writer: SocketMessageWriter | StreamMessageWriter | IPCMessageWriter; - logger: Logger; options: MappedServerOptions; }; async function initializeHandlers({ reader, writer, - logger, options, }: InitializerParams): Promise { - try { - const connection = createConnection(reader, writer); + const connection = createConnection(reader, writer); + const logger = new Logger(connection); + try { await addHandlers({ connection, logger, ...options }); return connection; } catch (err) {