diff --git a/packages/core/src/lib/tree-sitter/client.test.ts b/packages/core/src/lib/tree-sitter/client.test.ts index 06170d6e35..9961d9c5e1 100644 --- a/packages/core/src/lib/tree-sitter/client.test.ts +++ b/packages/core/src/lib/tree-sitter/client.test.ts @@ -1,5 +1,5 @@ import { test, expect, beforeEach, afterEach, beforeAll, describe } from "bun:test" -import { TreeSitterClient } from "./client.js" +import { TreeSitterClient, TreeSitterClientDestroyedError } from "./client.js" import { tmpdir } from "os" import { join } from "path" import { existsSync } from "fs" @@ -145,8 +145,7 @@ describe("TreeSitterClient", () => { const outcome = await initializeOutcome expect(outcome.status).toBe("rejected") if (outcome.status === "rejected") { - expect(outcome.error).toBeInstanceOf(Error) - expect((outcome.error as Error).message).toBe("Client destroyed during initialization") + expect(outcome.error).toBeInstanceOf(TreeSitterClientDestroyedError) } expect(client.isInitialized()).toBe(false) } finally { @@ -1228,8 +1227,8 @@ describe("TreeSitterClient Edge Cases", () => { // Immediately destroy await client.destroy() - // Init promise should reject with specific error - await expect(initPromise).rejects.toThrow("Client destroyed during initialization") + // Init promise should reject with a destroy-specific error + await expect(initPromise).rejects.toBeInstanceOf(TreeSitterClientDestroyedError) await new Promise((resolve) => setTimeout(resolve, 0)) expect(client.isInitialized()).toBe(false) @@ -1361,6 +1360,44 @@ describe("TreeSitterClient Edge Cases", () => { } }) + test("should reject pending requests with TreeSitterClientDestroyedError when destroyed", async () => { + const client = new TreeSitterClient({ dataPath }) + await client.initialize() + + const internals = client as unknown as { + worker?: { + postMessage: (message: { type?: string }) => void + } + } + const worker = internals.worker + expect(worker).toBeDefined() + if (!worker) { + throw new Error("Expected initialized client to have a worker") + } + + const originalPostMessage = worker.postMessage.bind(worker) + worker.postMessage = (message) => { + if (message.type !== "ONESHOT_HIGHLIGHT") { + originalPostMessage(message) + } + } + + const observe = (promise: Promise) => + promise.then( + (value) => ({ status: "fulfilled" as const, value }), + (error: unknown) => ({ status: "rejected" as const, error }), + ) + + const outcome = observe(client.highlightOnce("const value = 1", "javascript")) + await client.destroy() + + const settled = await outcome + expect(settled.status).toBe("rejected") + if (settled.status === "rejected") { + expect(settled.error).toBeInstanceOf(TreeSitterClientDestroyedError) + } + }) + test("should handle worker errors gracefully", async () => { const client = new TreeSitterClient({ dataPath }) diff --git a/packages/core/src/lib/tree-sitter/client.ts b/packages/core/src/lib/tree-sitter/client.ts index 1e3465578d..a23a937386 100644 --- a/packages/core/src/lib/tree-sitter/client.ts +++ b/packages/core/src/lib/tree-sitter/client.ts @@ -37,6 +37,13 @@ declare global { const OTUI_TREE_SITTER_WORKER_PATH: string } +export class TreeSitterClientDestroyedError extends Error { + constructor() { + super("TreeSitter client destroyed") + this.name = "TreeSitterClientDestroyedError" + } +} + interface EditQueueItem { edits: Edit[] newContent: string @@ -715,12 +722,12 @@ export class TreeSitterClient extends EventEmitter { }) this.destroyPromise = destroyPromise - const destroyError = new Error("Client destroyed during initialization") + const destroyError = new TreeSitterClientDestroyedError() this.lifecycleGeneration++ this.initialized = false this.initializePromise = undefined this.rejectActiveInitialization(destroyError) - this.rejectPendingRequests(new Error("TreeSitter client destroyed")) + this.rejectPendingRequests(destroyError) for (const callback of this.destroyCallbacks) { try { diff --git a/packages/core/src/renderables/Code.test.ts b/packages/core/src/renderables/Code.test.ts index 4743d4b91a..9ff552e6c5 100644 --- a/packages/core/src/renderables/Code.test.ts +++ b/packages/core/src/renderables/Code.test.ts @@ -1,10 +1,10 @@ -import { test, expect, beforeEach, afterEach } from "bun:test" +import { test, expect, beforeEach, afterEach, spyOn } from "bun:test" import { CodeRenderable } from "./Code.js" import { SyntaxStyle } from "../syntax-style.js" import { RGBA } from "../lib/RGBA.js" import { createTestRenderer, type TestRenderer, MockTreeSitterClient, type MockMouse } from "../testing.js" import { ManualClock } from "../testing/manual-clock.js" -import { TreeSitterClient } from "../lib/tree-sitter/index.js" +import { TreeSitterClient, TreeSitterClientDestroyedError } from "../lib/tree-sitter/index.js" import type { SimpleHighlight } from "../lib/tree-sitter/types.js" import { BoxRenderable } from "./Box.js" import { TextAttributes, type CapturedFrame } from "../types.js" @@ -2386,3 +2386,69 @@ test("CodeRenderable - streaming with drawUnstyledText=false falls back to unsty expect(codeRenderable.plainText).toBe("const updated = 'world';") }) + +test("CodeRenderable - does not warn when highlighting fails because the client was destroyed", async () => { + const warnSpy = spyOn(console, "warn").mockImplementation(() => {}) + + const syntaxStyle = SyntaxStyle.fromStyles({ + default: { fg: RGBA.fromValues(1, 1, 1, 1) }, + }) + + const mockClient = new MockTreeSitterClient() + + const codeRenderable = new CodeRenderable(currentRenderer, { + id: "test-code", + content: "const message = 'hello';", + filetype: "javascript", + syntaxStyle, + treeSitterClient: mockClient, + conceal: false, + }) + + try { + currentRenderer.root.add(codeRenderable) + await renderOnce() + + expect(mockClient.isHighlighting()).toBe(true) + + mockClient.rejectHighlightOnce(0, new TreeSitterClientDestroyedError()) + await waitForHighlight(codeRenderable) + + expect(warnSpy).not.toHaveBeenCalled() + } finally { + warnSpy.mockRestore() + } +}) + +test("CodeRenderable - still warns when highlighting fails with a real error", async () => { + const warnSpy = spyOn(console, "warn").mockImplementation(() => {}) + + const syntaxStyle = SyntaxStyle.fromStyles({ + default: { fg: RGBA.fromValues(1, 1, 1, 1) }, + }) + + const mockClient = new MockTreeSitterClient() + + const codeRenderable = new CodeRenderable(currentRenderer, { + id: "test-code", + content: "const message = 'hello';", + filetype: "javascript", + syntaxStyle, + treeSitterClient: mockClient, + conceal: false, + }) + + try { + currentRenderer.root.add(codeRenderable) + await renderOnce() + + expect(mockClient.isHighlighting()).toBe(true) + + mockClient.rejectHighlightOnce(0, new Error("synthetic highlighting failure")) + await waitForHighlight(codeRenderable) + + expect(warnSpy).toHaveBeenCalledWith("Code highlighting failed, falling back to plain text:", expect.any(Error)) + } finally { + warnSpy.mockRestore() + } +}) diff --git a/packages/core/src/renderables/Code.ts b/packages/core/src/renderables/Code.ts index 7b4819394d..76b99eef5f 100644 --- a/packages/core/src/renderables/Code.ts +++ b/packages/core/src/renderables/Code.ts @@ -1,7 +1,7 @@ import { type LineInfo, type RenderContext } from "../types.js" import { StyledText } from "../lib/styled-text.js" import { SyntaxStyle } from "../syntax-style.js" -import { getTreeSitterClient, TreeSitterClient } from "../lib/tree-sitter/index.js" +import { getTreeSitterClient, TreeSitterClient, TreeSitterClientDestroyedError } from "../lib/tree-sitter/index.js" import { TextBufferRenderable, type TextBufferOptions } from "./TextBufferRenderable.js" import type { OptimizedBuffer } from "../buffer.js" import type { SimpleHighlight } from "../lib/tree-sitter/types.js" @@ -408,6 +408,10 @@ export class CodeRenderable extends TextBufferRenderable { return } + if (this.isDestroyed || error instanceof TreeSitterClientDestroyedError) { + return + } + console.warn("Code highlighting failed, falling back to plain text:", error) if (this.isDestroyed) return this.textBuffer.setText(content) diff --git a/packages/core/src/testing/mock-tree-sitter-client.ts b/packages/core/src/testing/mock-tree-sitter-client.ts index 093e1765f1..b72b44c69a 100644 --- a/packages/core/src/testing/mock-tree-sitter-client.ts +++ b/packages/core/src/testing/mock-tree-sitter-client.ts @@ -6,6 +6,7 @@ export class MockTreeSitterClient extends TreeSitterClient { private _highlightPromises: Array<{ promise: Promise<{ highlights?: SimpleHighlight[]; warning?: string; error?: string }> resolve: (result: { highlights?: SimpleHighlight[]; warning?: string; error?: string }) => void + reject: (error: Error) => void timeout?: TimerHandle }> = [] private _mockResult: { highlights?: SimpleHighlight[]; warning?: string; error?: string } = { highlights: [] } @@ -27,7 +28,7 @@ export class MockTreeSitterClient extends TreeSitterClient { content: string, filetype: string, ): Promise<{ highlights?: SimpleHighlight[]; warning?: string; error?: string }> { - const { promise, resolve } = Promise.withResolvers<{ + const { promise, resolve, reject } = Promise.withResolvers<{ highlights?: SimpleHighlight[] warning?: string error?: string @@ -45,7 +46,7 @@ export class MockTreeSitterClient extends TreeSitterClient { }, this._autoResolveTimeout) } - this._highlightPromises.push({ promise, resolve, timeout }) + this._highlightPromises.push({ promise, resolve, reject, timeout }) return promise } @@ -65,6 +66,17 @@ export class MockTreeSitterClient extends TreeSitterClient { } } + rejectHighlightOnce(index: number = 0, error: Error = new Error("highlight failed")) { + if (index >= 0 && index < this._highlightPromises.length) { + const item = this._highlightPromises[index] + if (item.timeout) { + this._clock.clearTimeout(item.timeout) + } + item.reject(error) + this._highlightPromises.splice(index, 1) + } + } + resolveAllHighlightOnce() { for (const { resolve, timeout } of this._highlightPromises) { if (timeout) {