From 9d76e667f3cbac0146d3ee64c0f3bb5ee80d47f5 Mon Sep 17 00:00:00 2001 From: Kentaro Wakayama Date: Tue, 4 Aug 2026 16:39:49 +0200 Subject: [PATCH 1/3] fix(server): log server-side agent stream failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A VeryfrontError returned from the internal agent stream handler was converted to a response and returned without ever being logged. Only the generic non-VeryfrontError fallback below it logs. errorToResponse deliberately strips `detail` from 5xx bodies so internals never reach the caller (http-error.ts). Correct for the client, but the handler kept no record either — so a server-side failure left no detail in the response, none in the logs, and nothing in Sentry. On staging every preview-environment agent run returns 'Initialization failed' with a bare status code and no way to tell which precondition failed; the runtime has three distinct throws behind that one title. Log slug, category, detail and cause for 5xx on that branch. The response body is unchanged, so nothing new is exposed to the caller. 4xx stays quiet: those are client errors and already carry their detail. Refs veryfront-issue-inbox#356 --- .../handlers/request/agent-stream.handler.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/server/handlers/request/agent-stream.handler.ts b/src/server/handlers/request/agent-stream.handler.ts index f1c21b5076..5c89aa782f 100644 --- a/src/server/handlers/request/agent-stream.handler.ts +++ b/src/server/handlers/request/agent-stream.handler.ts @@ -942,6 +942,22 @@ export class AgentStreamHandler extends BaseHandler { if (isVeryfrontError(error)) { const response = errorToResponse(error, new URL(req.url).pathname); + // errorToResponse strips `detail` from 5xx bodies so internals never + // reach the caller. Nothing else on this branch logs, so without this + // the only record of a server-side failure is a bare status code — + // no detail in the response, none in the logs, none in Sentry. + if (response.status >= 500) { + logger.error("Internal agent stream request failed", { + projectId: ctx.projectId, + projectSlug: ctx.projectSlug, + status: response.status, + slug: error.slug, + category: error.category, + detail: error.detail, + error: error.message, + cause: error.cause instanceof Error ? error.cause.message : undefined, + }); + } return this.respond(applyBuilderHeaders(response, builder.headers)); } From 8a2bd85dcf87e0036948b7dd3f2a183a722f1650 Mon Sep 17 00:00:00 2001 From: Kentaro Wakayama Date: Tue, 4 Aug 2026 16:48:37 +0200 Subject: [PATCH 2/3] fix(server): capture string causes and cover the 5xx log branch Review follow-ups on #3359. VeryfrontError.cause is typed unknown and is frequently a plain string, so the instanceof Error check dropped exactly the provenance this change exists to surface. describeErrorCause handles Error, string and other values. Adds the test both reviewers asked for. The earlier attempt landed on the fallback branch; injecting through ensureProjectDiscovery reaches the isVeryfrontError branch. It asserts the detail, the string cause and the slug are logged, and that the response body still omits the detail. Verified it fails when the branch is disabled. --- .../request/agent-stream.handler.test.ts | 74 +++++++++++++++++++ .../handlers/request/agent-stream.handler.ts | 15 ++-- 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/src/server/handlers/request/agent-stream.handler.test.ts b/src/server/handlers/request/agent-stream.handler.test.ts index 6141a926cc..e4031aa820 100644 --- a/src/server/handlers/request/agent-stream.handler.test.ts +++ b/src/server/handlers/request/agent-stream.handler.test.ts @@ -1,4 +1,11 @@ import "#veryfront/schemas/_test-setup.ts"; +import { SERVICE_OVERLOADED } from "#veryfront/errors"; +import { + __registerLogRecordEmitter, + __resetLogRecordEmitterForTests, + type LogEntry, + refreshLoggerConfig, +} from "#veryfront/utils/logger/index.ts"; import { createEmptyDiscoveryResult } from "#veryfront/discovery"; import type { AgentMessage } from "#veryfront/agent"; import { AgentRunSessionManager } from "#veryfront/internal-agents/session-manager.ts"; @@ -3396,3 +3403,70 @@ describe("server/handlers/request/agent-stream.handler", () => { } }); }); + +describe("agent stream handler 5xx logging", () => { + it("logs slug, category, detail and cause for a 5xx VeryfrontError", async () => { + const entries: LogEntry[] = []; + const previousLogLevel = Deno.env.get("LOG_LEVEL"); + const detail = "Agent service context has not been initialized."; + + try { + Deno.env.set("LOG_LEVEL", "DEBUG"); + refreshLoggerConfig(); + __registerLogRecordEmitter((entry) => entries.push(entry)); + + const handler = createTestAgentStreamHandler({ + ensureProjectDiscovery: () => { + throw SERVICE_OVERLOADED.create({ detail, cause: "adapter boot failed" }); + }, + getAgent: () => undefined, + getAllAgentIds: () => [], + sessionManager: new AgentRunSessionManager(), + }); + + const body = createAgentStreamRequestBody({ + agentSource: { type: "branch", branch: "main" }, + }); + const { jws, publicKeyPem } = await createControlPlaneSignature(body, { + requestId: "run_1", + }); + + const result = await handler.handle( + new Request("https://example.com/api/control-plane/runs/run_1/stream", { + method: "POST", + headers: { + "content-type": "application/json", + "x-veryfront-control-plane-jws": jws, + }, + body, + }), + createCtx(publicKeyPem), + ); + + assertExists(result.response); + assertEquals(result.response.status >= 500, true, `got ${result.response.status}`); + + // The caller still must not see the detail. + const payload = await result.response.json(); + assertEquals(payload.detail, undefined); + + const logged = entries.find( + (entry) => entry.message === "Internal agent stream request failed", + ); + assertExists( + logged, + `handler did not log; saw: ${entries.map((e) => e.message).join(" | ")}`, + ); + const serialized = JSON.stringify(logged); + assertStringIncludes(serialized, detail); + // cause is typed `unknown` and is frequently a string, not an Error. + assertStringIncludes(serialized, "adapter boot failed"); + assertStringIncludes(serialized, "service-overloaded"); + } finally { + __resetLogRecordEmitterForTests(); + if (previousLogLevel === undefined) Deno.env.delete("LOG_LEVEL"); + else Deno.env.set("LOG_LEVEL", previousLogLevel); + refreshLoggerConfig(); + } + }); +}); diff --git a/src/server/handlers/request/agent-stream.handler.ts b/src/server/handlers/request/agent-stream.handler.ts index 5c89aa782f..319623d6a4 100644 --- a/src/server/handlers/request/agent-stream.handler.ts +++ b/src/server/handlers/request/agent-stream.handler.ts @@ -109,6 +109,13 @@ const defaultDeps: AgentStreamHandlerDeps = { getDiscoveredHostTools({ agentId }) as RuntimeAgentStreamExecutionDeps["localTools"], }; const logger = serverLogger.component("agent-stream-handler"); + +/** VeryfrontError.cause is `unknown` and is often a plain string. */ +function describeErrorCause(cause: unknown): string | undefined { + if (cause === undefined || cause === null) return undefined; + if (cause instanceof Error) return cause.message; + return typeof cause === "string" ? cause : String(cause); +} const RUN_STREAM_PATH_REGEX = /^\/api\/control-plane\/runs\/([^/]+)\/stream$/; const STUDIO_RUNTIME_REMOTE_TOOL_NAMES = new Set( [ @@ -942,10 +949,8 @@ export class AgentStreamHandler extends BaseHandler { if (isVeryfrontError(error)) { const response = errorToResponse(error, new URL(req.url).pathname); - // errorToResponse strips `detail` from 5xx bodies so internals never - // reach the caller. Nothing else on this branch logs, so without this - // the only record of a server-side failure is a bare status code — - // no detail in the response, none in the logs, none in Sentry. + // errorToResponse strips `detail` from 5xx bodies, and this branch is + // otherwise silent, so the detail would be lost entirely. if (response.status >= 500) { logger.error("Internal agent stream request failed", { projectId: ctx.projectId, @@ -955,7 +960,7 @@ export class AgentStreamHandler extends BaseHandler { category: error.category, detail: error.detail, error: error.message, - cause: error.cause instanceof Error ? error.cause.message : undefined, + cause: describeErrorCause(error.cause), }); } return this.respond(applyBuilderHeaders(response, builder.headers)); From 6a588602473d69ea520d7a4ad8fb3fb2c673a46b Mon Sep 17 00:00:00 2001 From: Kentaro Wakayama Date: Tue, 4 Aug 2026 16:58:33 +0200 Subject: [PATCH 3/3] test(server): assert the logged category, not just the slug Review follow-up: the assertion checked the slug value but never verified that error.category reaches the log record. Hold the thrown error in a local and assert both fields against it. --- src/server/handlers/request/agent-stream.handler.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/server/handlers/request/agent-stream.handler.test.ts b/src/server/handlers/request/agent-stream.handler.test.ts index e4031aa820..5dc70198ee 100644 --- a/src/server/handlers/request/agent-stream.handler.test.ts +++ b/src/server/handlers/request/agent-stream.handler.test.ts @@ -3409,6 +3409,7 @@ describe("agent stream handler 5xx logging", () => { const entries: LogEntry[] = []; const previousLogLevel = Deno.env.get("LOG_LEVEL"); const detail = "Agent service context has not been initialized."; + const thrown = SERVICE_OVERLOADED.create({ detail, cause: "adapter boot failed" }); try { Deno.env.set("LOG_LEVEL", "DEBUG"); @@ -3417,7 +3418,7 @@ describe("agent stream handler 5xx logging", () => { const handler = createTestAgentStreamHandler({ ensureProjectDiscovery: () => { - throw SERVICE_OVERLOADED.create({ detail, cause: "adapter boot failed" }); + throw thrown; }, getAgent: () => undefined, getAllAgentIds: () => [], @@ -3461,7 +3462,8 @@ describe("agent stream handler 5xx logging", () => { assertStringIncludes(serialized, detail); // cause is typed `unknown` and is frequently a string, not an Error. assertStringIncludes(serialized, "adapter boot failed"); - assertStringIncludes(serialized, "service-overloaded"); + assertStringIncludes(serialized, thrown.slug); + assertStringIncludes(serialized, thrown.category); } finally { __resetLogRecordEmitterForTests(); if (previousLogLevel === undefined) Deno.env.delete("LOG_LEVEL");