diff --git a/packages/core/src/rpc.ts b/packages/core/src/rpc.ts index 1f306f61066b..d085abc890d0 100644 --- a/packages/core/src/rpc.ts +++ b/packages/core/src/rpc.ts @@ -121,7 +121,15 @@ const layer = Layer.effect( // The heterogeneous registry erases handlers after their selected schema validates input. const execution: Effect.Effect = Reflect.apply(handler, undefined, [parsed, callContext]) return execution - }).pipe(Effect.catch((error) => encodeError(method, error))) + }).pipe( + Effect.catch((error) => encodeError(method, error)), + // Normalize handler bugs here so direct callers can recover just like HTTP callers. + Effect.catchDefect((defect) => + Effect.logError("rpc handler failed", { rpc: rpcID, method: name, defect }).pipe( + Effect.andThen(Effect.fail(failure("rpc.internal", "RPC call failed"))), + ), + ), + ) return yield* encode(method.output, result).pipe( Effect.mapError((error) => failure("rpc.invalid_output", errorMessage(error, "Invalid RPC output"))), ) diff --git a/packages/core/test/rpc-handler-errors.test.ts b/packages/core/test/rpc-handler-errors.test.ts new file mode 100644 index 000000000000..bcf77f799c69 --- /dev/null +++ b/packages/core/test/rpc-handler-errors.test.ts @@ -0,0 +1,62 @@ +import { expect } from "bun:test" +import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder" +import { Location } from "@opencode-ai/core/location" +import { Rpc } from "@opencode-ai/core/rpc" +import { AbsolutePath } from "@opencode-ai/core/schema" +import { Effect, Layer, Logger, Schema } from "effect" +import { location } from "./fixture/location" +import { testEffect } from "./lib/effect" + +const it = testEffect( + AppNodeBuilder.build(Rpc.node, [ + Location.node.replace(Layer.succeed(Location.Service, location({ directory: AbsolutePath.make("/rpc-project") }))), + ]), +) +const Broken = Rpc.define({ + id: "broken", + methods: { + dies: { input: Schema.Undefined, output: Schema.String }, + throws: { input: Schema.Undefined, output: Schema.String }, + raw: { input: Schema.Undefined, output: Schema.String }, + undeclared: { input: Schema.Undefined, output: Schema.String }, + invalidError: { + input: Schema.Undefined, + output: Schema.String, + errors: { known: Schema.Struct({ count: Schema.Int }) }, + }, + }, + events: {}, +}) + +for (const method of ["dies", "throws", "raw", "undeclared", "invalidError"] as const) { + it.effect(`recovers from ${method} through the typed rpc.internal failure`, () => + Effect.gen(function* () { + const rpc = yield* Rpc.Service + yield* rpc.register(Broken, { + dies: () => Effect.die(new Error("handler defect")), + throws: () => { + throw new Error("handler threw") + }, + // Raw Promise rejections reach this boundary as failed Effects. + // @ts-expect-error intentionally exercise an undeclared failure + raw: () => Effect.fail(new Error("raw failure")), + // @ts-expect-error intentionally exercise an undeclared error name + undeclared: (_input, context) => Effect.fail(context.error("unknown", "Unknown")), + invalidError: (_input, context) => Effect.fail(context.error("known", "Invalid count", { count: 1.5 })), + }) + const logged: unknown[] = [] + const result = yield* rpc + .client(Broken) + [method]() + .pipe( + Effect.catchIf( + (error) => "type" in error && error.type === "rpc.internal", + (error) => Effect.succeed(error), + ), + Effect.provideService(Logger.CurrentLoggers, new Set([Logger.make((entry) => logged.push(entry.message))])), + ) + expect(result).toEqual({ type: "rpc.internal", message: "RPC call failed" }) + expect(logged).toHaveLength(1) + }), + ) +} diff --git a/packages/server/src/handlers/rpc.ts b/packages/server/src/handlers/rpc.ts index efb0661215d0..27f1e04a17cf 100644 --- a/packages/server/src/handlers/rpc.ts +++ b/packages/server/src/handlers/rpc.ts @@ -14,7 +14,7 @@ export const RpcHandler = HttpApiBuilder.group(Api, "server.rpc", (handlers) => return output === undefined ? {} : { output } }).pipe( Effect.mapError((error) => - error.type === "rpc.invalid_output" + error.type === "rpc.invalid_output" || error.type === "rpc.internal" ? new RpcInternalError({ type: error.type, message: error.message }) : new RpcError({ type: error.type, @@ -22,12 +22,10 @@ export const RpcHandler = HttpApiBuilder.group(Api, "server.rpc", (handlers) => ...(error.data === undefined ? {} : { data: error.data }), }), ), - Effect.catchDefect((error) => - Effect.fail( - new RpcInternalError({ - type: "rpc.internal", - message: error instanceof Error ? error.message : "RPC call failed", - }), + // Defects outside handler execution are still logged, never echoed to the client. + Effect.catchDefect((defect) => + Effect.logError("rpc call failed", { rpc: params.rpcID, method: params.method, defect }).pipe( + Effect.andThen(Effect.fail(new RpcInternalError({ type: "rpc.internal", message: "RPC call failed" }))), ), ), ), diff --git a/packages/server/test/rpc-handler-errors.test.ts b/packages/server/test/rpc-handler-errors.test.ts new file mode 100644 index 000000000000..51f6c5bd1b23 --- /dev/null +++ b/packages/server/test/rpc-handler-errors.test.ts @@ -0,0 +1,74 @@ +import { expect } from "bun:test" +import { SdkPlugins } from "@opencode-ai/core/plugin/sdk" +import { define } from "@opencode-ai/plugin/effect/plugin" +import { Rpc } from "@opencode-ai/schema/rpc" +import { Context, Effect, Layer, Schema } from "effect" +import { HttpEffect, HttpRouter, HttpServer } from "effect/unstable/http" +import { tmpdirScoped } from "../../core/test/fixture/tmpdir" +import { it } from "../../core/test/lib/effect" +import { createEmbeddedRoutes } from "../src/routes" + +const Broken = Rpc.define({ + id: "broken", + methods: { + handler: { input: Schema.String, output: Schema.String }, + schema: { + input: Schema.String.check( + Schema.makeFilter(() => { + throw new Error("private schema detail") + }), + ), + output: Schema.String, + }, + }, + events: {}, +}) + +for (const method of ["handler", "schema"] as const) { + it.live(`returns HTTP 500 without exposing the ${method} defect`, () => + Effect.gen(function* () { + const directory = yield* tmpdirScoped() + const context = yield* Layer.build( + createEmbeddedRoutes({ + database: { path: ":memory:" }, + models: { fetch: false }, + config: { directory: directory.path, project: false, content: "{}" }, + fs: { filewatcher: false }, + }).pipe(Layer.provide(HttpServer.layerServices)), + ) + const sdk = Context.get(context, SdkPlugins.Service) + yield* sdk.register( + define({ + id: "broken-rpc", + effect: (ctx) => + ctx.rpc + .register(Broken, { + handler: () => Effect.die(new Error("private handler detail")), + schema: Effect.succeed, + }) + .pipe(Effect.asVoid, Effect.orDie), + }), + ) + const handler = Context.get(context, HttpRouter.HttpRouter) + .asHttpEffect() + .pipe(HttpEffect.toWebHandlerWith(context)) + const url = new URL(`/api/rpc/broken/${method}`, "http://opencode.local") + url.searchParams.set("location[directory]", directory.path) + const response = yield* Effect.promise(() => + handler( + new Request(url, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ input: "hello" }), + }), + ), + ) + expect(response.status).toBe(500) + expect(yield* Effect.promise(() => response.json())).toEqual({ + _tag: "RpcInternalError", + type: "rpc.internal", + message: "RPC call failed", + }) + }), + ) +}