Skip to content
Open
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
47 changes: 42 additions & 5 deletions packages/core/src/lib/tree-sitter/client.test.ts
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 = <T>(promise: Promise<T>) =>
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 })

Expand Down
11 changes: 9 additions & 2 deletions packages/core/src/lib/tree-sitter/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -715,12 +722,12 @@ export class TreeSitterClient extends EventEmitter<TreeSitterClientEvents> {
})
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 {
Expand Down
70 changes: 68 additions & 2 deletions packages/core/src/renderables/Code.test.ts
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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()
}
})
6 changes: 5 additions & 1 deletion packages/core/src/renderables/Code.ts
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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)
Expand Down
16 changes: 14 additions & 2 deletions packages/core/src/testing/mock-tree-sitter-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [] }
Expand All @@ -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
Expand All @@ -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
}
Expand All @@ -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) {
Expand Down