From 433a220adb36dc341edc00e365338515be98b3db Mon Sep 17 00:00:00 2001 From: wilfoa Date: Tue, 10 Feb 2026 09:12:21 +0200 Subject: [PATCH] fix: catch plugin event handler errors and provide logger to plugins Plugin event handlers that throw errors or call console.error() can corrupt the TUI display since stderr output appears inline in the terminal. This change: 1. Wraps plugin event handler calls in try-catch so unhandled exceptions are logged to the log file instead of crashing or corrupting the TUI. 2. Adds a `log` property to PluginInput, giving plugins a proper way to log messages to OpenCode's log file instead of using console.error(). 3. Adds `await` to the event handler call so errors are properly caught. 4. Adds tests proving: - Throwing plugins don't crash the bus event loop - Async rejections are caught - A throwing plugin doesn't prevent other plugins from receiving events - Errors are logged via Log system, not printed to stderr - The log property is available to plugins with all expected methods --- packages/opencode/src/plugin/index.ts | 15 +- .../test/plugin/event-error-handling.test.ts | 227 ++++++++++++++++++ packages/plugin/src/index.ts | 12 + 3 files changed, 251 insertions(+), 3 deletions(-) create mode 100644 packages/opencode/test/plugin/event-error-handling.test.ts diff --git a/packages/opencode/src/plugin/index.ts b/packages/opencode/src/plugin/index.ts index 24dc695d6350..096f0fd8ad26 100644 --- a/packages/opencode/src/plugin/index.ts +++ b/packages/opencode/src/plugin/index.ts @@ -30,6 +30,7 @@ export namespace Plugin { }) const config = await Config.get() const hooks: Hooks[] = [] + const pluginLog = Log.create({ service: "plugin.user" }) const input: PluginInput = { client, project: Instance.project, @@ -37,6 +38,7 @@ export namespace Plugin { directory: Instance.directory, serverUrl: Server.url(), $: Bun.$, + log: pluginLog, } for (const plugin of INTERNAL_PLUGINS) { @@ -129,9 +131,16 @@ export namespace Plugin { Bus.subscribeAll(async (input) => { const hooks = await state().then((x) => x.hooks) for (const hook of hooks) { - hook["event"]?.({ - event: input, - }) + try { + await hook["event"]?.({ + event: input, + }) + } catch (error) { + log.error("plugin event handler threw an error", { + eventType: input.type, + error: error instanceof Error ? error.message : String(error), + }) + } } }) } diff --git a/packages/opencode/test/plugin/event-error-handling.test.ts b/packages/opencode/test/plugin/event-error-handling.test.ts new file mode 100644 index 000000000000..613f6fa3064e --- /dev/null +++ b/packages/opencode/test/plugin/event-error-handling.test.ts @@ -0,0 +1,227 @@ +import { describe, expect, test, spyOn } from "bun:test" +import path from "path" +import fs from "fs/promises" +import { tmpdir } from "../fixture/fixture" +import { Instance } from "../../src/project/instance" +import { Plugin } from "../../src/plugin" +import { Bus } from "../../src/bus" +import { SessionStatus } from "../../src/session/status" +import { Log } from "../../src/util/log" + +describe("plugin.event-error-handling", () => { + test("plugin event handler that throws synchronously does not crash the bus", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + const pluginDir = path.join(dir, ".opencode", "plugin") + await fs.mkdir(pluginDir, { recursive: true }) + + await Bun.write( + path.join(pluginDir, "throwing-plugin.ts"), + [ + "export default async () => ({", + " event: async ({ event }) => {", + ' throw new Error("simulated plugin crash");', + " },", + "})", + "", + ].join("\n"), + ) + }, + }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await Plugin.init() + // Publishing a bus event should NOT throw even though the plugin throws + await expect( + Bus.publish(SessionStatus.Event.Idle, { sessionID: "test-session" }), + ).resolves.toBeDefined() + }, + }) + }, 30000) + + test("plugin event handler that rejects asynchronously does not crash the bus", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + const pluginDir = path.join(dir, ".opencode", "plugin") + await fs.mkdir(pluginDir, { recursive: true }) + + await Bun.write( + path.join(pluginDir, "rejecting-plugin.ts"), + [ + "export default async () => ({", + " event: async ({ event }) => {", + " await new Promise((_, reject) => {", + ' reject(new Error("simulated async rejection"));', + " });", + " },", + "})", + "", + ].join("\n"), + ) + }, + }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await Plugin.init() + await expect( + Bus.publish(SessionStatus.Event.Idle, { sessionID: "test-session" }), + ).resolves.toBeDefined() + }, + }) + }, 30000) + + test("throwing plugin does not prevent other plugins from receiving events", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + const pluginDir = path.join(dir, ".opencode", "plugin") + await fs.mkdir(pluginDir, { recursive: true }) + + // Plugin A throws + await Bun.write( + path.join(pluginDir, "a-throwing-plugin.ts"), + [ + "export default async () => ({", + " event: async ({ event }) => {", + ' throw new Error("plugin A exploded");', + " },", + "})", + "", + ].join("\n"), + ) + + // Plugin B writes a marker file to prove it ran + const markerPath = path.join(dir, "plugin-b-ran.marker") + await Bun.write( + path.join(pluginDir, "b-working-plugin.ts"), + [ + 'import { writeFileSync } from "fs";', + `const markerPath = ${JSON.stringify(markerPath)};`, + "export default async () => ({", + " event: async ({ event }) => {", + ' writeFileSync(markerPath, "plugin-b-received-event");', + " },", + "})", + "", + ].join("\n"), + ) + + return { markerPath } + }, + }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await Plugin.init() + await Bus.publish(SessionStatus.Event.Idle, { sessionID: "test-session" }) + + // Give async handlers a tick to complete + await new Promise((r) => setTimeout(r, 100)) + + // Plugin B should have run despite Plugin A throwing + const marker = await fs.readFile(tmp.extra.markerPath, "utf-8").catch(() => null) + expect(marker).toBe("plugin-b-received-event") + }, + }) + }, 30000) + + test("plugin event handler error is logged via Log system, not stderr", async () => { + const logSpy = spyOn(Log.create({ service: "plugin" }), "error") + + await using tmp = await tmpdir({ + init: async (dir) => { + const pluginDir = path.join(dir, ".opencode", "plugin") + await fs.mkdir(pluginDir, { recursive: true }) + + await Bun.write( + path.join(pluginDir, "stderr-plugin.ts"), + [ + "export default async () => ({", + " event: async ({ event }) => {", + ' throw new Error("this should be logged not printed to stderr");', + " },", + "})", + "", + ].join("\n"), + ) + }, + }) + + // Capture stderr to verify plugin errors don't appear there + const stderrChunks: string[] = [] + const originalWrite = process.stderr.write.bind(process.stderr) + const stderrSpy = spyOn(process.stderr, "write").mockImplementation((chunk: any) => { + const text = typeof chunk === "string" ? chunk : chunk?.toString?.() ?? "" + stderrChunks.push(text) + return originalWrite(chunk) + }) + + try { + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await Plugin.init() + await Bus.publish(SessionStatus.Event.Idle, { sessionID: "test-session" }) + // Give async handlers a tick to complete + await new Promise((r) => setTimeout(r, 100)) + }, + }) + + // The error message should NOT appear on stderr as an uncaught error + const stderrOutput = stderrChunks.join("") + expect(stderrOutput).not.toContain("this should be logged not printed to stderr") + } finally { + stderrSpy.mockRestore() + } + }, 30000) + + test("log property is provided to plugins via PluginInput", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + const pluginDir = path.join(dir, ".opencode", "plugin") + await fs.mkdir(pluginDir, { recursive: true }) + + const markerPath = path.join(dir, "log-available.marker") + await Bun.write( + path.join(pluginDir, "log-check-plugin.ts"), + [ + 'import { writeFileSync } from "fs";', + `const markerPath = ${JSON.stringify(markerPath)};`, + "export default async ({ log }) => {", + " // Verify log has the expected methods", + " const hasDebug = typeof log.debug === 'function';", + " const hasInfo = typeof log.info === 'function';", + " const hasWarn = typeof log.warn === 'function';", + " const hasError = typeof log.error === 'function';", + " const allPresent = hasDebug && hasInfo && hasWarn && hasError;", + ` writeFileSync(markerPath, allPresent ? "log-ok" : "log-missing-methods");`, + " // Use the logger - should not throw", + ' log.info("plugin loaded successfully", { test: true });', + ' log.error("test error from plugin", { code: 42 });', + " return {};", + "}", + "", + ].join("\n"), + ) + + return { markerPath } + }, + }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + // Just loading the plugin (via list()) triggers plugin initialization + // which passes PluginInput including log + await Plugin.list() + + const marker = await fs.readFile(tmp.extra.markerPath, "utf-8").catch(() => null) + expect(marker).toBe("log-ok") + }, + }) + }, 30000) +}) diff --git a/packages/plugin/src/index.ts b/packages/plugin/src/index.ts index 4cc84a5f3255..41d63881c7b5 100644 --- a/packages/plugin/src/index.ts +++ b/packages/plugin/src/index.ts @@ -23,6 +23,13 @@ export type ProviderContext = { options: Record } +export type PluginLogger = { + debug(message?: any, extra?: Record): void + info(message?: any, extra?: Record): void + warn(message?: any, extra?: Record): void + error(message?: any, extra?: Record): void +} + export type PluginInput = { client: ReturnType project: Project @@ -30,6 +37,11 @@ export type PluginInput = { worktree: string serverUrl: URL $: BunShell + /** + * Logger that writes to OpenCode's log file instead of stderr. + * Use this instead of `console.error()` to avoid corrupting the TUI display. + */ + log: PluginLogger } export type Plugin = (input: PluginInput) => Promise