mirror of
https://github.com/anomalyco/opencode.git
synced 2026-05-02 10:46:46 +00:00
refactor(server): simplify router middleware with next() (#21720)
This commit is contained in:
49
packages/opencode/test/memory/abort-leak-webfetch.ts
Normal file
49
packages/opencode/test/memory/abort-leak-webfetch.ts
Normal file
@@ -0,0 +1,49 @@
|
||||
import { abortAfterAny } from "../../src/util/abort"
|
||||
|
||||
const MB = 1024 * 1024
|
||||
const ITERATIONS = 50
|
||||
|
||||
const heap = () => {
|
||||
Bun.gc(true)
|
||||
return process.memoryUsage().heapUsed / MB
|
||||
}
|
||||
|
||||
const server = Bun.serve({
|
||||
port: 0,
|
||||
fetch() {
|
||||
return new Response("hello from local", {
|
||||
headers: {
|
||||
"content-type": "text/plain",
|
||||
},
|
||||
})
|
||||
},
|
||||
})
|
||||
|
||||
const url = `http://127.0.0.1:${server.port}`
|
||||
|
||||
async function run() {
|
||||
const { signal, clearTimeout } = abortAfterAny(30000, new AbortController().signal)
|
||||
try {
|
||||
const response = await fetch(url, { signal })
|
||||
await response.text()
|
||||
} finally {
|
||||
clearTimeout()
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
await run()
|
||||
Bun.sleepSync(100)
|
||||
const baseline = heap()
|
||||
|
||||
for (let i = 0; i < ITERATIONS; i++) {
|
||||
await run()
|
||||
}
|
||||
|
||||
Bun.sleepSync(100)
|
||||
const after = heap()
|
||||
process.stdout.write(JSON.stringify({ baseline, after, growth: after - baseline }))
|
||||
} finally {
|
||||
server.stop(true)
|
||||
process.exit(0)
|
||||
}
|
||||
127
packages/opencode/test/memory/abort-leak.test.ts
Normal file
127
packages/opencode/test/memory/abort-leak.test.ts
Normal file
@@ -0,0 +1,127 @@
|
||||
import { describe, test, expect } from "bun:test"
|
||||
import path from "path"
|
||||
|
||||
const projectRoot = path.join(import.meta.dir, "../..")
|
||||
const worker = path.join(import.meta.dir, "abort-leak-webfetch.ts")
|
||||
|
||||
const MB = 1024 * 1024
|
||||
const ITERATIONS = 50
|
||||
|
||||
const getHeapMB = () => {
|
||||
Bun.gc(true)
|
||||
return process.memoryUsage().heapUsed / MB
|
||||
}
|
||||
|
||||
describe("memory: abort controller leak", () => {
|
||||
test("webfetch does not leak memory over many invocations", async () => {
|
||||
// Measure the abort-timed fetch path in a fresh process so shared tool
|
||||
// runtime state does not dominate the heap signal.
|
||||
const proc = Bun.spawn({
|
||||
cmd: [process.execPath, worker],
|
||||
cwd: projectRoot,
|
||||
stdout: "pipe",
|
||||
stderr: "pipe",
|
||||
env: process.env,
|
||||
})
|
||||
|
||||
const [code, stdout, stderr] = await Promise.all([
|
||||
proc.exited,
|
||||
new Response(proc.stdout).text(),
|
||||
new Response(proc.stderr).text(),
|
||||
])
|
||||
|
||||
if (code !== 0) {
|
||||
throw new Error(stderr.trim() || stdout.trim() || `worker exited with code ${code}`)
|
||||
}
|
||||
|
||||
const result = JSON.parse(stdout.trim()) as {
|
||||
baseline: number
|
||||
after: number
|
||||
growth: number
|
||||
}
|
||||
|
||||
console.log(`Baseline: ${result.baseline.toFixed(2)} MB`)
|
||||
console.log(`After ${ITERATIONS} fetches: ${result.after.toFixed(2)} MB`)
|
||||
console.log(`Growth: ${result.growth.toFixed(2)} MB`)
|
||||
|
||||
// Memory growth should be minimal - less than 1MB per 10 requests.
|
||||
expect(result.growth).toBeLessThan(ITERATIONS / 10)
|
||||
}, 60000)
|
||||
|
||||
test("compare closure vs bind pattern directly", async () => {
|
||||
const ITERATIONS = 500
|
||||
|
||||
// Test OLD pattern: arrow function closure
|
||||
// Store closures in a map keyed by content to force retention
|
||||
const closureMap = new Map<string, () => void>()
|
||||
const timers: Timer[] = []
|
||||
const controllers: AbortController[] = []
|
||||
|
||||
Bun.gc(true)
|
||||
Bun.sleepSync(100)
|
||||
const baseline = getHeapMB()
|
||||
|
||||
for (let i = 0; i < ITERATIONS; i++) {
|
||||
// Simulate large response body like webfetch would have
|
||||
const content = `${i}:${"x".repeat(50 * 1024)}` // 50KB unique per iteration
|
||||
const controller = new AbortController()
|
||||
controllers.push(controller)
|
||||
|
||||
// OLD pattern - closure captures `content`
|
||||
const handler = () => {
|
||||
// Actually use content so it can't be optimized away
|
||||
if (content.length > 1000000000) controller.abort()
|
||||
}
|
||||
closureMap.set(content, handler)
|
||||
const timeoutId = setTimeout(handler, 30000)
|
||||
timers.push(timeoutId)
|
||||
}
|
||||
|
||||
Bun.gc(true)
|
||||
Bun.sleepSync(100)
|
||||
const after = getHeapMB()
|
||||
const oldGrowth = after - baseline
|
||||
|
||||
console.log(`OLD pattern (closure): ${oldGrowth.toFixed(2)} MB growth (${closureMap.size} closures)`)
|
||||
|
||||
// Cleanup after measuring
|
||||
timers.forEach(clearTimeout)
|
||||
controllers.forEach((c) => c.abort())
|
||||
closureMap.clear()
|
||||
|
||||
// Test NEW pattern: bind
|
||||
Bun.gc(true)
|
||||
Bun.sleepSync(100)
|
||||
const baseline2 = getHeapMB()
|
||||
const handlers2: (() => void)[] = []
|
||||
const timers2: Timer[] = []
|
||||
const controllers2: AbortController[] = []
|
||||
|
||||
for (let i = 0; i < ITERATIONS; i++) {
|
||||
const _content = `${i}:${"x".repeat(50 * 1024)}` // 50KB - won't be captured
|
||||
const controller = new AbortController()
|
||||
controllers2.push(controller)
|
||||
|
||||
// NEW pattern - bind doesn't capture surrounding scope
|
||||
const handler = controller.abort.bind(controller)
|
||||
handlers2.push(handler)
|
||||
const timeoutId = setTimeout(handler, 30000)
|
||||
timers2.push(timeoutId)
|
||||
}
|
||||
|
||||
Bun.gc(true)
|
||||
Bun.sleepSync(100)
|
||||
const after2 = getHeapMB()
|
||||
const newGrowth = after2 - baseline2
|
||||
|
||||
// Cleanup after measuring
|
||||
timers2.forEach(clearTimeout)
|
||||
controllers2.forEach((c) => c.abort())
|
||||
handlers2.length = 0
|
||||
|
||||
console.log(`NEW pattern (bind): ${newGrowth.toFixed(2)} MB growth`)
|
||||
console.log(`Improvement: ${(oldGrowth - newGrowth).toFixed(2)} MB saved`)
|
||||
|
||||
expect(newGrowth).toBeLessThanOrEqual(oldGrowth)
|
||||
})
|
||||
})
|
||||
@@ -13,8 +13,6 @@ const { PluginLoader } = await import("../../src/plugin/loader")
|
||||
const { readPackageThemes } = await import("../../src/plugin/shared")
|
||||
const { Instance } = await import("../../src/project/instance")
|
||||
const { Npm } = await import("../../src/npm")
|
||||
const { Bus } = await import("../../src/bus")
|
||||
const { Session } = await import("../../src/session")
|
||||
|
||||
afterAll(() => {
|
||||
if (disableDefault === undefined) {
|
||||
@@ -37,27 +35,6 @@ async function load(dir: string) {
|
||||
})
|
||||
}
|
||||
|
||||
async function errs(dir: string) {
|
||||
return Instance.provide({
|
||||
directory: dir,
|
||||
fn: async () => {
|
||||
const errors: string[] = []
|
||||
const off = Bus.subscribe(Session.Event.Error, (evt) => {
|
||||
const error = evt.properties.error
|
||||
if (!error || typeof error !== "object") return
|
||||
if (!("data" in error)) return
|
||||
if (!error.data || typeof error.data !== "object") return
|
||||
if (!("message" in error.data)) return
|
||||
if (typeof error.data.message !== "string") return
|
||||
errors.push(error.data.message)
|
||||
})
|
||||
await Plugin.list()
|
||||
off()
|
||||
return errors
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
describe("plugin.loader.shared", () => {
|
||||
test("loads a file:// plugin function export", async () => {
|
||||
await using tmp = await tmpdir({
|
||||
@@ -184,14 +161,13 @@ describe("plugin.loader.shared", () => {
|
||||
},
|
||||
})
|
||||
|
||||
const errors = await errs(tmp.path)
|
||||
await load(tmp.path)
|
||||
const called = await Bun.file(tmp.extra.mark)
|
||||
.text()
|
||||
.then(() => true)
|
||||
.catch(() => false)
|
||||
|
||||
expect(called).toBe(false)
|
||||
expect(errors.some((x) => x.includes("must export id"))).toBe(true)
|
||||
})
|
||||
|
||||
test("rejects v1 plugin that exports server and tui together", async () => {
|
||||
@@ -223,14 +199,13 @@ describe("plugin.loader.shared", () => {
|
||||
},
|
||||
})
|
||||
|
||||
const errors = await errs(tmp.path)
|
||||
await load(tmp.path)
|
||||
const called = await Bun.file(tmp.extra.mark)
|
||||
.text()
|
||||
.then(() => true)
|
||||
.catch(() => false)
|
||||
|
||||
expect(called).toBe(false)
|
||||
expect(errors.some((x) => x.includes("either server() or tui(), not both"))).toBe(true)
|
||||
})
|
||||
|
||||
test("resolves npm plugin specs with explicit and default versions", async () => {
|
||||
@@ -383,8 +358,7 @@ describe("plugin.loader.shared", () => {
|
||||
const install = spyOn(Npm, "add").mockResolvedValue({ directory: tmp.extra.mod, entrypoint: tmp.extra.mod })
|
||||
|
||||
try {
|
||||
const errors = await errs(tmp.path)
|
||||
expect(errors).toHaveLength(0)
|
||||
await load(tmp.path)
|
||||
expect(await Bun.file(tmp.extra.mark).text()).toBe("called")
|
||||
} finally {
|
||||
install.mockRestore()
|
||||
@@ -436,8 +410,7 @@ describe("plugin.loader.shared", () => {
|
||||
const install = spyOn(Npm, "add").mockResolvedValue({ directory: tmp.extra.mod, entrypoint: tmp.extra.mod })
|
||||
|
||||
try {
|
||||
const errors = await errs(tmp.path)
|
||||
expect(errors).toHaveLength(0)
|
||||
await load(tmp.path)
|
||||
expect(await Bun.file(tmp.extra.mark).text()).toBe("called")
|
||||
} finally {
|
||||
install.mockRestore()
|
||||
@@ -482,14 +455,13 @@ describe("plugin.loader.shared", () => {
|
||||
const install = spyOn(Npm, "add").mockResolvedValue({ directory: tmp.extra.mod, entrypoint: tmp.extra.mod })
|
||||
|
||||
try {
|
||||
const errors = await errs(tmp.path)
|
||||
await load(tmp.path)
|
||||
const called = await Bun.file(tmp.extra.mark)
|
||||
.text()
|
||||
.then(() => true)
|
||||
.catch(() => false)
|
||||
|
||||
expect(called).toBe(false)
|
||||
expect(errors).toHaveLength(0)
|
||||
} finally {
|
||||
install.mockRestore()
|
||||
}
|
||||
@@ -546,13 +518,12 @@ describe("plugin.loader.shared", () => {
|
||||
const install = spyOn(Npm, "add").mockResolvedValue({ directory: tmp.extra.mod, entrypoint: tmp.extra.mod })
|
||||
|
||||
try {
|
||||
const errors = await errs(tmp.path)
|
||||
await load(tmp.path)
|
||||
const called = await Bun.file(tmp.extra.mark)
|
||||
.text()
|
||||
.then(() => true)
|
||||
.catch(() => false)
|
||||
expect(called).toBe(false)
|
||||
expect(errors.some((x) => x.includes("outside plugin directory"))).toBe(true)
|
||||
} finally {
|
||||
install.mockRestore()
|
||||
}
|
||||
@@ -588,30 +559,49 @@ describe("plugin.loader.shared", () => {
|
||||
}
|
||||
})
|
||||
|
||||
test("publishes session.error when install fails", async () => {
|
||||
test("skips broken plugin when install fails", async () => {
|
||||
await using tmp = await tmpdir({
|
||||
init: async (dir) => {
|
||||
await Bun.write(path.join(dir, "opencode.json"), JSON.stringify({ plugin: ["broken-plugin@9.9.9"] }, null, 2))
|
||||
const ok = path.join(dir, "ok.ts")
|
||||
const mark = path.join(dir, "ok.txt")
|
||||
await Bun.write(
|
||||
ok,
|
||||
[
|
||||
"export default {",
|
||||
' id: "demo.ok",',
|
||||
" server: async () => {",
|
||||
` await Bun.write(${JSON.stringify(mark)}, "ok")`,
|
||||
" return {}",
|
||||
" },",
|
||||
"}",
|
||||
"",
|
||||
].join("\n"),
|
||||
)
|
||||
await Bun.write(
|
||||
path.join(dir, "opencode.json"),
|
||||
JSON.stringify({ plugin: ["broken-plugin@9.9.9", pathToFileURL(ok).href] }, null, 2),
|
||||
)
|
||||
return { mark }
|
||||
},
|
||||
})
|
||||
|
||||
const install = spyOn(Npm, "add").mockRejectedValue(new Error("boom"))
|
||||
|
||||
try {
|
||||
const errors = await errs(tmp.path)
|
||||
|
||||
expect(errors.some((x) => x.includes("Failed to install plugin broken-plugin@9.9.9") && x.includes("boom"))).toBe(
|
||||
true,
|
||||
)
|
||||
await load(tmp.path)
|
||||
expect(install).toHaveBeenCalledWith("broken-plugin@9.9.9")
|
||||
expect(await Bun.file(tmp.extra.mark).text()).toBe("ok")
|
||||
} finally {
|
||||
install.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
test("publishes session.error when plugin init throws", async () => {
|
||||
test("continues loading plugins when plugin init throws", async () => {
|
||||
await using tmp = await tmpdir({
|
||||
init: async (dir) => {
|
||||
const file = pathToFileURL(path.join(dir, "throws.ts")).href
|
||||
const ok = pathToFileURL(path.join(dir, "ok.ts")).href
|
||||
const mark = path.join(dir, "ok.txt")
|
||||
await Bun.write(
|
||||
path.join(dir, "throws.ts"),
|
||||
[
|
||||
@@ -624,51 +614,91 @@ describe("plugin.loader.shared", () => {
|
||||
"",
|
||||
].join("\n"),
|
||||
)
|
||||
await Bun.write(
|
||||
path.join(dir, "ok.ts"),
|
||||
[
|
||||
"export default {",
|
||||
' id: "demo.ok",',
|
||||
" server: async () => {",
|
||||
` await Bun.write(${JSON.stringify(mark)}, "ok")`,
|
||||
" return {}",
|
||||
" },",
|
||||
"}",
|
||||
"",
|
||||
].join("\n"),
|
||||
)
|
||||
|
||||
await Bun.write(path.join(dir, "opencode.json"), JSON.stringify({ plugin: [file] }, null, 2))
|
||||
await Bun.write(path.join(dir, "opencode.json"), JSON.stringify({ plugin: [file, ok] }, null, 2))
|
||||
|
||||
return { file }
|
||||
return { mark }
|
||||
},
|
||||
})
|
||||
|
||||
const errors = await errs(tmp.path)
|
||||
|
||||
expect(errors.some((x) => x.includes(`Failed to load plugin ${tmp.extra.file}: explode`))).toBe(true)
|
||||
await load(tmp.path)
|
||||
expect(await Bun.file(tmp.extra.mark).text()).toBe("ok")
|
||||
})
|
||||
|
||||
test("publishes session.error when plugin module has invalid export", async () => {
|
||||
test("continues loading plugins when plugin module has invalid export", async () => {
|
||||
await using tmp = await tmpdir({
|
||||
init: async (dir) => {
|
||||
const file = pathToFileURL(path.join(dir, "invalid.ts")).href
|
||||
const ok = pathToFileURL(path.join(dir, "ok.ts")).href
|
||||
const mark = path.join(dir, "ok.txt")
|
||||
await Bun.write(
|
||||
path.join(dir, "invalid.ts"),
|
||||
["export default {", ' id: "demo.invalid",', " nope: true,", "}", ""].join("\n"),
|
||||
)
|
||||
await Bun.write(
|
||||
path.join(dir, "ok.ts"),
|
||||
[
|
||||
"export default {",
|
||||
' id: "demo.ok",',
|
||||
" server: async () => {",
|
||||
` await Bun.write(${JSON.stringify(mark)}, "ok")`,
|
||||
" return {}",
|
||||
" },",
|
||||
"}",
|
||||
"",
|
||||
].join("\n"),
|
||||
)
|
||||
|
||||
await Bun.write(path.join(dir, "opencode.json"), JSON.stringify({ plugin: [file] }, null, 2))
|
||||
await Bun.write(path.join(dir, "opencode.json"), JSON.stringify({ plugin: [file, ok] }, null, 2))
|
||||
|
||||
return { file }
|
||||
return { mark }
|
||||
},
|
||||
})
|
||||
|
||||
const errors = await errs(tmp.path)
|
||||
|
||||
expect(errors.some((x) => x.includes(`Failed to load plugin ${tmp.extra.file}`))).toBe(true)
|
||||
await load(tmp.path)
|
||||
expect(await Bun.file(tmp.extra.mark).text()).toBe("ok")
|
||||
})
|
||||
|
||||
test("publishes session.error when plugin import fails", async () => {
|
||||
test("continues loading plugins when plugin import fails", async () => {
|
||||
await using tmp = await tmpdir({
|
||||
init: async (dir) => {
|
||||
const missing = pathToFileURL(path.join(dir, "missing-plugin.ts")).href
|
||||
await Bun.write(path.join(dir, "opencode.json"), JSON.stringify({ plugin: [missing] }, null, 2))
|
||||
const ok = pathToFileURL(path.join(dir, "ok.ts")).href
|
||||
const mark = path.join(dir, "ok.txt")
|
||||
await Bun.write(
|
||||
path.join(dir, "ok.ts"),
|
||||
[
|
||||
"export default {",
|
||||
' id: "demo.ok",',
|
||||
" server: async () => {",
|
||||
` await Bun.write(${JSON.stringify(mark)}, "ok")`,
|
||||
" return {}",
|
||||
" },",
|
||||
"}",
|
||||
"",
|
||||
].join("\n"),
|
||||
)
|
||||
await Bun.write(path.join(dir, "opencode.json"), JSON.stringify({ plugin: [missing, ok] }, null, 2))
|
||||
|
||||
return { missing }
|
||||
return { mark }
|
||||
},
|
||||
})
|
||||
|
||||
const errors = await errs(tmp.path)
|
||||
|
||||
expect(errors.some((x) => x.includes(`Failed to load plugin ${tmp.extra.missing}`))).toBe(true)
|
||||
await load(tmp.path)
|
||||
expect(await Bun.file(tmp.extra.mark).text()).toBe("ok")
|
||||
})
|
||||
|
||||
test("loads object plugin via plugin.server", async () => {
|
||||
|
||||
@@ -147,7 +147,7 @@ describe("session messages endpoint", () => {
|
||||
|
||||
describe("session.prompt_async error handling", () => {
|
||||
test("prompt_async route has error handler for detached prompt call", async () => {
|
||||
const src = await Bun.file(new URL("../../src/server/routes/session.ts", import.meta.url)).text()
|
||||
const src = await Bun.file(new URL("../../src/server/instance/session.ts", import.meta.url)).text()
|
||||
const start = src.indexOf('"/:sessionID/prompt_async"')
|
||||
const end = src.indexOf('"/:sessionID/command"', start)
|
||||
expect(start).toBeGreaterThan(-1)
|
||||
|
||||
Reference in New Issue
Block a user