From e9a29e4908fa1d28213e5b4788611e9cde1a92f2 Mon Sep 17 00:00:00 2001 From: Shoubhit Dash Date: Wed, 13 May 2026 12:25:04 +0530 Subject: [PATCH] fix(storage): type not found errors (#27265) --- .../httpapi/handlers/session-errors.ts | 6 ++--- .../instance/httpapi/middleware/error.ts | 5 ++++- packages/opencode/src/session/session.ts | 2 +- packages/opencode/src/storage/storage.ts | 20 ++++++++++++----- .../test/session/messages-pagination.test.ts | 22 ++++++++++++++++--- .../opencode/test/storage/storage.test.ts | 17 ++++++++------ 6 files changed, 51 insertions(+), 21 deletions(-) diff --git a/packages/opencode/src/server/routes/instance/httpapi/handlers/session-errors.ts b/packages/opencode/src/server/routes/instance/httpapi/handlers/session-errors.ts index 98ac2b9ad6..0fef2e7763 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/handlers/session-errors.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/session-errors.ts @@ -2,8 +2,6 @@ import type { NotFoundError as StorageNotFoundError } from "@/storage/storage" import { Effect } from "effect" import * as ApiError from "../errors" -type StorageNotFound = InstanceType - -export function mapStorageNotFound(self: Effect.Effect) { - return self.pipe(Effect.mapError((error) => ApiError.notFound(error.data.message))) +export function mapStorageNotFound(self: Effect.Effect) { + return self.pipe(Effect.mapError((error) => ApiError.notFound(error.message))) } diff --git a/packages/opencode/src/server/routes/instance/httpapi/middleware/error.ts b/packages/opencode/src/server/routes/instance/httpapi/middleware/error.ts index 585f4bd18a..c9d4871b04 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/middleware/error.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/middleware/error.ts @@ -24,11 +24,14 @@ export const errorLayer = HttpRouter.middleware<{ handles: unknown }>()((effect) const error = defect.defect log.error("failed", { error, cause: Cause.pretty(cause) }) + if (error instanceof NotFoundError) { + return Effect.succeed(HttpServerResponse.jsonUnsafe(error.toObject(), { status: 404 })) + } + if (error instanceof NamedError) { return Effect.succeed( HttpServerResponse.jsonUnsafe(error.toObject(), { status: iife(() => { - if (error instanceof NotFoundError) return 404 if (error instanceof Provider.ModelNotFoundError) return 400 if (error.name === "ProviderAuthValidationFailed") return 400 if (error.name.startsWith("Worktree")) return 400 diff --git a/packages/opencode/src/session/session.ts b/packages/opencode/src/session/session.ts index 5e574d9911..0913b85687 100644 --- a/packages/opencode/src/session/session.ts +++ b/packages/opencode/src/session/session.ts @@ -448,7 +448,7 @@ export class BusyError extends Error { } } -export type NotFound = InstanceType +export type NotFound = NotFoundError export interface Interface { readonly list: (input?: ListInput) => Effect.Effect diff --git a/packages/opencode/src/storage/storage.ts b/packages/opencode/src/storage/storage.ts index e1f5f681bb..ee2ee45f85 100644 --- a/packages/opencode/src/storage/storage.ts +++ b/packages/opencode/src/storage/storage.ts @@ -1,7 +1,6 @@ import * as Log from "@opencode-ai/core/util/log" import path from "path" import { Global } from "@opencode-ai/core/global" -import { NamedError } from "@opencode-ai/core/util/error" import { AppFileSystem } from "@opencode-ai/core/filesystem" import { Effect, Exit, Layer, Option, RcMap, Schema, Context, TxReentrantLock } from "effect" import { NonNegativeInt } from "@opencode-ai/core/schema" @@ -15,11 +14,22 @@ type Migration = ( git: Git.Interface, ) => Effect.Effect -export const NotFoundError = NamedError.create("NotFoundError", { +export class NotFoundError extends Schema.TaggedErrorClass()("NotFoundError", { message: Schema.String, -}) +}) { + static isInstance(input: unknown): input is NotFoundError { + return input instanceof NotFoundError + } -export type Error = AppFileSystem.Error | InstanceType + toObject() { + return { + name: "NotFoundError" as const, + data: { message: this.message }, + } + } +} + +export type Error = AppFileSystem.Error | NotFoundError const RootFile = Schema.Struct({ path: Schema.optional( @@ -245,7 +255,7 @@ export const layer = Layer.effect( }), ) - const fail = (target: string): Effect.Effect> => + const fail = (target: string): Effect.Effect => Effect.fail(new NotFoundError({ message: `Resource not found: ${target}` })) const wrap = (target: string, body: Effect.Effect) => diff --git a/packages/opencode/test/session/messages-pagination.test.ts b/packages/opencode/test/session/messages-pagination.test.ts index 09e8d7b429..5585aa0b01 100644 --- a/packages/opencode/test/session/messages-pagination.test.ts +++ b/packages/opencode/test/session/messages-pagination.test.ts @@ -4,6 +4,7 @@ import { Session as SessionNs } from "@/session/session" import { MessageV2 } from "../../src/session/message-v2" import { MessageID, PartID, type SessionID } from "../../src/session/schema" import { ModelID, ProviderID } from "../../src/provider/schema" +import { NotFoundError } from "@/storage/storage" import * as Log from "@opencode-ai/core/util/log" import { testEffect } from "../lib/effect" @@ -11,6 +12,20 @@ void Log.init({ print: false }) const it = testEffect(SessionNs.defaultLayer) +function expectNotFound(fn: () => unknown, message: string) { + let thrown: unknown + try { + fn() + } catch (error) { + thrown = error + } + expect(thrown).toBeInstanceOf(NotFoundError) + if (thrown instanceof NotFoundError) { + expect(thrown._tag).toBe("NotFoundError") + expect(thrown.message).toBe(message) + } +} + const withSession = ( fn: (input: { session: SessionNs.Interface; sessionID: SessionID }) => Effect.Effect, ) => @@ -186,7 +201,7 @@ describe("MessageV2.page", () => { it.instance("throws NotFoundError for non-existent session", () => Effect.gen(function* () { const fake = "non-existent-session" as SessionID - expect(() => MessageV2.page({ sessionID: fake, limit: 10 })).toThrow("NotFoundError") + expectNotFound(() => MessageV2.page({ sessionID: fake, limit: 10 }), `Session not found: ${fake}`) }), ) @@ -471,7 +486,8 @@ describe("MessageV2.get", () => { it.instance("throws NotFoundError for non-existent message", () => withSession(({ sessionID }) => Effect.gen(function* () { - expect(() => MessageV2.get({ sessionID, messageID: MessageID.ascending() })).toThrow("NotFoundError") + const messageID = MessageID.ascending() + expectNotFound(() => MessageV2.get({ sessionID, messageID }), `Message not found: ${messageID}`) }), ), ) @@ -483,7 +499,7 @@ describe("MessageV2.get", () => { const b = yield* session.create({}) const [id] = yield* fill(a.id, 1) - expect(() => MessageV2.get({ sessionID: b.id, messageID: id })).toThrow("NotFoundError") + expectNotFound(() => MessageV2.get({ sessionID: b.id, messageID: id }), `Message not found: ${id}`) const result = MessageV2.get({ sessionID: a.id, messageID: id }) expect(result.info.id).toBe(id) diff --git a/packages/opencode/test/storage/storage.test.ts b/packages/opencode/test/storage/storage.test.ts index f0aff4ba78..d0fe5dd34c 100644 --- a/packages/opencode/test/storage/storage.test.ts +++ b/packages/opencode/test/storage/storage.test.ts @@ -74,20 +74,23 @@ describe("Storage", () => { it.live("maps missing reads to NotFoundError", () => Effect.gen(function* () { const { root, svc } = yield* scope() - const exit = yield* svc.read([...root, "missing", "value"]).pipe(Effect.exit) - expect(Exit.isFailure(exit)).toBe(true) + const error = yield* Effect.flip(svc.read([...root, "missing", "value"])) + expect(error).toBeInstanceOf(Storage.NotFoundError) + expect(error._tag).toBe("NotFoundError") + expect(error.message).toContain(path.join(...root, "missing", "value") + ".json") }), ) it.live("update on missing key throws NotFoundError", () => Effect.gen(function* () { const { root, svc } = yield* scope() - const exit = yield* svc - .update<{ value: number }>([...root, "missing", "key"], (draft) => { + const error = yield* Effect.flip( + svc.update<{ value: number }>([...root, "missing", "key"], (draft) => { draft.value += 1 - }) - .pipe(Effect.exit) - expect(Exit.isFailure(exit)).toBe(true) + }), + ) + expect(error).toBeInstanceOf(Storage.NotFoundError) + expect(error._tag).toBe("NotFoundError") }), )