diff --git a/src/common/chunk-util.ts b/src/common/chunk-util.ts index 8bb416654..36701c973 100644 --- a/src/common/chunk-util.ts +++ b/src/common/chunk-util.ts @@ -10,7 +10,8 @@ const defaultWaiter = (waitTime: number): Promise => export const retryAsyncLoad = async ( load: () => Promise, - waiter: (waitTime: number) => Promise = defaultWaiter + waiter: (waitTime: number) => Promise = defaultWaiter, + shouldRetry: (e: unknown) => boolean = () => true ): Promise => { let waitTime = 250; let attempts = 0; @@ -20,6 +21,9 @@ export const retryAsyncLoad = async ( // Must await here! return await load(); } catch (e) { + if (!shouldRetry(e)) { + throw e; + } // If the user is offline, bail immediately. if (!navigator.onLine) { throw new OfflineError(); diff --git a/src/e2e/language-server-retry.test.ts b/src/e2e/language-server-retry.test.ts new file mode 100644 index 000000000..a20cfcd79 --- /dev/null +++ b/src/e2e/language-server-retry.test.ts @@ -0,0 +1,30 @@ +/** + * (c) 2026, Micro:bit Educational Foundation and contributors + * + * SPDX-License-Identifier: MIT + */ +import { expect } from "@playwright/test"; +import { test } from "./app-test-fixtures.js"; + +test.describe("language server start-up", () => { + // A service worker would serve the script from its precache, out of + // reach of the route below. + test.use({ serviceWorkers: "block" }); + + let requests: number; + + test.beforeEach(async ({ context }) => { + requests = 0; + await context.route(/pyright-locale-/, (route) => + ++requests <= 2 ? route.abort("failed") : route.continue() + ); + }); + + test("recovers when its language script fails to load", async ({ app }) => { + await expect.poll(() => requests).toBeGreaterThan(2); + + await app.selectAllInEditor(); + await app.typeInEditor("from microbit import *\ndisplay.s"); + await app.expectCompletionOptions(["scroll", "set_pixel", "show"]); + }); +}); diff --git a/src/language-server/client.test.ts b/src/language-server/client.test.ts new file mode 100644 index 000000000..14da51e04 --- /dev/null +++ b/src/language-server/client.test.ts @@ -0,0 +1,126 @@ +/** + * (c) 2026, Micro:bit Educational Foundation and contributors + * + * SPDX-License-Identifier: MIT + */ +import { ToastFn } from "@microbit/ui"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + ConnectionError, + ConnectionErrors, + MessageConnection, +} from "vscode-jsonrpc"; +import { + InitializedNotification, + InitializeParams, + InitializeRequest, +} from "vscode-languageserver-protocol"; +import { createUri, LanguageServerClient } from "./client"; + +const loadFailed = () => + new Error("Request initialize failed with message: Load failed"); + +const createClient = (locale: string = "fr") => { + const sendRequest = vi.fn(); + const sendNotification = vi.fn(); + const connection = { + sendRequest, + sendNotification, + onNotification: vi.fn(), + onRequest: vi.fn(), + } as unknown as MessageConnection; + const toast = vi.fn(); + const client = new LanguageServerClient( + connection, + locale, + createUri(""), + toast as unknown as ToastFn + ); + const requestedLocales = () => + sendRequest.mock.calls.map((call) => { + const [type, params] = call as [unknown, InitializeParams]; + expect(type).toBe(InitializeRequest.type); + return params.locale; + }); + return { client, sendRequest, sendNotification, toast, requestedLocales }; +}; + +describe("LanguageServerClient initialize", () => { + let onLine: boolean; + + beforeEach(() => { + onLine = true; + vi.spyOn(navigator, "onLine", "get").mockImplementation(() => onLine); + // Loading the real typeshed is slow and is I/O that fake timers can't + // advance past. + vi.spyOn( + LanguageServerClient.prototype as any, + "getInitializationOptions" + ).mockResolvedValue({}); + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it("retries when the server fails to initialize", async () => { + const { client, sendRequest, sendNotification, toast, requestedLocales } = + createClient(); + const capabilities = { hoverProvider: true }; + sendRequest + .mockRejectedValueOnce(loadFailed()) + .mockRejectedValueOnce(loadFailed()) + .mockResolvedValueOnce({ capabilities }); + + const result = client.initialize(); + await vi.runAllTimersAsync(); + + expect(await result).toBe(true); + expect(requestedLocales()).toEqual(["fr", "fr", "fr"]); + expect(client.capabilities).toBe(capabilities); + expect(sendNotification).toHaveBeenCalledExactlyOnceWith( + InitializedNotification.type, + {} + ); + expect(toast).not.toHaveBeenCalled(); + }); + + it("can be tried again after giving up", async () => { + const { client, sendRequest } = createClient(); + sendRequest.mockRejectedValue(loadFailed()); + + const failed = expect(client.initialize()).rejects.toThrow("Load failed"); + await vi.runAllTimersAsync(); + await failed; + expect(sendRequest).toHaveBeenCalledTimes(5); + + sendRequest.mockReset().mockResolvedValue({ capabilities: {} }); + expect(await client.initialize()).toBe(true); + }); + + it("falls back to the precached locale when offline", async () => { + const { client, sendRequest, toast, requestedLocales } = createClient(); + onLine = false; + sendRequest + .mockRejectedValueOnce(loadFailed()) + .mockResolvedValueOnce({ capabilities: {} }); + + expect(await client.initialize()).toBe(true); + expect(requestedLocales()).toEqual(["fr", "en"]); + expect(client.locale).toBe("en"); + expect(toast).toHaveBeenCalledTimes(1); + }); + + it("does not retry when the connection was disposed", async () => { + const { client, sendRequest, sendNotification } = createClient(); + sendRequest.mockRejectedValue( + new ConnectionError(ConnectionErrors.Disposed, "Connection is disposed.") + ); + + expect(await client.initialize()).toBe(false); + expect(sendRequest).toHaveBeenCalledTimes(1); + expect(sendNotification).not.toHaveBeenCalled(); + }); +}); diff --git a/src/language-server/client.ts b/src/language-server/client.ts index 0e837087f..459c5879e 100644 --- a/src/language-server/client.ts +++ b/src/language-server/client.ts @@ -103,109 +103,129 @@ export class LanguageServerClient extends TypedEventTarget { } this.initializePromise = (async () => { try { - this.connection.onNotification(LogMessageNotification.type, (params) => - console.log("[LS]", params.message) - ); - - this.connection.onNotification( - PublishDiagnosticsNotification.type, - (params) => { - this.diagnostics.set(params.uri, params.diagnostics); - // Republish as you can't listen twice. - this.dispatchTypedEvent( - "diagnostics", - new DiagnosticsEvent(params) - ); - } - ); - this.connection.onRequest(RegistrationRequest.type, () => { - // Ignore. I don't think we should get these at all given our - // capabilities, but Pyright is sending one anyway. - }); - - const initializeParams: InitializeParams = { - // Our pyright fork names its message files in lowercase. - locale: this.locale.toLowerCase(), - capabilities: { - textDocument: { - moniker: {}, - synchronization: { - willSave: false, - didSave: false, - willSaveWaitUntil: false, - }, - completion: { - completionItem: { - snippetSupport: false, - commitCharactersSupport: true, - documentationFormat: ["markdown"], - deprecatedSupport: false, - preselectSupport: false, - }, - contextSupport: true, - }, - signatureHelp: { - signatureInformation: { - documentationFormat: ["markdown"], - activeParameterSupport: true, - parameterInformation: { - labelOffsetSupport: true, - }, - }, - }, - publishDiagnostics: { - tagSupport: { - valueSet: [ - DiagnosticTag.Unnecessary, - DiagnosticTag.Deprecated, - ], - }, - }, - }, - workspace: { - workspaceFolders: true, - didChangeConfiguration: {}, - configuration: true, - }, - }, - initializationOptions: await this.getInitializationOptions(), - processId: null, - // Do we need both of these? - rootUri: this.rootUri, - workspaceFolders: [ - { - name: "src", - uri: this.rootUri, - }, - ], - }; - const { capabilities } = await this.connection.sendRequest( - InitializeRequest.type, - initializeParams - ); - this.capabilities = capabilities; + this.registerHandlers(); + this.capabilities = await this.initializeWithRetry(); this.connection.sendNotification(InitializedNotification.type, {}); + return true; } catch (e) { if (isErrorDueToDispose(e)) { // We've intentionally disposed the connection because we're recreating the client. // This mostly happens due to React 18 strict mode but could happen due to language changes. return false; } - if (!navigator.onLine) { - showOfflineLanguageToast(this.toast); - // Fallback to the precached locale if user is offline. - this.locale = fallbackLocale; - this.initializePromise = undefined; - this.initialize(); - } else { - throw e; - } + // Let a later call try again rather than replaying this failure. + this.initializePromise = undefined; + throw e; } - return true; })(); return this.initializePromise; } + private registerHandlers() { + this.connection.onNotification(LogMessageNotification.type, (params) => + console.log("[LS]", params.message) + ); + this.connection.onNotification( + PublishDiagnosticsNotification.type, + (params) => { + this.diagnostics.set(params.uri, params.diagnostics); + // Republish as you can't listen twice. + this.dispatchTypedEvent("diagnostics", new DiagnosticsEvent(params)); + } + ); + this.connection.onRequest(RegistrationRequest.type, () => { + // Ignore. I don't think we should get these at all given our + // capabilities, but Pyright is sending one anyway. + }); + } + + /** + * The server loads its messages for the locale over the network before + * anything else, so initialize fails on a network error and is safe to + * send again on the same connection. + */ + private async initializeWithRetry(): Promise { + const sendInitialize = async (params: InitializeParams) => { + const { capabilities } = await this.connection.sendRequest( + InitializeRequest.type, + params + ); + return capabilities; + }; + const params = await this.getInitializeParams(); + try { + return await retryAsyncLoad( + () => sendInitialize(params), + undefined, + (e) => !isErrorDueToDispose(e) + ); + } catch (e) { + if (e instanceof OfflineError && this.locale !== fallbackLocale) { + showOfflineLanguageToast(this.toast); + // The fallback locale is precached. + this.locale = fallbackLocale; + return sendInitialize(await this.getInitializeParams()); + } + throw e; + } + } + + private async getInitializeParams(): Promise { + return { + // Our pyright fork names its message files in lowercase. + locale: this.locale.toLowerCase(), + capabilities: { + textDocument: { + moniker: {}, + synchronization: { + willSave: false, + didSave: false, + willSaveWaitUntil: false, + }, + completion: { + completionItem: { + snippetSupport: false, + commitCharactersSupport: true, + documentationFormat: ["markdown"], + deprecatedSupport: false, + preselectSupport: false, + }, + contextSupport: true, + }, + signatureHelp: { + signatureInformation: { + documentationFormat: ["markdown"], + activeParameterSupport: true, + parameterInformation: { + labelOffsetSupport: true, + }, + }, + }, + publishDiagnostics: { + tagSupport: { + valueSet: [DiagnosticTag.Unnecessary, DiagnosticTag.Deprecated], + }, + }, + }, + workspace: { + workspaceFolders: true, + didChangeConfiguration: {}, + configuration: true, + }, + }, + initializationOptions: await this.getInitializationOptions(), + processId: null, + // Do we need both of these? + rootUri: this.rootUri, + workspaceFolders: [ + { + name: "src", + uri: this.rootUri, + }, + ], + }; + } + private async getInitializationOptions(): Promise { const branch = microPythonConfig.stubs; let typeshed; diff --git a/src/language-server/pyright.ts b/src/language-server/pyright.ts index 13a1ca60a..b5b6d8a04 100644 --- a/src/language-server/pyright.ts +++ b/src/language-server/pyright.ts @@ -45,15 +45,16 @@ export const pyright = async ( return undefined; } if (cache) { - // This is safe to call if already initialized. - await cache.client.initialize(); + const { client } = cache; if (cache.language === language) { - return cache.client; - } else { - // Dispose it, we'll create a new one. - cache?.client.dispose(); - cache = undefined; + // This is safe to call if already initialized. + await client.initialize(); + return client; } + // Dispose without waiting for it to initialize, which could be a long + // wait on retries for a language we no longer want. + client.dispose(); + cache = undefined; } const idSuffix = counter++;