chore: refactoring and tests (#12629)

This commit is contained in:
Adam
2026-02-08 05:02:19 -06:00
committed by GitHub
parent 19b1222cd8
commit d1ebe0767c
16 changed files with 744 additions and 113 deletions

View File

@@ -0,0 +1,102 @@
import { beforeAll, beforeEach, describe, expect, mock, test } from "bun:test"
type PersistTestingType = typeof import("./persist").PersistTesting
class MemoryStorage implements Storage {
private values = new Map<string, string>()
readonly events: string[] = []
readonly calls = { get: 0, set: 0, remove: 0 }
clear() {
this.values.clear()
}
get length() {
return this.values.size
}
key(index: number) {
return Array.from(this.values.keys())[index] ?? null
}
getItem(key: string) {
this.calls.get += 1
this.events.push(`get:${key}`)
if (key.startsWith("opencode.throw")) throw new Error("storage get failed")
return this.values.get(key) ?? null
}
setItem(key: string, value: string) {
this.calls.set += 1
this.events.push(`set:${key}`)
if (key.startsWith("opencode.quota")) throw new DOMException("quota", "QuotaExceededError")
if (key.startsWith("opencode.throw")) throw new Error("storage set failed")
this.values.set(key, value)
}
removeItem(key: string) {
this.calls.remove += 1
this.events.push(`remove:${key}`)
if (key.startsWith("opencode.throw")) throw new Error("storage remove failed")
this.values.delete(key)
}
}
const storage = new MemoryStorage()
let persistTesting: PersistTestingType
beforeAll(async () => {
mock.module("@/context/platform", () => ({
usePlatform: () => ({ platform: "web" }),
}))
const mod = await import("./persist")
persistTesting = mod.PersistTesting
})
beforeEach(() => {
storage.clear()
storage.events.length = 0
storage.calls.get = 0
storage.calls.set = 0
storage.calls.remove = 0
Object.defineProperty(globalThis, "localStorage", {
value: storage,
configurable: true,
})
})
describe("persist localStorage resilience", () => {
test("does not cache values as persisted when quota write and eviction fail", () => {
const storageApi = persistTesting.localStorageWithPrefix("opencode.quota.scope")
storageApi.setItem("value", '{"value":1}')
expect(storage.getItem("opencode.quota.scope:value")).toBeNull()
expect(storageApi.getItem("value")).toBeNull()
})
test("disables only the failing scope when storage throws", () => {
const bad = persistTesting.localStorageWithPrefix("opencode.throw.scope")
bad.setItem("value", '{"value":1}')
const before = storage.calls.set
bad.setItem("value", '{"value":2}')
expect(storage.calls.set).toBe(before)
expect(bad.getItem("value")).toBeNull()
const healthy = persistTesting.localStorageWithPrefix("opencode.safe.scope")
healthy.setItem("value", '{"value":3}')
expect(storage.getItem("opencode.safe.scope:value")).toBe('{"value":3}')
})
test("failing fallback scope does not poison direct storage scope", () => {
const broken = persistTesting.localStorageWithPrefix("opencode.throw.scope2")
broken.setItem("value", '{"value":1}')
const direct = persistTesting.localStorageDirect()
direct.setItem("direct-value", '{"value":5}')
expect(storage.getItem("direct-value")).toBe('{"value":5}')
})
})

View File

@@ -17,7 +17,7 @@ type PersistTarget = {
const LEGACY_STORAGE = "default.dat"
const GLOBAL_STORAGE = "opencode.global.dat"
const LOCAL_PREFIX = "opencode."
const fallback = { disabled: false }
const fallback = new Map<string, boolean>()
const CACHE_MAX_ENTRIES = 500
const CACHE_MAX_BYTES = 8 * 1024 * 1024
@@ -65,6 +65,14 @@ function cacheGet(key: string) {
return entry.value
}
function fallbackDisabled(scope: string) {
return fallback.get(scope) === true
}
function fallbackSet(scope: string) {
fallback.set(scope, true)
}
function quota(error: unknown) {
if (error instanceof DOMException) {
if (error.name === "QuotaExceededError") return true
@@ -142,7 +150,6 @@ function write(storage: Storage, key: string, value: string) {
}
const ok = evict(storage, key, value)
if (!ok) cacheSet(key, value)
return ok
}
@@ -196,18 +203,19 @@ function workspaceStorage(dir: string) {
function localStorageWithPrefix(prefix: string): SyncStorage {
const base = `${prefix}:`
const scope = `prefix:${prefix}`
const item = (key: string) => base + key
return {
getItem: (key) => {
const name = item(key)
const cached = cacheGet(name)
if (fallback.disabled && cached !== undefined) return cached
if (fallbackDisabled(scope)) return cached ?? null
const stored = (() => {
try {
return localStorage.getItem(name)
} catch {
fallback.disabled = true
fallbackSet(scope)
return null
}
})()
@@ -217,40 +225,40 @@ function localStorageWithPrefix(prefix: string): SyncStorage {
},
setItem: (key, value) => {
const name = item(key)
cacheSet(name, value)
if (fallback.disabled) return
if (fallbackDisabled(scope)) return
try {
if (write(localStorage, name, value)) return
} catch {
fallback.disabled = true
fallbackSet(scope)
return
}
fallback.disabled = true
fallbackSet(scope)
},
removeItem: (key) => {
const name = item(key)
cacheDelete(name)
if (fallback.disabled) return
if (fallbackDisabled(scope)) return
try {
localStorage.removeItem(name)
} catch {
fallback.disabled = true
fallbackSet(scope)
}
},
}
}
function localStorageDirect(): SyncStorage {
const scope = "direct"
return {
getItem: (key) => {
const cached = cacheGet(key)
if (fallback.disabled && cached !== undefined) return cached
if (fallbackDisabled(scope)) return cached ?? null
const stored = (() => {
try {
return localStorage.getItem(key)
} catch {
fallback.disabled = true
fallbackSet(scope)
return null
}
})()
@@ -259,28 +267,32 @@ function localStorageDirect(): SyncStorage {
return stored
},
setItem: (key, value) => {
cacheSet(key, value)
if (fallback.disabled) return
if (fallbackDisabled(scope)) return
try {
if (write(localStorage, key, value)) return
} catch {
fallback.disabled = true
fallbackSet(scope)
return
}
fallback.disabled = true
fallbackSet(scope)
},
removeItem: (key) => {
cacheDelete(key)
if (fallback.disabled) return
if (fallbackDisabled(scope)) return
try {
localStorage.removeItem(key)
} catch {
fallback.disabled = true
fallbackSet(scope)
}
},
}
}
export const PersistTesting = {
localStorageDirect,
localStorageWithPrefix,
}
export const Persist = {
global(key: string, legacy?: string[]): PersistTarget {
return { storage: GLOBAL_STORAGE, key, legacy }

View File

@@ -1,6 +1,12 @@
import { describe, expect, test } from "bun:test"
import { checkServerHealth } from "./server-health"
function abortFromInput(input: RequestInfo | URL, init?: RequestInit) {
if (init?.signal) return init.signal
if (input instanceof Request) return input.signal
return undefined
}
describe("checkServerHealth", () => {
test("returns healthy response with version", async () => {
const fetch = (async () =>
@@ -24,10 +30,40 @@ describe("checkServerHealth", () => {
expect(result).toEqual({ healthy: false })
})
test("uses timeout fallback when AbortSignal.timeout is unavailable", async () => {
const timeout = Object.getOwnPropertyDescriptor(AbortSignal, "timeout")
Object.defineProperty(AbortSignal, "timeout", {
configurable: true,
value: undefined,
})
let aborted = false
const fetch = ((input: RequestInfo | URL, init?: RequestInit) =>
new Promise<Response>((_resolve, reject) => {
const signal = abortFromInput(input, init)
signal?.addEventListener(
"abort",
() => {
aborted = true
reject(new DOMException("Aborted", "AbortError"))
},
{ once: true },
)
})) as unknown as typeof globalThis.fetch
const result = await checkServerHealth("http://localhost:4096", fetch, { timeoutMs: 10 }).finally(() => {
if (timeout) Object.defineProperty(AbortSignal, "timeout", timeout)
if (!timeout) Reflect.deleteProperty(AbortSignal, "timeout")
})
expect(aborted).toBe(true)
expect(result).toEqual({ healthy: false })
})
test("uses provided abort signal", async () => {
let signal: AbortSignal | undefined
const fetch = (async (input: RequestInfo | URL, init?: RequestInit) => {
signal = init?.signal ?? (input instanceof Request ? input.signal : undefined)
signal = abortFromInput(input, init)
return new Response(JSON.stringify({ healthy: true, version: "1.2.3" }), {
status: 200,
headers: { "content-type": "application/json" },
@@ -39,4 +75,40 @@ describe("checkServerHealth", () => {
expect(signal).toBe(abort.signal)
})
test("retries transient failures and eventually succeeds", async () => {
let count = 0
const fetch = (async () => {
count += 1
if (count < 3) throw new TypeError("network")
return new Response(JSON.stringify({ healthy: true, version: "1.2.3" }), {
status: 200,
headers: { "content-type": "application/json" },
})
}) as unknown as typeof globalThis.fetch
const result = await checkServerHealth("http://localhost:4096", fetch, {
retryCount: 2,
retryDelayMs: 1,
})
expect(count).toBe(3)
expect(result).toEqual({ healthy: true, version: "1.2.3" })
})
test("returns unhealthy when retries are exhausted", async () => {
let count = 0
const fetch = (async () => {
count += 1
throw new TypeError("network")
}) as unknown as typeof globalThis.fetch
const result = await checkServerHealth("http://localhost:4096", fetch, {
retryCount: 2,
retryDelayMs: 1,
})
expect(count).toBe(3)
expect(result).toEqual({ healthy: false })
})
})

View File

@@ -5,10 +5,50 @@ export type ServerHealth = { healthy: boolean; version?: string }
interface CheckServerHealthOptions {
timeoutMs?: number
signal?: AbortSignal
retryCount?: number
retryDelayMs?: number
}
const defaultTimeoutMs = 3000
const defaultRetryCount = 2
const defaultRetryDelayMs = 100
function timeoutSignal(timeoutMs: number) {
return (AbortSignal as unknown as { timeout?: (ms: number) => AbortSignal }).timeout?.(timeoutMs)
const timeout = (AbortSignal as unknown as { timeout?: (ms: number) => AbortSignal }).timeout
if (timeout) {
try {
return { signal: timeout.call(AbortSignal, timeoutMs), clear: undefined as (() => void) | undefined }
} catch {}
}
const controller = new AbortController()
const timer = setTimeout(() => controller.abort(), timeoutMs)
return { signal: controller.signal, clear: () => clearTimeout(timer) }
}
function wait(ms: number, signal?: AbortSignal) {
return new Promise<void>((resolve, reject) => {
if (signal?.aborted) {
reject(new DOMException("Aborted", "AbortError"))
return
}
const timer = setTimeout(() => {
signal?.removeEventListener("abort", onAbort)
resolve()
}, ms)
const onAbort = () => {
clearTimeout(timer)
reject(new DOMException("Aborted", "AbortError"))
}
signal?.addEventListener("abort", onAbort, { once: true })
})
}
function retryable(error: unknown, signal?: AbortSignal) {
if (signal?.aborted) return false
if (!(error instanceof Error)) return false
if (error.name === "AbortError" || error.name === "TimeoutError") return false
if (error instanceof TypeError) return true
return /network|fetch|econnreset|econnrefused|enotfound|timedout/i.test(error.message)
}
export async function checkServerHealth(
@@ -16,14 +56,24 @@ export async function checkServerHealth(
fetch: typeof globalThis.fetch,
opts?: CheckServerHealthOptions,
): Promise<ServerHealth> {
const signal = opts?.signal ?? timeoutSignal(opts?.timeoutMs ?? 3000)
const sdk = createOpencodeClient({
baseUrl: url,
fetch,
signal,
})
return sdk.global
.health()
.then((x) => ({ healthy: x.data?.healthy === true, version: x.data?.version }))
.catch(() => ({ healthy: false }))
const timeout = opts?.signal ? undefined : timeoutSignal(opts?.timeoutMs ?? defaultTimeoutMs)
const signal = opts?.signal ?? timeout?.signal
const retryCount = opts?.retryCount ?? defaultRetryCount
const retryDelayMs = opts?.retryDelayMs ?? defaultRetryDelayMs
const next = (count: number, error: unknown) => {
if (count >= retryCount || !retryable(error, signal)) return Promise.resolve({ healthy: false } as const)
return wait(retryDelayMs * (count + 1), signal)
.then(() => attempt(count + 1))
.catch(() => ({ healthy: false }))
}
const attempt = (count: number): Promise<ServerHealth> =>
createOpencodeClient({
baseUrl: url,
fetch,
signal,
})
.global.health()
.then((x) => (x.error ? next(count, x.error) : { healthy: x.data?.healthy === true, version: x.data?.version }))
.catch((error) => next(count, error))
return attempt(0).finally(() => timeout?.clear?.())
}