From e026d4885af5cf8e639c3e7e0f2503e67423ffdd Mon Sep 17 00:00:00 2001 From: Raman Tehlan Date: Thu, 22 Oct 2026 03:40:24 +0530 Subject: [PATCH 01/13] fix: report actionable failures to Sentry Mark infrastructure and unexpected internal errors on the log record so the server logger can send them to Sentry without reporting provider inference errors, bad credentials, or client disconnects. Signed-off-by: Raman Tehlan --- .changeset/sentry-allowlisted-logs.md | 6 ++ .../src/agent-session/SessionHandle.ts | 2 + .../src/agent-session/TurnHandle.ts | 6 +- .../builtins/LargeToolResponse.ts | 6 +- packages/trueforge-core/src/core/index.ts | 3 +- .../src/core/llm/VercelAILLM.ts | 25 ++++++- .../src/core/sandbox/Sandbox.ts | 7 +- .../sandbox/codeMode/CodeModeDispatcher.ts | 8 ++- .../codeMode/nats/CodeModeNatsTransport.ts | 31 ++++++-- .../core/sandbox/provider/DaytonaProvider.ts | 13 +++- .../sandbox/provider/TFYSandboxProvider.ts | 19 ++++- .../src/core/util/errorLogFields.ts | 59 +++++++++++++++ .../src/core/util/promiseUtils.ts | 6 +- .../src/core/util/reportableErrorLog.ts | 18 +++++ .../src/request-reply/executor.ts | 44 +++++++++++- .../tests/core/llm/VercelAILLM.stream.test.ts | 38 ++++++++++ packages/trueforge/src/apis/auth.ts | 21 ++++-- packages/trueforge/src/apis/mcpOAuth.ts | 7 +- packages/trueforge/src/apis/mcpServers.ts | 6 +- packages/trueforge/src/apis/sessions.ts | 12 +++- packages/trueforge/src/apis/turns.ts | 12 +++- packages/trueforge/src/app.ts | 8 ++- packages/trueforge/src/controller.ts | 8 ++- .../trueforge/src/controller/Controller.ts | 7 +- .../src/controller/sandboxEnvBuild.ts | 4 -- packages/trueforge/src/logger.ts | 7 +- packages/trueforge/src/main.ts | 13 +++- packages/trueforge/src/runtime/redis.ts | 13 +++- .../progressSandboxEnvironmentVersion.ts | 16 +---- .../trueforge/src/sentry/reportMarkedLog.ts | 69 ++++++++++++++++++ .../TrueFoundryServiceFoundryServerClient.ts | 12 +++- .../trueforge/tests/unit/apis/auth.test.ts | 41 +++++++++++ .../unit/controller/sandboxEnvBuild.test.ts | 20 ++++++ .../progressSandboxEnvironmentVersion.test.ts | 19 +++++ .../tests/unit/sentry/reportMarkedLog.test.ts | 71 +++++++++++++++++++ 35 files changed, 592 insertions(+), 65 deletions(-) create mode 100644 .changeset/sentry-allowlisted-logs.md create mode 100644 packages/trueforge-core/src/core/util/reportableErrorLog.ts create mode 100644 packages/trueforge/src/sentry/reportMarkedLog.ts create mode 100644 packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts diff --git a/.changeset/sentry-allowlisted-logs.md b/.changeset/sentry-allowlisted-logs.md new file mode 100644 index 0000000000..4a49033127 --- /dev/null +++ b/.changeset/sentry-allowlisted-logs.md @@ -0,0 +1,6 @@ +--- +'@truefoundry/trueforge': patch +'@truefoundry/trueforge-core': patch +--- + +Report actionable network, timeout, and internal failures to Sentry when the log is explicitly marked. Provider inference errors, invalid credentials, and missing sandbox versions stay in the logs. diff --git a/packages/trueforge-core/src/agent-session/SessionHandle.ts b/packages/trueforge-core/src/agent-session/SessionHandle.ts index 815dafee7c..ef04b1b63d 100644 --- a/packages/trueforge-core/src/agent-session/SessionHandle.ts +++ b/packages/trueforge-core/src/agent-session/SessionHandle.ts @@ -18,6 +18,7 @@ import { import type { CreateDynamicSubAgentThread } from '../core/runtime/CreateDynamicSubAgentThread'; import type { HarnessSandbox } from '../core/sandbox/Sandbox'; import type { AgentTracing } from '../core/tracing/AgentTracing'; +import { reportableErrorFields } from '../core/util/reportableErrorLog'; import { builtinsFromSpec } from './builtinsFromSpec'; import type { ITurnResourceResolver, ResolvedAgentDefinition } from './ITurnResourceResolver'; import type { SessionRecord } from './models/SessionRecord'; @@ -336,6 +337,7 @@ export class SessionHandle< await input.resolver.close().catch((closeError: unknown) => { input.resolver.logger.warn('TurnResourceResolver.close() failed after createTurn() error', { err: closeError, + ...reportableErrorFields({ error: closeError, operation: 'resolverCloseAfterCreateTurn' }), }); }); throw error; diff --git a/packages/trueforge-core/src/agent-session/TurnHandle.ts b/packages/trueforge-core/src/agent-session/TurnHandle.ts index 560bed739c..41490af2b8 100644 --- a/packages/trueforge-core/src/agent-session/TurnHandle.ts +++ b/packages/trueforge-core/src/agent-session/TurnHandle.ts @@ -15,6 +15,7 @@ import type { AgentThreadOrchestrator } from '../core/runtime/AgentThreadOrchest import { getEmptyCurrentContextUsage } from '../core/runtime/contextUsage'; import { isInternalThreadDoneCancelled } from '../core/runtime/contextUtils'; import type { AgentThreadMetrics } from '../core/runtime/metrics'; +import { reportableErrorFields } from '../core/util/reportableErrorLog'; import type { ITurnResourceResolver } from './ITurnResourceResolver'; import type { TurnRecord } from './models/TurnRecord'; import { EventType, type PersistedTurnEvent, type TurnCreatedEvent, type TurnDoneEvent } from './schemas/events'; @@ -374,7 +375,10 @@ export class TurnHandle> { } await resolver.close().catch((err: unknown) => { - resolver.logger.warn('TurnResourceResolver.close() failed', { err }); + resolver.logger.warn('TurnResourceResolver.close() failed', { + err, + ...reportableErrorFields({ error: err, operation: 'resolverClose' }), + }); }); if (!frozenByStore) { diff --git a/packages/trueforge-core/src/core/capabilities/builtins/LargeToolResponse.ts b/packages/trueforge-core/src/core/capabilities/builtins/LargeToolResponse.ts index bdfcc88ea2..a1132158f3 100644 --- a/packages/trueforge-core/src/core/capabilities/builtins/LargeToolResponse.ts +++ b/packages/trueforge-core/src/core/capabilities/builtins/LargeToolResponse.ts @@ -5,6 +5,7 @@ import type { ToolCallResult } from '../../mcp/executeToolCalls'; import type { HarnessSandbox, SandboxInfo } from '../../sandbox/Sandbox'; import { createSandboxLargeToolResponseGuidance, SANDBOX_SCHEMA_INFER_TAG } from '../../sandbox/Sandbox'; import { extractErrorLogFields } from '../../util/errorLogFields'; +import { reportableErrorFields } from '../../util/reportableErrorLog'; import type { AgentCapability } from '../AgentCapability'; import type { AgentThreadExecutionContext } from '../AgentContextProcessor'; import type { ToolResponseProcessor, ToolResponseProcessorResult } from '../ToolResponseProcessor'; @@ -165,7 +166,10 @@ export class LargeToolResponseProcessor implements ToolResponseProcessor { entry.toolCallResult.isStructuredContent = false; return stored.sandboxCreated; } catch (error) { - this.logger.error('Failed to upload to sandbox', extractErrorLogFields(error)); + this.logger.error('Failed to upload to sandbox', { + ...extractErrorLogFields(error), + ...reportableErrorFields({ error, operation: 'sandboxUpload' }), + }); return undefined; } } diff --git a/packages/trueforge-core/src/core/index.ts b/packages/trueforge-core/src/core/index.ts index 6ff8d866a9..f5701ba412 100644 --- a/packages/trueforge-core/src/core/index.ts +++ b/packages/trueforge-core/src/core/index.ts @@ -158,7 +158,7 @@ export type { // Errors / utils export { AgentHarnessError, McpConnectionError, McpDcrConfigurationError } from './errors'; export { REDIS_KEY_NAMESPACE, redisKey } from './redisKeys'; -export { describeUnknownError, extractErrorLogFields } from './util/errorLogFields'; +export { describeUnknownError, extractErrorLogFields, isReportableTransportFailure } from './util/errorLogFields'; export { DEFAULT_MCP_HTTP_BODY_TIMEOUT_MS, DEFAULT_MCP_HTTP_CONNECT_TIMEOUT_MS, @@ -181,6 +181,7 @@ export { ssrfFetch, } from './util/outboundFetch'; export { PromiseTimeoutError, withTimeout } from './util/promiseUtils'; +export { reportableErrorFields, type ReportableErrorLog } from './util/reportableErrorLog'; export { assertSafeOutboundUrl, configureOutboundUrlGuard, diff --git a/packages/trueforge-core/src/core/llm/VercelAILLM.ts b/packages/trueforge-core/src/core/llm/VercelAILLM.ts index 397cebc819..ea755c9072 100644 --- a/packages/trueforge-core/src/core/llm/VercelAILLM.ts +++ b/packages/trueforge-core/src/core/llm/VercelAILLM.ts @@ -34,8 +34,9 @@ import type { ChatCompletionTool, } from 'openai/resources/chat'; import type { Logger } from 'winston'; -import { describeUnknownError, extractErrorLogFields } from '../util/errorLogFields'; +import { describeUnknownError, extractErrorLogFields, isReportableTransportFailure } from '../util/errorLogFields'; import { modelSsrfFetch } from '../util/outboundFetch'; +import { reportableErrorFields } from '../util/reportableErrorLog'; import type { ILLM, LLMCreateParams, LLMCreateParamsStreaming } from './ILLM'; import { type CompletionUsage, @@ -991,6 +992,14 @@ export function normalizeUsage(usage: { }; } +/** Provider HTTP responses stay in the log. Only a dropped connection or timeout is reportable. */ +export function isReportableLlmTransportFailure(error: unknown): boolean { + if (APICallError.isInstance(error) && error.statusCode != null) { + return false; + } + return isReportableTransportFailure(error); +} + /** * Prefer AI SDK `APICallError` (`message` / `statusCode`): * https://ai-sdk.dev/docs/reference/ai-sdk-errors/ai-api-call-error#properties @@ -1445,7 +1454,12 @@ export class VercelAILLM implements ILLM { if (this.signal?.aborted) { this.logger.debug('LLM call aborted', extractErrorLogFields(error)); } else { - this.logger.error('Error creating streaming chat completion', extractErrorLogFields(error)); + this.logger.error('Error creating streaming chat completion', { + ...extractErrorLogFields(error), + ...(isReportableLlmTransportFailure(error) + ? reportableErrorFields({ error, operation: 'llmStreamCreate' }) + : {}), + }); } throw toStreamError(error); } @@ -1483,7 +1497,12 @@ export class VercelAILLM implements ILLM { if (this.signal?.aborted) { this.logger.debug('LLM stream aborted', extractErrorLogFields(error)); } else { - this.logger.error('Error reading LLM stream', extractErrorLogFields(error)); + this.logger.error('Error reading LLM stream', { + ...extractErrorLogFields(error), + ...(isReportableLlmTransportFailure(error) + ? reportableErrorFields({ error, operation: 'llmStreamRead' }) + : {}), + }); } throw toStreamError(error); } diff --git a/packages/trueforge-core/src/core/sandbox/Sandbox.ts b/packages/trueforge-core/src/core/sandbox/Sandbox.ts index 9162eb1c6d..0626774dd0 100644 --- a/packages/trueforge-core/src/core/sandbox/Sandbox.ts +++ b/packages/trueforge-core/src/core/sandbox/Sandbox.ts @@ -15,6 +15,7 @@ import { } from '../runtime/DeferredTool'; import type { AgentTracing } from '../tracing/AgentTracing'; import { extractErrorLogFields } from '../util/errorLogFields'; +import { reportableErrorFields } from '../util/reportableErrorLog'; import { CodeModeDispatcher } from './codeMode/CodeModeDispatcher'; import { type CodeModeClientInstall, type CodeModeTransport } from './codeMode/CodeModeTransport'; import { ensureExecSuccess, shellEscape, type SandboxProvider } from './provider/Provider'; @@ -541,7 +542,10 @@ export class Sandbox extends LocalToolMCP { try { ({ sandboxInfo, sandboxCreated } = await this.ensureReadySandbox()); } catch (e) { - this.logger.error('Sandbox initialization failed', extractErrorLogFields(e)); + this.logger.error('Sandbox initialization failed', { + ...extractErrorLogFields(e), + ...reportableErrorFields({ error: e, operation: 'sandboxInit' }), + }); const message = e instanceof Error ? e.message : 'Sandbox initialization failed'; const fallback = this.existingSandboxInfo; return toolResultResponse({ @@ -801,6 +805,7 @@ export class Sandbox extends LocalToolMCP { this.logger.error('Code Mode transport start failed', { ...extractErrorLogFields(e), sandboxId, + ...reportableErrorFields({ error: e, operation: 'codeModeTransportStart', extra: { sandboxId } }), }); return undefined; } diff --git a/packages/trueforge-core/src/core/sandbox/codeMode/CodeModeDispatcher.ts b/packages/trueforge-core/src/core/sandbox/codeMode/CodeModeDispatcher.ts index 1eb2953331..e8497a5355 100644 --- a/packages/trueforge-core/src/core/sandbox/codeMode/CodeModeDispatcher.ts +++ b/packages/trueforge-core/src/core/sandbox/codeMode/CodeModeDispatcher.ts @@ -8,6 +8,7 @@ import { type IToolSet, } from '../../mcp/IMCPServer'; import { extractErrorLogFields } from '../../util/errorLogFields'; +import { reportableErrorFields } from '../../util/reportableErrorLog'; import type { CodeModeErrorSource, CodeModeReply, CodeModeRequest } from './types'; /** Minimal logger surface used by Code Mode (avoids a hard winston dep for PoC binders). */ @@ -87,7 +88,12 @@ export class CodeModeDispatcher { } catch (e) { const source = classifyErrorSource(e); const errorMessage = e instanceof Error ? e.message : 'Unknown Code Mode dispatch error'; - this.logger.error(`Code Mode dispatch failed (source=${source})`, extractErrorLogFields(e)); + this.logger.error(`Code Mode dispatch failed (source=${source})`, { + ...extractErrorLogFields(e), + ...(source === 'caller' + ? {} + : reportableErrorFields({ error: e, operation: 'codeModeDispatch', extra: { source } })), + }); return { ok: false, error: errorMessage, source }; } } diff --git a/packages/trueforge-core/src/core/sandbox/codeMode/nats/CodeModeNatsTransport.ts b/packages/trueforge-core/src/core/sandbox/codeMode/nats/CodeModeNatsTransport.ts index d028c716f9..10929366a5 100644 --- a/packages/trueforge-core/src/core/sandbox/codeMode/nats/CodeModeNatsTransport.ts +++ b/packages/trueforge-core/src/core/sandbox/codeMode/nats/CodeModeNatsTransport.ts @@ -6,6 +6,7 @@ import type { Logger } from 'winston'; import { WebSocket } from 'ws'; import { extractErrorLogFields } from '../../../util/errorLogFields'; import { withTimeout } from '../../../util/promiseUtils'; +import { reportableErrorFields } from '../../../util/reportableErrorLog'; import { DEFAULT_SANDBOX_NATS_WS_PORT } from '../../constants'; import { sandboxScripts } from '../../sandboxScripts.gen'; import type { CodeModeDispatcher } from '../CodeModeDispatcher'; @@ -93,7 +94,10 @@ export class CodeModeNatsTransport implements CodeModeTransport { try { await withTimeout(nc.drain(), NATS_DRAIN_TIMEOUT_MS); } catch (e) { - this.logger.warn('Code Mode NATS drain failed/timed out; forcing close', extractErrorLogFields(e)); + this.logger.warn('Code Mode NATS drain failed/timed out; forcing close', { + ...extractErrorLogFields(e), + ...reportableErrorFields({ error: e, operation: 'codeModeNatsDrain' }), + }); await nc.close().catch(() => { /* no-op */ }); @@ -160,7 +164,10 @@ export class CodeModeNatsTransport implements CodeModeTransport { private async handleMessage(err: Error | null, msg: Msg): Promise { if (err) { - this.logger.error('Code Mode NATS subscription error', extractErrorLogFields(err)); + this.logger.error('Code Mode NATS subscription error', { + ...extractErrorLogFields(err), + ...reportableErrorFields({ error: err, operation: 'codeModeNatsSubscription' }), + }); return; } if (!msg.reply) { @@ -173,7 +180,10 @@ export class CodeModeNatsTransport implements CodeModeTransport { try { msg.respond(this.encode({ ok: false, error: 'Code Mode dispatcher is not configured', source: 'internal' })); } catch (e) { - this.logger.warn(`Failed to send dispatcher-missing reply on '${msg.subject}'`, extractErrorLogFields(e)); + this.logger.warn(`Failed to send dispatcher-missing reply on '${msg.subject}'`, { + ...extractErrorLogFields(e), + ...reportableErrorFields({ error: e, operation: 'codeModeNatsMissingDispatcherReply' }), + }); } return; } @@ -192,11 +202,17 @@ export class CodeModeNatsTransport implements CodeModeTransport { try { request = this.decode(msg); } catch (e) { - this.logger.error(`Failed to decode Code Mode NATS request on '${msg.subject}'`, extractErrorLogFields(e)); + this.logger.error(`Failed to decode Code Mode NATS request on '${msg.subject}'`, { + ...extractErrorLogFields(e), + ...reportableErrorFields({ error: e, operation: 'codeModeNatsDecode' }), + }); try { msg.respond(this.encode({ ok: false, error: 'Malformed Code Mode request', source: 'caller' })); } catch (respondErr) { - this.logger.warn(`Failed to send decode-error reply on '${msg.subject}'`, extractErrorLogFields(respondErr)); + this.logger.warn(`Failed to send decode-error reply on '${msg.subject}'`, { + ...extractErrorLogFields(respondErr), + ...reportableErrorFields({ error: respondErr, operation: 'codeModeNatsDecodeReply' }), + }); } return; } @@ -205,7 +221,10 @@ export class CodeModeNatsTransport implements CodeModeTransport { try { msg.respond(this.encode(reply)); } catch (e) { - this.logger.error('Failed to publish Code Mode NATS reply', extractErrorLogFields(e)); + this.logger.error('Failed to publish Code Mode NATS reply', { + ...extractErrorLogFields(e), + ...reportableErrorFields({ error: e, operation: 'codeModeNatsPublish' }), + }); } } diff --git a/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts b/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts index 473cf3f513..e29276f4a2 100644 --- a/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts +++ b/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts @@ -7,6 +7,7 @@ import { join } from 'node:path/posix'; import type { Logger } from 'winston'; import { extractErrorLogFields } from '../../util/errorLogFields'; import { withTimeout } from '../../util/promiseUtils'; +import { reportableErrorFields } from '../../util/reportableErrorLog'; import { SandboxFileNotFoundError, SandboxFileTooLargeError, @@ -444,6 +445,7 @@ export class DaytonaSandboxProvider implements SandboxProvider= 500 + ? reportableErrorFields({ + error: new Error(`Sandbox server returned ${String(response.status)}`), + operation: 'sandboxServer5xx', + extra: { status: response.status }, + }) + : {}), + }); return { success: false, error: `Sandbox server returned ${String(response.status)} from ${execUrl}: ${text}`, @@ -184,13 +194,18 @@ export class TFYSandboxProvider implements SandboxProvider { this.logger.error(`Sandbox exec timed out after ${String(timeoutSeconds)}s`, { url: execUrl, ...extractErrorLogFields(e), + ...reportableErrorFields({ error: e, operation: 'sandboxExecTimeout' }), }); return { success: false, error: `Sandbox exec to ${execUrl} timed out after ${String(timeoutSeconds)}s`, }; } - this.logger.error('Sandbox exec failed', { url: execUrl, ...extractErrorLogFields(e) }); + this.logger.error('Sandbox exec failed', { + url: execUrl, + ...extractErrorLogFields(e), + ...reportableErrorFields({ error: e, operation: 'sandboxExec' }), + }); return { success: false, error: `Sandbox exec to ${execUrl} failed: ${describeUnknownError(e)}`, diff --git a/packages/trueforge-core/src/core/util/errorLogFields.ts b/packages/trueforge-core/src/core/util/errorLogFields.ts index 95bc020838..e24f2144c4 100644 --- a/packages/trueforge-core/src/core/util/errorLogFields.ts +++ b/packages/trueforge-core/src/core/util/errorLogFields.ts @@ -90,6 +90,65 @@ function stackWithCauses(error: Error): string | undefined { return stacks.length > 0 ? stacks.join('\nCaused by: ') : undefined; } +const REPORTABLE_TRANSPORT_ERROR_NAMES = new Set([ + 'AbortError', + 'TimeoutError', + 'ConnectTimeoutError', + 'HeadersTimeoutError', + 'BodyTimeoutError', +]); + +const REPORTABLE_TRANSPORT_ERROR_CODES = new Set([ + 'ECONNRESET', + 'ECONNREFUSED', + 'ETIMEDOUT', + 'ENOTFOUND', + 'EAI_AGAIN', + 'EPIPE', + 'EHOSTUNREACH', + 'ENETUNREACH', + 'ECONNABORTED', + 'UND_ERR_CONNECT_TIMEOUT', + 'UND_ERR_HEADERS_TIMEOUT', + 'UND_ERR_BODY_TIMEOUT', + 'UND_ERR_SOCKET', + 'CERT_HAS_EXPIRED', + 'UNABLE_TO_VERIFY_LEAF_SIGNATURE', + 'DEPTH_ZERO_SELF_SIGNED_CERT', + 'ERR_TLS_CERT_ALTNAME_INVALID', +]); + +function transportErrorCode(error: object): string | undefined { + if (!('code' in error)) { + return undefined; + } + const code: unknown = Reflect.get(error, 'code'); + return typeof code === 'string' ? code : undefined; +} + +function hasReportableTransportSignal(error: unknown, seen: Set): boolean { + if (typeof error !== 'object' || error === null || seen.has(error)) { + return false; + } + seen.add(error); + if (error instanceof Error && REPORTABLE_TRANSPORT_ERROR_NAMES.has(error.name)) { + return true; + } + const code = transportErrorCode(error); + if (code !== undefined && REPORTABLE_TRANSPORT_ERROR_CODES.has(code)) { + return true; + } + if (!('cause' in error)) { + return false; + } + return hasReportableTransportSignal(Reflect.get(error, 'cause'), seen); +} + +/** Timeout, DNS, TLS, and socket failures, including those nested on `error.cause`. */ +export function isReportableTransportFailure(error: unknown): boolean { + return hasReportableTransportSignal(error, new Set()); +} + export function extractErrorLogFields(error: unknown): ErrorLogFields { if (error instanceof Error) { const chain = describeErrorChain(error, new Set()); diff --git a/packages/trueforge-core/src/core/util/promiseUtils.ts b/packages/trueforge-core/src/core/util/promiseUtils.ts index f146e1503d..d2f791970f 100644 --- a/packages/trueforge-core/src/core/util/promiseUtils.ts +++ b/packages/trueforge-core/src/core/util/promiseUtils.ts @@ -1,5 +1,6 @@ import type { Logger } from 'winston'; import { extractErrorLogFields } from './errorLogFields'; +import { reportableErrorFields } from './reportableErrorLog'; export class PromiseTimeoutError extends Error { constructor(message: string) { @@ -43,7 +44,10 @@ export async function* mergeAsyncGenerators( } const nextPromise = generator.next().then(result => ({ idx, result })); nextPromise.catch((error: unknown) => { - logger.error(`Unexpected error in mergeAsyncGenerators generator ${String(idx)}`, extractErrorLogFields(error)); + logger.error(`Unexpected error in mergeAsyncGenerators generator ${String(idx)}`, { + ...extractErrorLogFields(error), + ...reportableErrorFields({ error, operation: 'mergeAsyncGenerators', extra: { generator: idx } }), + }); }); return nextPromise; }; diff --git a/packages/trueforge-core/src/core/util/reportableErrorLog.ts b/packages/trueforge-core/src/core/util/reportableErrorLog.ts new file mode 100644 index 0000000000..1dc9f10310 --- /dev/null +++ b/packages/trueforge-core/src/core/util/reportableErrorLog.ts @@ -0,0 +1,18 @@ +export interface ReportableErrorLog { + error: unknown; + operation: string; + extra: Record | undefined; +} + +/** Metadata the server logger uses to decide that a Winston line should be reported. */ +export function reportableErrorFields(input: { error: unknown; operation: string; extra?: Record }): { + reportable_error: ReportableErrorLog; +} { + return { + reportable_error: { + error: input.error, + operation: input.operation, + extra: input.extra, + }, + }; +} diff --git a/packages/trueforge-core/src/request-reply/executor.ts b/packages/trueforge-core/src/request-reply/executor.ts index b1936e8e91..52a5275590 100644 --- a/packages/trueforge-core/src/request-reply/executor.ts +++ b/packages/trueforge-core/src/request-reply/executor.ts @@ -2,6 +2,7 @@ import { randomUUID } from 'node:crypto'; import type { Logger } from 'winston'; import z from 'zod'; import { extractErrorLogFields } from '../core/util/errorLogFields'; +import { reportableErrorFields } from '../core/util/reportableErrorLog'; import { ReplyError } from './errors'; import type { RedisClient } from './redisClient'; import type { JSONReply, RequestHandler } from './types'; @@ -106,12 +107,24 @@ export class RequestReplyExecutor { this.logger.error('[RequestReplyExecutor] Subscriber error', { executorId: this.executorId, ...extractErrorLogFields(err), + ...reportableErrorFields({ + error: err, + operation: 'requestReplySubscriber', + extra: { executorId: this.executorId }, + }), }); this.onError?.(err, { executorId: this.executorId, channel: this.channel }); // error is often transient; the client can reconnect and fire ready again. Do not stop the heartbeat. }); this.subscriberClient.on('end', () => { - this.logger.warn('[RequestReplyExecutor] Subscriber connection ended', { executorId: this.executorId }); + this.logger.warn('[RequestReplyExecutor] Subscriber connection ended', { + executorId: this.executorId, + ...reportableErrorFields({ + error: new Error('Subscriber connection ended'), + operation: 'requestReplySubscriberEnded', + extra: { executorId: this.executorId }, + }), + }); this.stopHeartbeat(); }); // ready fires on the first connect and every reconnect; re-subscribing is @@ -141,6 +154,11 @@ export class RequestReplyExecutor { this.logger.error('[RequestReplyExecutor] Error subscribing to request channel', { executorId: this.executorId, ...extractErrorLogFields(error), + ...reportableErrorFields({ + error, + operation: 'requestReplySubscribe', + extra: { executorId: this.executorId }, + }), }); this.onError?.(error, { executorId: this.executorId, channel: this.channel }); this.stopHeartbeat(); @@ -155,6 +173,11 @@ export class RequestReplyExecutor { this.logger.error('[RequestReplyExecutor] Heartbeat error', { executorId: this.executorId, ...extractErrorLogFields(err), + ...reportableErrorFields({ + error: err, + operation: 'requestReplyHeartbeat', + extra: { executorId: this.executorId }, + }), }); } } @@ -187,7 +210,14 @@ export class RequestReplyExecutor { raw = JSON.parse(message); } catch (err) { // Drop the request, this should never happen - this.logger.warn('[RequestReplyExecutor] Request message is not valid JSON', { executorId: this.executorId }); + this.logger.warn('[RequestReplyExecutor] Request message is not valid JSON', { + executorId: this.executorId, + ...reportableErrorFields({ + error: err, + operation: 'requestReplyInvalidJson', + extra: { executorId: this.executorId }, + }), + }); this.onError?.( new Error(`Request message is not valid JSON for executorId: ${this.executorId}`, { cause: err, @@ -207,6 +237,11 @@ export class RequestReplyExecutor { this.logger.warn('[RequestReplyExecutor] Invalid request message shape', { executorId: this.executorId, zod: zodError, + ...reportableErrorFields({ + error: parsedRequest.error, + operation: 'requestReplyInvalidShape', + extra: { executorId: this.executorId }, + }), }); this.onError?.( new Error( @@ -239,6 +274,11 @@ export class RequestReplyExecutor { executorId: this.executorId, replyKey, ...extractErrorLogFields(err), + ...reportableErrorFields({ + error: err, + operation: 'requestReplyProcessing', + extra: { executorId: this.executorId }, + }), }); } } diff --git a/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts b/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts index 6435bfcdd0..b8a1b042eb 100644 --- a/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts +++ b/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts @@ -14,6 +14,7 @@ import { APICallError } from 'ai'; import type { ExtendedChatCompletionChunk, RawAssistantMessageWithUsage } from '../../../src/core/llm/LLMTypes'; import { describeStreamError, + isReportableLlmTransportFailure, mapFinishReason, mapStreamToChunks, normalizeUsage, @@ -680,3 +681,40 @@ describe('mapStreamToChunks', () => { expect(nonFinishChunks).toHaveLength(0); }); }); + +describe('isReportableLlmTransportFailure', () => { + it('reports timeouts and dropped connections', () => { + const timedOut = new Error('timed out'); + timedOut.name = 'TimeoutError'; + const refused = new TypeError('fetch failed', { + cause: Object.assign(new Error('connect ECONNREFUSED'), { code: 'ECONNREFUSED' }), + }); + const providerNetwork = new APICallError({ + message: 'fetch failed', + url: 'https://example.com', + requestBodyValues: {}, + cause: Object.assign(new Error('socket hang up'), { code: 'ECONNRESET' }), + }); + + expect(isReportableLlmTransportFailure(timedOut)).toBe(true); + expect(isReportableLlmTransportFailure(refused)).toBe(true); + expect(isReportableLlmTransportFailure(providerNetwork)).toBe(true); + }); + + it('skips provider HTTP responses, including rate limits and provider 5xx', () => { + for (const statusCode of [401, 429, 503]) { + expect( + isReportableLlmTransportFailure( + new APICallError({ + message: 'provider rejected the request', + url: 'https://example.com', + requestBodyValues: {}, + statusCode, + cause: Object.assign(new Error('connect ECONNRESET'), { code: 'ECONNRESET' }), + }), + ), + ).toBe(false); + } + expect(isReportableLlmTransportFailure(new Error('model produced an invalid tool call'))).toBe(false); + }); +}); diff --git a/packages/trueforge/src/apis/auth.ts b/packages/trueforge/src/apis/auth.ts index 8998bfc498..c90c55bc15 100644 --- a/packages/trueforge/src/apis/auth.ts +++ b/packages/trueforge/src/apis/auth.ts @@ -1,5 +1,9 @@ import { OpenAPIHono } from '@hono/zod-openapi'; -import { extractErrorLogFields } from '@truefoundry/trueforge-core/core'; +import { + extractErrorLogFields, + isReportableTransportFailure, + reportableErrorFields, +} from '@truefoundry/trueforge-core/core'; import type { Context, MiddlewareHandler } from 'hono'; import type { Configuration } from 'openid-client'; import type { Logger } from 'winston'; @@ -61,7 +65,10 @@ export function createAuthRouter(params: { }); return c.redirect(authorizationUrl, 302); } catch (error) { - params.logger.error('Failed to build login authorization', extractErrorLogFields(error)); + params.logger.error('Failed to build login authorization', { + ...extractErrorLogFields(error), + ...reportableErrorFields({ error, operation: 'loginAuthorization' }), + }); return c.redirect(oauthErrorRedirect('login_failed'), 302); } } @@ -70,7 +77,10 @@ export function createAuthRouter(params: { try { return c.redirect(resolveTrueFoundryLoginReturnTo(returnTo), 302); } catch (error) { - params.logger.error('Failed to build TrueFoundry external login URL', extractErrorLogFields(error)); + params.logger.error('Failed to build TrueFoundry external login URL', { + ...extractErrorLogFields(error), + ...reportableErrorFields({ error, operation: 'externalLoginUrl' }), + }); return c.redirect(oauthErrorRedirect('login_failed'), 302); } } @@ -114,7 +124,10 @@ export function createAuthRouter(params: { }); return c.redirect(safeReturnTo(pending.return_to), 302); } catch (error) { - params.logger.error('Failed to exchange authorization code', extractErrorLogFields(error)); + params.logger.error('Failed to exchange authorization code', { + ...extractErrorLogFields(error), + ...(isReportableTransportFailure(error) ? reportableErrorFields({ error, operation: 'oidcExchange' }) : {}), + }); const soft = await redirectIfAlreadyAuthenticated({ context: c, whenAuthenticated: safeReturnTo(pending.return_to), diff --git a/packages/trueforge/src/apis/mcpOAuth.ts b/packages/trueforge/src/apis/mcpOAuth.ts index d00d82dd87..2ec48b96ae 100644 --- a/packages/trueforge/src/apis/mcpOAuth.ts +++ b/packages/trueforge/src/apis/mcpOAuth.ts @@ -1,5 +1,5 @@ import { OpenAPIHono, type RouteHandler } from '@hono/zod-openapi'; -import { extractErrorLogFields, McpConnectionError } from '@truefoundry/trueforge-core/core'; +import { extractErrorLogFields, McpConnectionError, reportableErrorFields } from '@truefoundry/trueforge-core/core'; import type { Context } from 'hono'; import type { Logger } from 'winston'; import type { RequestContext } from '../auth/identity'; @@ -119,7 +119,10 @@ export function createMcpOAuthRouter(deps: McpOAuthRouterDeps(deps: McpServersRouterDeps< } return c.json({ error: { message: error.message } }, 500); } - deps.logger.error(`MCP authorize unexpected failure for "${name}"`, extractErrorLogFields(error)); + deps.logger.error(`MCP authorize unexpected failure for "${name}"`, { + ...extractErrorLogFields(error), + ...reportableErrorFields({ error, operation: 'mcpAuthorizeUnexpected', extra: { mcp_server_name: name } }), + }); return c.json({ error: { message: 'Internal server error' } }, 500); } }; diff --git a/packages/trueforge/src/apis/sessions.ts b/packages/trueforge/src/apis/sessions.ts index 5890b7a486..e03450fd75 100644 --- a/packages/trueforge/src/apis/sessions.ts +++ b/packages/trueforge/src/apis/sessions.ts @@ -16,7 +16,7 @@ import { SessionStoreNotFoundError, TurnNotFoundError, } from '@truefoundry/trueforge-core/agent-session'; -import { extractErrorLogFields } from '@truefoundry/trueforge-core/core'; +import { extractErrorLogFields, reportableErrorFields } from '@truefoundry/trueforge-core/core'; import { redisRequest, RequestTimeoutError, @@ -201,9 +201,15 @@ export async function cancelSessionTurn( ...extractErrorLogFields(error), }; if (error instanceof RequestTimeoutError) { - deps.logger.warn('Timed out waiting for owning executor to cancel; freezing the running turn', fields); + deps.logger.warn('Timed out waiting for owning executor to cancel; freezing the running turn', { + ...fields, + ...reportableErrorFields({ error, operation: 'cancelPeerTimeout', extra: { turn_id: turnId, owner } }), + }); } else { - deps.logger.warn('Failed to reach owning executor over Redis; freezing the running turn', fields); + deps.logger.warn('Failed to reach owning executor over Redis; freezing the running turn', { + ...fields, + ...reportableErrorFields({ error, operation: 'cancelPeerRedis', extra: { turn_id: turnId, owner } }), + }); } } await freezeTurnIgnoringMissing(deps.session, { turnId, reason }); diff --git a/packages/trueforge/src/apis/turns.ts b/packages/trueforge/src/apis/turns.ts index af77bd6e1e..25668b2d0d 100644 --- a/packages/trueforge/src/apis/turns.ts +++ b/packages/trueforge/src/apis/turns.ts @@ -26,6 +26,7 @@ import { PromiseTimeoutError, rawSandboxId, redisKey, + reportableErrorFields, SandboxError, VercelAILLM, withTimeout, @@ -405,7 +406,10 @@ export async function drainTurnEvents(input: { if (error instanceof SessionStoreNotFoundError) { logger.warn('Turn stream ended after session/turn was removed', extractErrorLogFields(error)); } else { - logger.error('Unexpected error in turn event drain', extractErrorLogFields(error)); + logger.error('Unexpected error in turn event drain', { + ...extractErrorLogFields(error), + ...reportableErrorFields({ error, operation: 'eventDrain' }), + }); } } finally { clearTimeout(maxExecutionTimer); @@ -728,6 +732,7 @@ export function createTurnsRouter(deps: TurnsRouterDeps) { ...extractErrorLogFields(error), sandboxId, path, + ...reportableErrorFields({ error, operation: 'sandboxFileDownload', extra: { sandboxId, path } }), }); return c.json({ error: { message: 'Failed to download file from sandbox' } }, 424); } @@ -971,7 +976,10 @@ export function createTurnsRouter(deps: TurnsRouterDeps) { } } catch (error) { if (!subscribeAbort.signal.aborted) { - deps.logger.error('Unexpected error in turn subscribe SSE loop', extractErrorLogFields(error)); + deps.logger.error('Unexpected error in turn subscribe SSE loop', { + ...extractErrorLogFields(error), + ...reportableErrorFields({ error, operation: 'subscribeSse' }), + }); } } finally { clearTimeout(timeoutHandler); diff --git a/packages/trueforge/src/app.ts b/packages/trueforge/src/app.ts index b66bbab552..9e519d2376 100644 --- a/packages/trueforge/src/app.ts +++ b/packages/trueforge/src/app.ts @@ -2,7 +2,7 @@ import { swaggerUI } from '@hono/swagger-ui'; import { OpenAPIHono, z } from '@hono/zod-openapi'; import type { ISessionStore, Sessions, TurnStreamingEvent } from '@truefoundry/trueforge-core/agent-session'; -import { extractErrorLogFields } from '@truefoundry/trueforge-core/core'; +import { extractErrorLogFields, reportableErrorFields } from '@truefoundry/trueforge-core/core'; import type { RedisClient, RequestReplyRouter } from '@truefoundry/trueforge-core/request-reply'; import type { Context, ErrorHandler, MiddlewareHandler } from 'hono'; import { bodyLimit } from 'hono/body-limit'; @@ -159,6 +159,12 @@ export function createAppErrorHandler(params: { logger: Logger }): ErrorHandler path: c.req.path, status, ...extractErrorLogFields(error), + ...(status >= 500 + ? reportableErrorFields({ + error, + operation: message === 'Unhandled error' ? 'unhandled' : 'serverApiError', + }) + : {}), }; if (status >= 500) { params.logger.error(message, fields); diff --git a/packages/trueforge/src/controller.ts b/packages/trueforge/src/controller.ts index 21e5958f61..092ac34f7c 100644 --- a/packages/trueforge/src/controller.ts +++ b/packages/trueforge/src/controller.ts @@ -1,3 +1,4 @@ +import { reportableErrorFields } from '@truefoundry/trueforge-core/core'; import type { Logger } from 'winston'; import { Controller } from './controller/Controller'; import { sandboxEnvBuildLoop } from './controller/sandboxEnvBuild'; @@ -54,7 +55,12 @@ export function runController(params: { // Passes only hold short transactions, so the deadline should never elapse. setTimeout(() => { - logger.warn(`Controller drain timed out after ${String(gracefulTimeoutSeconds)}s, exiting`); + logger.warn(`Controller drain timed out after ${String(gracefulTimeoutSeconds)}s, exiting`, { + ...reportableErrorFields({ + error: new Error(`Controller drain timed out after ${String(gracefulTimeoutSeconds)}s, exiting`), + operation: 'controllerDrainTimeout', + }), + }); process.exit(1); }, gracefulTimeoutSeconds * 1000).unref(); diff --git a/packages/trueforge/src/controller/Controller.ts b/packages/trueforge/src/controller/Controller.ts index 04b5de6bcd..c41aeb0960 100644 --- a/packages/trueforge/src/controller/Controller.ts +++ b/packages/trueforge/src/controller/Controller.ts @@ -8,6 +8,7 @@ * the server itself; when `STANDALONE=false` it is the dedicated single-replica * process. Loops are written assuming this. */ +import { reportableErrorFields } from '@truefoundry/trueforge-core/core'; import type { Logger } from 'winston'; /** Reason passed to {@link AbortController.abort} when {@link Controller.stop} runs. */ @@ -119,7 +120,11 @@ export class Controller { if (signal.aborted) { return; } - this.#logger.error('Control loop pass failed', { loop: loop.name, error }); + this.#logger.error('Control loop pass failed', { + loop: loop.name, + error, + ...reportableErrorFields({ error, operation: 'controlLoop', extra: { loop: loop.name } }), + }); } })(); diff --git a/packages/trueforge/src/controller/sandboxEnvBuild.ts b/packages/trueforge/src/controller/sandboxEnvBuild.ts index 865f4c4cc3..583489d67c 100644 --- a/packages/trueforge/src/controller/sandboxEnvBuild.ts +++ b/packages/trueforge/src/controller/sandboxEnvBuild.ts @@ -25,10 +25,6 @@ export async function dispatchSandboxEnvBuilds({ logger }: { logger: Logger }): logger.warn('Sandbox environment version not found; skipping', { environment_version_id: environmentVersionId, }); - captureCriticalException(error, { - tags: { module: 'sandboxEnvBuild', operation: 'progressNotFound' }, - extra: { environment_version_id: environmentVersionId }, - }); continue; } logger.error('Sandbox environment version progress failed', { diff --git a/packages/trueforge/src/logger.ts b/packages/trueforge/src/logger.ts index f6c366e092..fcbdc6d24d 100644 --- a/packages/trueforge/src/logger.ts +++ b/packages/trueforge/src/logger.ts @@ -7,6 +7,7 @@ */ import { AsyncLocalStorage } from 'node:async_hooks'; import winston, { type Logger } from 'winston'; +import { reportMarkedLogFormat } from './sentry/reportMarkedLog'; const STANDALONE_META_SKIP = new Set(['level', 'message', 'timestamp', 'component', 'stack', 'splat']); @@ -132,10 +133,8 @@ function jsonFormat(): winston.Logform.Format { } function serverLogFormat(options: { standalone: boolean }): winston.Logform.Format { - if (options.standalone) { - return standaloneFormat(shouldColorize()); - } - return jsonFormat(); + const output = options.standalone ? standaloneFormat(shouldColorize()) : jsonFormat(); + return winston.format.combine(reportMarkedLogFormat(), output); } function createLogger(options: { level: string; standalone: boolean; component: 'server' | 'controller' }): Logger { diff --git a/packages/trueforge/src/main.ts b/packages/trueforge/src/main.ts index 6098482859..a2edcfa705 100644 --- a/packages/trueforge/src/main.ts +++ b/packages/trueforge/src/main.ts @@ -13,6 +13,7 @@ import { configureOutboundFetches, configureOutboundUrlGuard, extractErrorLogFields, + reportableErrorFields, } from '@truefoundry/trueforge-core/core'; import type { Context } from 'hono'; import { mkdir } from 'node:fs/promises'; @@ -862,7 +863,10 @@ try { if (redis.mode !== 'sentinel') { requestReplySubscriber = redis.client.duplicate(); requestReplySubscriber.on('error', (error: Error) => { - logger.error('[RedisSubscriber] Client error', extractErrorLogFields(error)); + logger.error('[RedisSubscriber] Client error', { + ...extractErrorLogFields(error), + ...reportableErrorFields({ error, operation: 'redisSubscriber' }), + }); }); await requestReplySubscriber.connect(); } else { @@ -921,7 +925,12 @@ try { // Arm at the start of each shutdown; unref so this timer alone cannot keep the process alive. setTimeout(() => { - logger.warn(`Drain timed out after ${String(configuration.GRACEFUL_TIMEOUT_SECONDS)}s, exiting`); + logger.warn(`Drain timed out after ${String(configuration.GRACEFUL_TIMEOUT_SECONDS)}s, exiting`, { + ...reportableErrorFields({ + error: new Error(`Drain timed out after ${String(configuration.GRACEFUL_TIMEOUT_SECONDS)}s, exiting`), + operation: 'serverDrainTimeout', + }), + }); process.exit(1); }, configuration.GRACEFUL_TIMEOUT_SECONDS * 1000).unref(); diff --git a/packages/trueforge/src/runtime/redis.ts b/packages/trueforge/src/runtime/redis.ts index 9563ba04b3..6f8116618c 100644 --- a/packages/trueforge/src/runtime/redis.ts +++ b/packages/trueforge/src/runtime/redis.ts @@ -7,7 +7,7 @@ import { existsSync, readFileSync } from 'node:fs'; import { setTimeout as sleep } from 'node:timers/promises'; -import { extractErrorLogFields } from '@truefoundry/trueforge-core/core'; +import { extractErrorLogFields, reportableErrorFields } from '@truefoundry/trueforge-core/core'; import { createClient, createSentinel, type RedisClientType, type RedisSentinelType } from 'redis'; import type { Logger } from 'winston'; @@ -152,7 +152,10 @@ async function connectSentinelWithRetry(input: { }); // Without an 'error' listener node-redis crashes the process on emit. client.on('error', (error: Error) => { - input.logger.error('[Redis] Client error', extractErrorLogFields(error)); + input.logger.error('[Redis] Client error', { + ...extractErrorLogFields(error), + ...reportableErrorFields({ error, operation: 'redisClient' }), + }); }); try { await client.connect(); @@ -171,6 +174,7 @@ async function connectSentinelWithRetry(input: { input.logger.error('[Redis] Sentinel connect failed, rebuilding and retrying', { attempt, ...extractErrorLogFields(error), + ...reportableErrorFields({ error, operation: 'redisSentinelConnect', extra: { attempt } }), }); await sleep(Math.min(SENTINEL_RETRY_BASE_MS * 2 ** attempt, SENTINEL_RETRY_MAX_MS)); } @@ -215,7 +219,10 @@ export async function connectRedis(input: ConnectRedisInput): Promise { - input.logger.error('[Redis] Client error', extractErrorLogFields(error)); + input.logger.error('[Redis] Client error', { + ...extractErrorLogFields(error), + ...reportableErrorFields({ error, operation: 'redisClient' }), + }); }); await created.connect(); input.logger.info('Connected to Redis'); diff --git a/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts b/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts index 6a803aa4d6..e602d995df 100644 --- a/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts +++ b/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts @@ -11,7 +11,6 @@ import { toSandboxEnvironment, toSandboxStatus, } from '../sandbox/providerUtils'; -import { captureCriticalException } from '../sentry'; export async function progressSandboxEnvironmentVersion({ sandboxEnvironmentStore, @@ -26,12 +25,7 @@ export async function progressSandboxEnvironmentVersion({ }): Promise { const pending = await sandboxEnvironmentStore.getSandboxEnvironmentVersion({ environment_version_id }); if (pending === undefined) { - const error = new HTTPException(404, { message: 'Sandbox environment version not found' }); - captureCriticalException(error, { - tags: { module: 'progressSandboxEnvironmentVersion', operation: 'notFound' }, - extra: { environment_version_id }, - }); - throw error; + throw new HTTPException(404, { message: 'Sandbox environment version not found' }); } const providerRecord = await sandboxProviderStore.getSandboxProvider(pending.tenant_id); @@ -77,16 +71,8 @@ export async function progressSandboxEnvironmentVersion({ environment_version_id, status_reason, }); - captureCriticalException(error, { - tags: { module: 'progressSandboxEnvironmentVersion', operation: 'authz' }, - extra: { environment_version_id, tenant_id: pending.tenant_id, status_reason }, - }); return; } - captureCriticalException(error, { - tags: { module: 'progressSandboxEnvironmentVersion', operation: 'build' }, - extra: { environment_version_id, tenant_id: pending.tenant_id }, - }); throw error; } } diff --git a/packages/trueforge/src/sentry/reportMarkedLog.ts b/packages/trueforge/src/sentry/reportMarkedLog.ts new file mode 100644 index 0000000000..53ae003957 --- /dev/null +++ b/packages/trueforge/src/sentry/reportMarkedLog.ts @@ -0,0 +1,69 @@ +import winston from 'winston'; +import { captureCriticalException } from './captureCriticalException'; + +const REPORT_THROTTLE_MS = 60_000; + +const reportedAt = new Map(); + +export function resetMarkedLogReports(): void { + reportedAt.clear(); +} + +function readExtra(value: unknown): Record | undefined { + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + return undefined; + } + const extra: Record = {}; + for (const key of Object.keys(value)) { + extra[key] = Reflect.get(value, key); + } + return extra; +} + +function readReportableError( + value: unknown, +): { error: unknown; operation: string; extra: Record | undefined } | undefined { + if (typeof value !== 'object' || value === null || !('operation' in value) || !('error' in value)) { + return undefined; + } + const operation = Reflect.get(value, 'operation'); + if (typeof operation !== 'string' || operation.length === 0) { + return undefined; + } + return { + error: Reflect.get(value, 'error'), + operation, + extra: readExtra(Reflect.get(value, 'extra')), + }; +} + +function toError(value: unknown): Error { + if (value instanceof Error) { + return value; + } + const message = typeof value === 'string' && value.length > 0 ? value : 'Reportable error'; + return new Error(message, { cause: value }); +} + +function reserveReport(operation: string, nowMs: number): boolean { + const previous = reportedAt.get(operation); + if (previous !== undefined && nowMs - previous < REPORT_THROTTLE_MS) { + return false; + } + reportedAt.set(operation, nowMs); + return true; +} + +export function reportMarkedLogFormat(): winston.Logform.Format { + return winston.format(info => { + const marked = readReportableError(info['reportable_error']); + if (marked !== undefined && reserveReport(marked.operation, Date.now())) { + captureCriticalException(toError(marked.error), { + tags: { operation: marked.operation }, + ...(marked.extra !== undefined ? { extra: marked.extra } : {}), + }); + } + Reflect.deleteProperty(info, 'reportable_error'); + return info; + })(); +} diff --git a/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts b/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts index 58cf51f576..6dea95f99c 100644 --- a/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts +++ b/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts @@ -1,4 +1,4 @@ -import { extractErrorLogFields } from '@truefoundry/trueforge-core/core'; +import { extractErrorLogFields, reportableErrorFields } from '@truefoundry/trueforge-core/core'; import { HTTPException } from 'hono/http-exception'; import { LRUCache } from 'lru-cache'; import { fetch as undiciFetch, type Dispatcher } from 'undici'; @@ -272,6 +272,7 @@ export class TrueFoundryServiceFoundryServerClient { if (!parsed.success) { this.#logger.error('TrueFoundry ServiceFoundry put remote agent returned an unexpected response', { ...extractErrorLogFields(parsed.error), + ...reportableErrorFields({ error: parsed.error, operation: 'putRemoteAgent' }), }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry put remote agent returned an unexpected response', @@ -359,6 +360,7 @@ export class TrueFoundryServiceFoundryServerClient { } catch (error) { this.#logger.error('TrueFoundry ServiceFoundry resolve agent-skill-versions returned an unexpected response', { ...extractErrorLogFields(error), + ...reportableErrorFields({ error, operation: 'resolveAgentSkillVersions' }), }); throw new HTTPException(500, { message: 'TrueFoundry ServiceFoundry resolve agent-skill-versions returned an unexpected response', @@ -422,6 +424,7 @@ export class TrueFoundryServiceFoundryServerClient { this.#logger.error('TrueFoundry ServiceFoundry MCP authorize returned an unexpected response', { mcpServerId: input.mcpServerId, ...extractErrorLogFields(error), + ...reportableErrorFields({ error, operation: 'mcpAuthorize', extra: { mcp_server_id: input.mcpServerId } }), }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry MCP authorize returned an unexpected response', @@ -451,6 +454,7 @@ export class TrueFoundryServiceFoundryServerClient { this.#logger.error('TrueFoundry ServiceFoundry MCP auth status returned an unexpected response', { mcpServerId: input.mcpServerId, ...extractErrorLogFields(error), + ...reportableErrorFields({ error, operation: 'mcpAuthStatus', extra: { mcp_server_id: input.mcpServerId } }), }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry MCP auth status returned an unexpected response', @@ -566,6 +570,7 @@ export class TrueFoundryServiceFoundryServerClient { if (!parsed.success) { this.#logger.error('TrueFoundry ServiceFoundry agent permissions response was malformed', { ...extractErrorLogFields(parsed.error), + ...reportableErrorFields({ error: parsed.error, operation: 'agentPermissions' }), }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry agent permissions response was malformed', @@ -589,6 +594,7 @@ export class TrueFoundryServiceFoundryServerClient { if (!parsed.success) { this.#logger.error('TrueFoundry ServiceFoundry tenant permissions response was malformed', { ...extractErrorLogFields(parsed.error), + ...reportableErrorFields({ error: parsed.error, operation: 'tenantPermissions' }), }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry tenant permissions response was malformed', @@ -624,6 +630,7 @@ export class TrueFoundryServiceFoundryServerClient { if (!parsed.success) { this.#logger.error('TrueFoundry ServiceFoundry vend-token response was malformed', { ...extractErrorLogFields(parsed.error), + ...reportableErrorFields({ error: parsed.error, operation: 'vendToken' }), }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry vend-token response was malformed', @@ -638,6 +645,7 @@ export class TrueFoundryServiceFoundryServerClient { if (!parsed.success) { this.#logger.error('TrueFoundry ServiceFoundry server returned an unexpected list response', { ...extractErrorLogFields(parsed.error), + ...reportableErrorFields({ error: parsed.error, operation: 'unexpectedList' }), }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry server returned an unexpected list response', @@ -694,6 +702,7 @@ export class TrueFoundryServiceFoundryServerClient { durationMs: Date.now() - startedAt, timedOut, ...extractErrorLogFields(error), + ...reportableErrorFields({ error, operation: 'serviceFoundryRequest', extra: { timedOut } }), }); throw new HTTPException(500, { message: timedOut @@ -736,6 +745,7 @@ export class TrueFoundryServiceFoundryServerClient { this.#logger.error('TrueFoundry ServiceFoundry server returned non-JSON', { url: input.url.href, ...extractErrorLogFields(error), + ...reportableErrorFields({ error, operation: 'serviceFoundryNonJson' }), }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry server returned non-JSON', diff --git a/packages/trueforge/tests/unit/apis/auth.test.ts b/packages/trueforge/tests/unit/apis/auth.test.ts index ca88e04207..93f0122354 100644 --- a/packages/trueforge/tests/unit/apis/auth.test.ts +++ b/packages/trueforge/tests/unit/apis/auth.test.ts @@ -441,6 +441,7 @@ describe('auth router (auth enabled)', () => { // openid-client throws with the token-endpoint response embedded in the message, which // carries the issuer host and upstream error body. None of that may reach the browser's // address bar, where it lands in history, screenshots and support tickets. + const errorLog = jest.spyOn(logger, 'error'); const router = createTestAuthRouter({ oidcClient }); const loginRes = await router.request('/login?return_to=/', { redirect: 'manual' }); const stateCookieRaw = cookieValue(setCookies(loginRes), STATE_COOKIE) ?? ''; @@ -469,6 +470,46 @@ describe('auth router (auth enabled)', () => { expect(location).toBe('/?error=login_failed'); expect(location).not.toContain(new URL(ISSUER).host); expect(location).not.toContain('invalid_grant'); + expect(errorLog).toHaveBeenCalledWith( + 'Failed to exchange authorization code', + expect.not.objectContaining({ reportable_error: expect.anything() }), + ); + errorLog.mockRestore(); + }); + + it('GET /callback marks a token-endpoint network failure for Sentry', async () => { + const errorLog = jest.spyOn(logger, 'error'); + const router = createTestAuthRouter({ oidcClient }); + const loginRes = await router.request('/login?return_to=/', { redirect: 'manual' }); + const stateCookieRaw = cookieValue(setCookies(loginRes), STATE_COOKIE) ?? ''; + const authorizationUrl = new URL(loginRes.headers.get('location') ?? ''); + const state = authorizationUrl.searchParams.get('state') ?? ''; + + const fetchStub = globalThis.fetch; + const failingFetch: typeof fetch = async (input, init) => { + if (String(input) === `${ISSUER}/token` && init?.method === 'POST') { + throw new TypeError('fetch failed', { + cause: Object.assign(new Error('connect ECONNREFUSED'), { code: 'ECONNREFUSED' }), + }); + } + return fetchStub(input, init); + }; + globalThis.fetch = failingFetch; + + const res = await router.request(`/callback?code=abc&state=${state}&iss=${encodeURIComponent(ISSUER)}`, { + redirect: 'manual', + headers: { Cookie: `${STATE_COOKIE}=${stateCookieRaw}` }, + }); + + expect(res.status).toBe(302); + expect(res.headers.get('location')).toBe('/?error=login_failed'); + expect(errorLog).toHaveBeenCalledWith( + 'Failed to exchange authorization code', + expect.objectContaining({ + reportable_error: expect.objectContaining({ operation: 'oidcExchange' }), + }), + ); + errorLog.mockRestore(); }); it('GET /callback keeps login_failed when exchange fails and the leftover cookie is blocked', async () => { diff --git a/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts b/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts index 4cce5be44f..287a6ceec8 100644 --- a/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts +++ b/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts @@ -1,12 +1,22 @@ import { TrueForgeApi } from '@truefoundry/trueforge-sdk'; +import { createLogger } from 'winston'; import { internalTrueForgeClient } from '../../../src/controller/internalTrueForgeClient'; import { dispatchSandboxEnvBuilds, sandboxEnvBuildLoop } from '../../../src/controller/sandboxEnvBuild'; +import * as sentry from '../../../src/sentry'; + +jest.mock('../../../src/sentry', () => ({ + captureCriticalException: jest.fn(), +})); function fakeLogger() { return { error: jest.fn(), debug: jest.fn(), info: jest.fn(), warn: jest.fn() }; } describe('sandboxEnvBuildLoop', () => { + beforeEach(() => { + jest.mocked(sentry.captureCriticalException).mockClear(); + }); + afterEach(() => { jest.restoreAllMocks(); }); @@ -41,6 +51,16 @@ describe('sandboxEnvBuildLoop', () => { expect(progress).toHaveBeenCalledTimes(2); expect(logger.warn).toHaveBeenCalled(); + expect(sentry.captureCriticalException).not.toHaveBeenCalled(); + }); + + it('reports a progress failure once', async () => { + jest.spyOn(internalTrueForgeClient, 'listPendingSandboxEnvironmentVersions').mockResolvedValue(['ver-1']); + jest.spyOn(internalTrueForgeClient, 'progressSandboxEnvironmentVersion').mockRejectedValue(new Error('down')); + + await dispatchSandboxEnvBuilds({ logger: createLogger({ silent: true }) }); + + expect(sentry.captureCriticalException).toHaveBeenCalledTimes(1); }); it('does not tick when aborted', async () => { diff --git a/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts b/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts index 5f54b4a978..e8caa7701b 100644 --- a/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts +++ b/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts @@ -89,6 +89,25 @@ describe('progressSandboxEnvironmentVersion', () => { logger, }), ).rejects.toMatchObject({ status: 404 } satisfies Partial); + expect(sentry.captureCriticalException).not.toHaveBeenCalled(); + }); + + it('does not report a build failure from this layer', async () => { + const { sandboxEnvironmentStore, sandboxProviderStore } = makeStores(); + mockProvider({ + getBuildStatus: jest.fn().mockRejectedValue(new Error('network down')), + build: jest.fn(), + }); + + await expect( + progressSandboxEnvironmentVersion({ + sandboxEnvironmentStore, + sandboxProviderStore, + environment_version_id: 'ver-1', + logger, + }), + ).rejects.toThrow('network down'); + expect(sentry.captureCriticalException).not.toHaveBeenCalled(); }); it('marks ready when getBuildStatus reports ready', async () => { diff --git a/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts b/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts new file mode 100644 index 0000000000..8bdbc56024 --- /dev/null +++ b/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts @@ -0,0 +1,71 @@ +import { reportableErrorFields } from '@truefoundry/trueforge-core/core'; +import { Writable } from 'node:stream'; +import winston from 'winston'; +import { captureCriticalException } from '../../../src/sentry/captureCriticalException'; +import { reportMarkedLogFormat, resetMarkedLogReports } from '../../../src/sentry/reportMarkedLog'; + +jest.mock('../../../src/sentry/captureCriticalException', () => ({ + captureCriticalException: jest.fn(), +})); + +function loggedLines(): { logger: winston.Logger; lines: string[] } { + const lines: string[] = []; + const stream = new Writable({ + write(chunk: Buffer | string, _encoding: BufferEncoding, callback: (error?: Error | null) => void) { + lines.push(chunk.toString()); + callback(); + }, + }); + const logger = winston.createLogger({ + format: winston.format.combine(reportMarkedLogFormat(), winston.format.json()), + transports: [new winston.transports.Stream({ stream })], + }); + return { logger, lines }; +} + +describe('reportMarkedLogFormat', () => { + beforeEach(() => { + resetMarkedLogReports(); + jest.mocked(captureCriticalException).mockClear(); + jest.spyOn(Date, 'now').mockReturnValue(1_000); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('reports a marked record and removes the marker from the printed line', () => { + const failure = new Error('boom'); + const { logger, lines } = loggedLines(); + logger.error('Unhandled error', { + error: 'boom', + ...reportableErrorFields({ error: failure, operation: 'unhandled', extra: { path: '/api' } }), + }); + + expect(captureCriticalException).toHaveBeenCalledWith(failure, { + tags: { operation: 'unhandled' }, + extra: { path: '/api' }, + }); + expect(lines.join('')).not.toContain('reportable_error'); + }); + + it('leaves unmarked lines in Winston only', () => { + const { logger } = loggedLines(); + logger.error('Client API error', { error: 'bad request' }); + logger.error('Error reading LLM stream', { error: 'invalid api key' }); + + expect(captureCriticalException).not.toHaveBeenCalled(); + }); + + it('reports the first repeated operation and the next one after 60 seconds', () => { + const { logger } = loggedLines(); + const failure = new Error('socket closed'); + logger.error('[Redis] Client error', reportableErrorFields({ error: failure, operation: 'redisClient' })); + logger.error('[Redis] Client error', reportableErrorFields({ error: failure, operation: 'redisClient' })); + expect(captureCriticalException).toHaveBeenCalledTimes(1); + + jest.spyOn(Date, 'now').mockReturnValue(1_000 + 60_000); + logger.error('[Redis] Client error', reportableErrorFields({ error: failure, operation: 'redisClient' })); + expect(captureCriticalException).toHaveBeenCalledTimes(2); + }); +}); From 8408adb0319e2fd8b2239f972bbc4fa20c2e5ef8 Mon Sep 17 00:00:00 2001 From: Raman Tehlan Date: Thu, 22 Oct 2026 03:51:13 +0530 Subject: [PATCH 02/13] fix: avoid expected and duplicate Sentry reports A normal Redis close during shutdown is not a failure, and an HTTP 500 whose cause was already marked should not be reported again. Signed-off-by: Raman Tehlan --- packages/trueforge-core/src/core/index.ts | 2 +- .../src/core/util/reportableErrorLog.ts | 23 +++++++++++++++++++ .../src/request-reply/executor.ts | 14 +++++++---- .../core/util/reportableErrorLog.test.ts | 21 +++++++++++++++++ packages/trueforge/src/app.ts | 8 +++++-- .../tests/unit/appErrorHandler.test.ts | 22 ++++++++++++++++++ 6 files changed, 82 insertions(+), 8 deletions(-) create mode 100644 packages/trueforge-core/tests/core/util/reportableErrorLog.test.ts diff --git a/packages/trueforge-core/src/core/index.ts b/packages/trueforge-core/src/core/index.ts index f5701ba412..7d529e8bc9 100644 --- a/packages/trueforge-core/src/core/index.ts +++ b/packages/trueforge-core/src/core/index.ts @@ -181,7 +181,7 @@ export { ssrfFetch, } from './util/outboundFetch'; export { PromiseTimeoutError, withTimeout } from './util/promiseUtils'; -export { reportableErrorFields, type ReportableErrorLog } from './util/reportableErrorLog'; +export { hasMarkedReportableError, reportableErrorFields, type ReportableErrorLog } from './util/reportableErrorLog'; export { assertSafeOutboundUrl, configureOutboundUrlGuard, diff --git a/packages/trueforge-core/src/core/util/reportableErrorLog.ts b/packages/trueforge-core/src/core/util/reportableErrorLog.ts index 1dc9f10310..3d9692cff3 100644 --- a/packages/trueforge-core/src/core/util/reportableErrorLog.ts +++ b/packages/trueforge-core/src/core/util/reportableErrorLog.ts @@ -4,10 +4,33 @@ export interface ReportableErrorLog { extra: Record | undefined; } +const markedReportableErrors = new WeakSet(); + +function rememberMarkedError(error: unknown): void { + if (typeof error === 'object' && error !== null) { + markedReportableErrors.add(error); + } +} + +/** True when this value, or a `cause` beneath it, was already marked for Sentry. */ +export function hasMarkedReportableError(error: unknown): boolean { + const seen = new Set(); + let current: unknown = error; + while (typeof current === 'object' && current !== null && !seen.has(current)) { + seen.add(current); + if (markedReportableErrors.has(current)) { + return true; + } + current = 'cause' in current ? Reflect.get(current, 'cause') : undefined; + } + return false; +} + /** Metadata the server logger uses to decide that a Winston line should be reported. */ export function reportableErrorFields(input: { error: unknown; operation: string; extra?: Record }): { reportable_error: ReportableErrorLog; } { + rememberMarkedError(input.error); return { reportable_error: { error: input.error, diff --git a/packages/trueforge-core/src/request-reply/executor.ts b/packages/trueforge-core/src/request-reply/executor.ts index 52a5275590..9560e36920 100644 --- a/packages/trueforge-core/src/request-reply/executor.ts +++ b/packages/trueforge-core/src/request-reply/executor.ts @@ -116,14 +116,18 @@ export class RequestReplyExecutor { this.onError?.(err, { executorId: this.executorId, channel: this.channel }); // error is often transient; the client can reconnect and fire ready again. Do not stop the heartbeat. }); + // Shutdown drains the executor, then closes the client. That close emits `end`. + // Only an end while the executor is still open is an unexpected drop. this.subscriberClient.on('end', () => { this.logger.warn('[RequestReplyExecutor] Subscriber connection ended', { executorId: this.executorId, - ...reportableErrorFields({ - error: new Error('Subscriber connection ended'), - operation: 'requestReplySubscriberEnded', - extra: { executorId: this.executorId }, - }), + ...(this.closed + ? {} + : reportableErrorFields({ + error: new Error('Subscriber connection ended'), + operation: 'requestReplySubscriberEnded', + extra: { executorId: this.executorId }, + })), }); this.stopHeartbeat(); }); diff --git a/packages/trueforge-core/tests/core/util/reportableErrorLog.test.ts b/packages/trueforge-core/tests/core/util/reportableErrorLog.test.ts new file mode 100644 index 0000000000..c632b2e803 --- /dev/null +++ b/packages/trueforge-core/tests/core/util/reportableErrorLog.test.ts @@ -0,0 +1,21 @@ +import { hasMarkedReportableError, reportableErrorFields } from '../../../src/core/util/reportableErrorLog'; + +describe('hasMarkedReportableError', () => { + it('is false until the error is marked, including through cause', () => { + const cause = new Error('socket hang up'); + const wrapped = new Error('request failed', { cause }); + + expect(hasMarkedReportableError(wrapped)).toBe(false); + + reportableErrorFields({ error: cause, operation: 'serviceFoundryRequest' }); + + expect(hasMarkedReportableError(cause)).toBe(true); + expect(hasMarkedReportableError(wrapped)).toBe(true); + }); + + it('does not treat a different error as marked', () => { + reportableErrorFields({ error: new Error('marked'), operation: 'redisClient' }); + + expect(hasMarkedReportableError(new Error('other'))).toBe(false); + }); +}); diff --git a/packages/trueforge/src/app.ts b/packages/trueforge/src/app.ts index 9e519d2376..f0915719ff 100644 --- a/packages/trueforge/src/app.ts +++ b/packages/trueforge/src/app.ts @@ -2,7 +2,11 @@ import { swaggerUI } from '@hono/swagger-ui'; import { OpenAPIHono, z } from '@hono/zod-openapi'; import type { ISessionStore, Sessions, TurnStreamingEvent } from '@truefoundry/trueforge-core/agent-session'; -import { extractErrorLogFields, reportableErrorFields } from '@truefoundry/trueforge-core/core'; +import { + extractErrorLogFields, + hasMarkedReportableError, + reportableErrorFields, +} from '@truefoundry/trueforge-core/core'; import type { RedisClient, RequestReplyRouter } from '@truefoundry/trueforge-core/request-reply'; import type { Context, ErrorHandler, MiddlewareHandler } from 'hono'; import { bodyLimit } from 'hono/body-limit'; @@ -159,7 +163,7 @@ export function createAppErrorHandler(params: { logger: Logger }): ErrorHandler path: c.req.path, status, ...extractErrorLogFields(error), - ...(status >= 500 + ...(status >= 500 && !hasMarkedReportableError(error) ? reportableErrorFields({ error, operation: message === 'Unhandled error' ? 'unhandled' : 'serverApiError', diff --git a/packages/trueforge/tests/unit/appErrorHandler.test.ts b/packages/trueforge/tests/unit/appErrorHandler.test.ts index b2d8b1bad9..3e4711fda6 100644 --- a/packages/trueforge/tests/unit/appErrorHandler.test.ts +++ b/packages/trueforge/tests/unit/appErrorHandler.test.ts @@ -1,4 +1,5 @@ import { OpenAPIHono, createRoute, z } from '@hono/zod-openapi'; +import { reportableErrorFields } from '@truefoundry/trueforge-core/core'; import { HTTPException } from 'hono/http-exception'; import winston from 'winston'; import { createAppErrorHandler } from '../../src/app'; @@ -41,6 +42,7 @@ describe('createAppErrorHandler', () => { status: 500, error: thrown.message, stack: thrown.stack, + ...reportableErrorFields({ error: thrown, operation: 'unhandled' }), }); }); @@ -60,6 +62,26 @@ describe('createAppErrorHandler', () => { status, error: thrown.message, stack: thrown.stack, + ...reportableErrorFields({ error: thrown, operation: 'serverApiError' }), + }); + }); + + it('does not mark a 500 whose cause was already marked', async () => { + const logger = winston.createLogger({ silent: true }); + const errorLog = jest.spyOn(logger, 'error'); + const cause = new Error('socket hang up'); + reportableErrorFields({ error: cause, operation: 'serviceFoundryRequest' }); + const thrown = new HTTPException(500, { message: 'Service unavailable', cause }); + + const response = await appThrowing(logger, thrown).request('/test'); + + expect(response.status).toBe(500); + expect(errorLog).toHaveBeenCalledWith('Server API error', { + method: 'GET', + path: '/test', + status: 500, + error: `${thrown.message}: ${cause.message}`, + stack: `${thrown.stack ?? ''}\nCaused by: ${cause.stack ?? ''}`, }); }); From efdf1e7bd784a8f3c56d6c4baefacc707e902bdb Mon Sep 17 00:00:00 2001 From: Raman Tehlan Date: Thu, 22 Oct 2026 04:41:33 +0530 Subject: [PATCH 03/13] fix: keep existing direct Sentry captures Restore the sandbox captures this branch had dropped, and remember a directly captured error so the HTTP handler does not send it again. Signed-off-by: Raman Tehlan --- .changeset/sentry-allowlisted-logs.md | 2 +- packages/trueforge-core/src/core/index.ts | 7 +++- .../src/core/util/reportableErrorLog.ts | 5 +++ .../core/util/reportableErrorLog.test.ts | 14 ++++++- .../src/controller/sandboxEnvBuild.ts | 4 ++ .../progressSandboxEnvironmentVersion.ts | 16 ++++++- .../src/sentry/captureCriticalException.ts | 2 + .../unit/controller/sandboxEnvBuild.test.ts | 5 ++- .../progressSandboxEnvironmentVersion.test.ts | 42 +++++++++++++++++-- 9 files changed, 89 insertions(+), 8 deletions(-) diff --git a/.changeset/sentry-allowlisted-logs.md b/.changeset/sentry-allowlisted-logs.md index 4a49033127..8afcd0b254 100644 --- a/.changeset/sentry-allowlisted-logs.md +++ b/.changeset/sentry-allowlisted-logs.md @@ -3,4 +3,4 @@ '@truefoundry/trueforge-core': patch --- -Report actionable network, timeout, and internal failures to Sentry when the log is explicitly marked. Provider inference errors, invalid credentials, and missing sandbox versions stay in the logs. +Report actionable network, timeout, and internal failures to Sentry when the log is explicitly marked. Provider inference errors, invalid credentials, and client disconnects stay in the logs. Existing direct Sentry captures are unchanged. diff --git a/packages/trueforge-core/src/core/index.ts b/packages/trueforge-core/src/core/index.ts index 7d529e8bc9..e2f75cdc10 100644 --- a/packages/trueforge-core/src/core/index.ts +++ b/packages/trueforge-core/src/core/index.ts @@ -181,7 +181,12 @@ export { ssrfFetch, } from './util/outboundFetch'; export { PromiseTimeoutError, withTimeout } from './util/promiseUtils'; -export { hasMarkedReportableError, reportableErrorFields, type ReportableErrorLog } from './util/reportableErrorLog'; +export { + hasMarkedReportableError, + rememberReportedError, + reportableErrorFields, + type ReportableErrorLog, +} from './util/reportableErrorLog'; export { assertSafeOutboundUrl, configureOutboundUrlGuard, diff --git a/packages/trueforge-core/src/core/util/reportableErrorLog.ts b/packages/trueforge-core/src/core/util/reportableErrorLog.ts index 3d9692cff3..d2b3cdfb80 100644 --- a/packages/trueforge-core/src/core/util/reportableErrorLog.ts +++ b/packages/trueforge-core/src/core/util/reportableErrorLog.ts @@ -12,6 +12,11 @@ function rememberMarkedError(error: unknown): void { } } +/** Remember an error already sent to Sentry, so a later log of the same object is not sent again. */ +export function rememberReportedError(error: unknown): void { + rememberMarkedError(error); +} + /** True when this value, or a `cause` beneath it, was already marked for Sentry. */ export function hasMarkedReportableError(error: unknown): boolean { const seen = new Set(); diff --git a/packages/trueforge-core/tests/core/util/reportableErrorLog.test.ts b/packages/trueforge-core/tests/core/util/reportableErrorLog.test.ts index c632b2e803..1c061324b7 100644 --- a/packages/trueforge-core/tests/core/util/reportableErrorLog.test.ts +++ b/packages/trueforge-core/tests/core/util/reportableErrorLog.test.ts @@ -1,4 +1,8 @@ -import { hasMarkedReportableError, reportableErrorFields } from '../../../src/core/util/reportableErrorLog'; +import { + hasMarkedReportableError, + rememberReportedError, + reportableErrorFields, +} from '../../../src/core/util/reportableErrorLog'; describe('hasMarkedReportableError', () => { it('is false until the error is marked, including through cause', () => { @@ -13,6 +17,14 @@ describe('hasMarkedReportableError', () => { expect(hasMarkedReportableError(wrapped)).toBe(true); }); + it('treats an error remembered from a direct capture as already marked', () => { + const error = new Error('already sent'); + rememberReportedError(error); + + expect(hasMarkedReportableError(error)).toBe(true); + expect(hasMarkedReportableError(new Error('wrapper', { cause: error }))).toBe(true); + }); + it('does not treat a different error as marked', () => { reportableErrorFields({ error: new Error('marked'), operation: 'redisClient' }); diff --git a/packages/trueforge/src/controller/sandboxEnvBuild.ts b/packages/trueforge/src/controller/sandboxEnvBuild.ts index 583489d67c..865f4c4cc3 100644 --- a/packages/trueforge/src/controller/sandboxEnvBuild.ts +++ b/packages/trueforge/src/controller/sandboxEnvBuild.ts @@ -25,6 +25,10 @@ export async function dispatchSandboxEnvBuilds({ logger }: { logger: Logger }): logger.warn('Sandbox environment version not found; skipping', { environment_version_id: environmentVersionId, }); + captureCriticalException(error, { + tags: { module: 'sandboxEnvBuild', operation: 'progressNotFound' }, + extra: { environment_version_id: environmentVersionId }, + }); continue; } logger.error('Sandbox environment version progress failed', { diff --git a/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts b/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts index e602d995df..6a803aa4d6 100644 --- a/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts +++ b/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts @@ -11,6 +11,7 @@ import { toSandboxEnvironment, toSandboxStatus, } from '../sandbox/providerUtils'; +import { captureCriticalException } from '../sentry'; export async function progressSandboxEnvironmentVersion({ sandboxEnvironmentStore, @@ -25,7 +26,12 @@ export async function progressSandboxEnvironmentVersion({ }): Promise { const pending = await sandboxEnvironmentStore.getSandboxEnvironmentVersion({ environment_version_id }); if (pending === undefined) { - throw new HTTPException(404, { message: 'Sandbox environment version not found' }); + const error = new HTTPException(404, { message: 'Sandbox environment version not found' }); + captureCriticalException(error, { + tags: { module: 'progressSandboxEnvironmentVersion', operation: 'notFound' }, + extra: { environment_version_id }, + }); + throw error; } const providerRecord = await sandboxProviderStore.getSandboxProvider(pending.tenant_id); @@ -71,8 +77,16 @@ export async function progressSandboxEnvironmentVersion({ environment_version_id, status_reason, }); + captureCriticalException(error, { + tags: { module: 'progressSandboxEnvironmentVersion', operation: 'authz' }, + extra: { environment_version_id, tenant_id: pending.tenant_id, status_reason }, + }); return; } + captureCriticalException(error, { + tags: { module: 'progressSandboxEnvironmentVersion', operation: 'build' }, + extra: { environment_version_id, tenant_id: pending.tenant_id }, + }); throw error; } } diff --git a/packages/trueforge/src/sentry/captureCriticalException.ts b/packages/trueforge/src/sentry/captureCriticalException.ts index 062ae33971..7331f6c1e1 100644 --- a/packages/trueforge/src/sentry/captureCriticalException.ts +++ b/packages/trueforge/src/sentry/captureCriticalException.ts @@ -1,9 +1,11 @@ import * as Sentry from '@sentry/node'; +import { rememberReportedError } from '@truefoundry/trueforge-core/core'; export function captureCriticalException( err: unknown, options?: { tags?: Record; extra?: Record }, ): void { + rememberReportedError(err); Sentry.withScope(scope => { if (options?.tags) { scope.setTags(options.tags); diff --git a/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts b/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts index 287a6ceec8..2aa4849de5 100644 --- a/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts +++ b/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts @@ -51,7 +51,10 @@ describe('sandboxEnvBuildLoop', () => { expect(progress).toHaveBeenCalledTimes(2); expect(logger.warn).toHaveBeenCalled(); - expect(sentry.captureCriticalException).not.toHaveBeenCalled(); + expect(sentry.captureCriticalException).toHaveBeenCalledWith(expect.any(TrueForgeApi.NotFoundError), { + tags: { module: 'sandboxEnvBuild', operation: 'progressNotFound' }, + extra: { environment_version_id: 'gone' }, + }); }); it('reports a progress failure once', async () => { diff --git a/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts b/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts index e8caa7701b..43ae9eb92b 100644 --- a/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts +++ b/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts @@ -1,3 +1,4 @@ +import { DaytonaError } from '@daytona/sdk'; import { DAYTONA_SNAPSHOT_NOT_STARTED_REASON } from '@truefoundry/trueforge-core/core'; import { HTTPException } from 'hono/http-exception'; import { createLogger } from 'winston'; @@ -89,10 +90,13 @@ describe('progressSandboxEnvironmentVersion', () => { logger, }), ).rejects.toMatchObject({ status: 404 } satisfies Partial); - expect(sentry.captureCriticalException).not.toHaveBeenCalled(); + expect(sentry.captureCriticalException).toHaveBeenCalledWith(expect.objectContaining({ status: 404 }), { + tags: { module: 'progressSandboxEnvironmentVersion', operation: 'notFound' }, + extra: { environment_version_id: 'missing' }, + }); }); - it('does not report a build failure from this layer', async () => { + it('reports a build failure from this layer', async () => { const { sandboxEnvironmentStore, sandboxProviderStore } = makeStores(); mockProvider({ getBuildStatus: jest.fn().mockRejectedValue(new Error('network down')), @@ -107,7 +111,39 @@ describe('progressSandboxEnvironmentVersion', () => { logger, }), ).rejects.toThrow('network down'); - expect(sentry.captureCriticalException).not.toHaveBeenCalled(); + expect(sentry.captureCriticalException).toHaveBeenCalledWith(expect.any(Error), { + tags: { module: 'progressSandboxEnvironmentVersion', operation: 'build' }, + extra: { environment_version_id: 'ver-1', tenant_id: 'tenant-1' }, + }); + }); + + it('reports an authz failure and marks the version failed', async () => { + const { sandboxEnvironmentStore, sandboxProviderStore, envStore } = makeStores(); + const authError = new DaytonaError('Unauthorized', 401); + mockProvider({ + getBuildStatus: jest.fn().mockRejectedValue(authError), + build: jest.fn(), + }); + + await progressSandboxEnvironmentVersion({ + sandboxEnvironmentStore, + sandboxProviderStore, + environment_version_id: 'ver-1', + logger, + }); + + expect(envStore.markVersionFailed).toHaveBeenCalledWith({ + environment_version_id: 'ver-1', + status_reason: 'Sandbox provider rejected the API key — check the credentials', + }); + expect(sentry.captureCriticalException).toHaveBeenCalledWith(authError, { + tags: { module: 'progressSandboxEnvironmentVersion', operation: 'authz' }, + extra: { + environment_version_id: 'ver-1', + tenant_id: 'tenant-1', + status_reason: 'Sandbox provider rejected the API key — check the credentials', + }, + }); }); it('marks ready when getBuildStatus reports ready', async () => { From c42b8c1979efb1c05f0a6699948d5448174c01b8 Mon Sep 17 00:00:00 2001 From: Raman Tehlan Date: Fri, 23 Oct 2026 16:34:54 +0530 Subject: [PATCH 04/13] review comments --- .changeset/sentry-allowlisted-logs.md | 2 +- packages/trueforge-core/src/core/index.ts | 7 +--- .../core/sandbox/provider/DaytonaProvider.ts | 10 ++--- .../src/core/util/reportableErrorLog.ts | 30 +------------- .../core/util/reportableErrorLog.test.ts | 33 --------------- packages/trueforge/src/app.ts | 8 +--- .../src/controller/sandboxEnvBuild.ts | 20 +++++---- .../src/controller/scheduleDispatch.ts | 36 ++++++++-------- packages/trueforge/src/logger.ts | 4 +- packages/trueforge/src/main.ts | 5 ++- .../progressSandboxEnvironmentVersion.ts | 30 ++++++++------ .../trueforge/src/sandbox/providerUtils.ts | 6 --- .../src/sentry/captureCriticalException.ts | 26 ++++++++++-- packages/trueforge/src/sentry/initSentry.ts | 15 +++++-- .../trueforge/src/sentry/reportMarkedLog.ts | 36 ++++++---------- .../src/truefoundry/TrueFoundryAgentStore.ts | 34 +++++++-------- .../TrueFoundryServiceFoundryServerClient.ts | 2 - packages/trueforge/src/truefoundry/errors.ts | 12 +----- .../trueforge/tests/unit/apis/auth.test.ts | 35 ---------------- .../tests/unit/appErrorHandler.test.ts | 4 +- .../unit/controller/sandboxEnvBuild.test.ts | 32 +++++++-------- .../progressSandboxEnvironmentVersion.test.ts | 41 ++++++++----------- .../tests/unit/sentry/reportMarkedLog.test.ts | 14 +------ .../truefoundry/TrueFoundryAgentStore.test.ts | 1 + 24 files changed, 165 insertions(+), 278 deletions(-) delete mode 100644 packages/trueforge-core/tests/core/util/reportableErrorLog.test.ts diff --git a/.changeset/sentry-allowlisted-logs.md b/.changeset/sentry-allowlisted-logs.md index 8afcd0b254..9e5267e82a 100644 --- a/.changeset/sentry-allowlisted-logs.md +++ b/.changeset/sentry-allowlisted-logs.md @@ -3,4 +3,4 @@ '@truefoundry/trueforge-core': patch --- -Report actionable network, timeout, and internal failures to Sentry when the log is explicitly marked. Provider inference errors, invalid credentials, and client disconnects stay in the logs. Existing direct Sentry captures are unchanged. +Report actionable network, timeout, and internal failures to Sentry when the log is explicitly marked. Those events include the active tenant, request, session, and turn ids. Provider inference errors, invalid credentials, and client disconnects stay in the logs. Existing direct Sentry captures are unchanged. diff --git a/packages/trueforge-core/src/core/index.ts b/packages/trueforge-core/src/core/index.ts index 3377348494..6033fa4381 100644 --- a/packages/trueforge-core/src/core/index.ts +++ b/packages/trueforge-core/src/core/index.ts @@ -181,12 +181,7 @@ export { ssrfFetch, } from './util/outboundFetch'; export { PromiseTimeoutError, withTimeout } from './util/promiseUtils'; -export { - hasMarkedReportableError, - rememberReportedError, - reportableErrorFields, - type ReportableErrorLog, -} from './util/reportableErrorLog'; +export { reportableErrorFields, type ReportableErrorLog } from './util/reportableErrorLog'; export { assertSafeOutboundUrl, configureOutboundUrlGuard, diff --git a/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts b/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts index e29276f4a2..ac270724a8 100644 --- a/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts +++ b/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts @@ -82,8 +82,6 @@ export interface DaytonaSandboxProviderOptions { /** Defaults to 1 hour (same as the gateway's max agent execution time). */ previewUrlExpirySeconds?: number; logger: Logger; - /** Host alert hook (e.g. Sentry P1). */ - onCriticalAlert?: (error: Error) => void; } export class DaytonaSandboxProvider implements SandboxProvider { @@ -100,7 +98,6 @@ export class DaytonaSandboxProvider implements SandboxProvider void) | undefined; private readonly daytona: Daytona; private static readonly cachedSandboxes = new Map(); // De-dupes concurrent recovery attempts on the same sandbox to a single refreshData+start round-trip. @@ -118,7 +115,6 @@ export class DaytonaSandboxProvider implements SandboxProvider | undefined; } -const markedReportableErrors = new WeakSet(); - -function rememberMarkedError(error: unknown): void { - if (typeof error === 'object' && error !== null) { - markedReportableErrors.add(error); - } -} - -/** Remember an error already sent to Sentry, so a later log of the same object is not sent again. */ -export function rememberReportedError(error: unknown): void { - rememberMarkedError(error); -} - -/** True when this value, or a `cause` beneath it, was already marked for Sentry. */ -export function hasMarkedReportableError(error: unknown): boolean { - const seen = new Set(); - let current: unknown = error; - while (typeof current === 'object' && current !== null && !seen.has(current)) { - seen.add(current); - if (markedReportableErrors.has(current)) { - return true; - } - current = 'cause' in current ? Reflect.get(current, 'cause') : undefined; - } - return false; -} - -/** Metadata the server logger uses to decide that a Winston line should be reported. */ +/** Metadata the Sentry log format uses to report a Winston line. */ export function reportableErrorFields(input: { error: unknown; operation: string; extra?: Record }): { reportable_error: ReportableErrorLog; } { - rememberMarkedError(input.error); return { reportable_error: { error: input.error, diff --git a/packages/trueforge-core/tests/core/util/reportableErrorLog.test.ts b/packages/trueforge-core/tests/core/util/reportableErrorLog.test.ts deleted file mode 100644 index 1c061324b7..0000000000 --- a/packages/trueforge-core/tests/core/util/reportableErrorLog.test.ts +++ /dev/null @@ -1,33 +0,0 @@ -import { - hasMarkedReportableError, - rememberReportedError, - reportableErrorFields, -} from '../../../src/core/util/reportableErrorLog'; - -describe('hasMarkedReportableError', () => { - it('is false until the error is marked, including through cause', () => { - const cause = new Error('socket hang up'); - const wrapped = new Error('request failed', { cause }); - - expect(hasMarkedReportableError(wrapped)).toBe(false); - - reportableErrorFields({ error: cause, operation: 'serviceFoundryRequest' }); - - expect(hasMarkedReportableError(cause)).toBe(true); - expect(hasMarkedReportableError(wrapped)).toBe(true); - }); - - it('treats an error remembered from a direct capture as already marked', () => { - const error = new Error('already sent'); - rememberReportedError(error); - - expect(hasMarkedReportableError(error)).toBe(true); - expect(hasMarkedReportableError(new Error('wrapper', { cause: error }))).toBe(true); - }); - - it('does not treat a different error as marked', () => { - reportableErrorFields({ error: new Error('marked'), operation: 'redisClient' }); - - expect(hasMarkedReportableError(new Error('other'))).toBe(false); - }); -}); diff --git a/packages/trueforge/src/app.ts b/packages/trueforge/src/app.ts index f0915719ff..9e519d2376 100644 --- a/packages/trueforge/src/app.ts +++ b/packages/trueforge/src/app.ts @@ -2,11 +2,7 @@ import { swaggerUI } from '@hono/swagger-ui'; import { OpenAPIHono, z } from '@hono/zod-openapi'; import type { ISessionStore, Sessions, TurnStreamingEvent } from '@truefoundry/trueforge-core/agent-session'; -import { - extractErrorLogFields, - hasMarkedReportableError, - reportableErrorFields, -} from '@truefoundry/trueforge-core/core'; +import { extractErrorLogFields, reportableErrorFields } from '@truefoundry/trueforge-core/core'; import type { RedisClient, RequestReplyRouter } from '@truefoundry/trueforge-core/request-reply'; import type { Context, ErrorHandler, MiddlewareHandler } from 'hono'; import { bodyLimit } from 'hono/body-limit'; @@ -163,7 +159,7 @@ export function createAppErrorHandler(params: { logger: Logger }): ErrorHandler path: c.req.path, status, ...extractErrorLogFields(error), - ...(status >= 500 && !hasMarkedReportableError(error) + ...(status >= 500 ? reportableErrorFields({ error, operation: message === 'Unhandled error' ? 'unhandled' : 'serverApiError', diff --git a/packages/trueforge/src/controller/sandboxEnvBuild.ts b/packages/trueforge/src/controller/sandboxEnvBuild.ts index 865f4c4cc3..bad1080880 100644 --- a/packages/trueforge/src/controller/sandboxEnvBuild.ts +++ b/packages/trueforge/src/controller/sandboxEnvBuild.ts @@ -1,9 +1,9 @@ /** * Control loop: list pending sandbox-env versions and hand each to the server over HTTP. */ +import { reportableErrorFields } from '@truefoundry/trueforge-core/core'; import { TrueForgeApi } from '@truefoundry/trueforge-sdk'; import type { Logger } from 'winston'; -import { captureCriticalException } from '../sentry'; import type { ControlLoop } from './Controller'; import { internalTrueForgeClient } from './internalTrueForgeClient'; @@ -24,20 +24,22 @@ export async function dispatchSandboxEnvBuilds({ logger }: { logger: Logger }): if (error instanceof TrueForgeApi.NotFoundError) { logger.warn('Sandbox environment version not found; skipping', { environment_version_id: environmentVersionId, - }); - captureCriticalException(error, { - tags: { module: 'sandboxEnvBuild', operation: 'progressNotFound' }, - extra: { environment_version_id: environmentVersionId }, + ...reportableErrorFields({ + error, + operation: 'progressNotFound', + extra: { environment_version_id: environmentVersionId }, + }), }); continue; } logger.error('Sandbox environment version progress failed', { environment_version_id: environmentVersionId, error: error instanceof Error ? error.message : String(error), - }); - captureCriticalException(error, { - tags: { module: 'sandboxEnvBuild', operation: 'progress' }, - extra: { environment_version_id: environmentVersionId }, + ...reportableErrorFields({ + error, + operation: 'progress', + extra: { environment_version_id: environmentVersionId }, + }), }); } } diff --git a/packages/trueforge/src/controller/scheduleDispatch.ts b/packages/trueforge/src/controller/scheduleDispatch.ts index c1dd6797bc..40f7eae7b1 100644 --- a/packages/trueforge/src/controller/scheduleDispatch.ts +++ b/packages/trueforge/src/controller/scheduleDispatch.ts @@ -1,4 +1,5 @@ import type { SessionHandle, Sessions, TurnInputItem } from '@truefoundry/trueforge-core/agent-session'; +import { reportableErrorFields } from '@truefoundry/trueforge-core/core'; import type { Logger } from 'winston'; import type { AgentRecord, IAgentStore } from '../db/agentStore'; import { @@ -10,7 +11,6 @@ import { import type { WithTransaction } from '../db/transaction'; import { nextTriggerAfter } from '../runtime/cron'; import { InvalidCronError, type ScheduleRunStatus } from '../schemas/schedule'; -import { captureCriticalException } from '../sentry'; import type { ControlLoop } from './Controller'; import { internalTrueForgeClient } from './internalTrueForgeClient'; @@ -295,14 +295,15 @@ export async function dispatchScheduledRuns(params: { schedule_id: schedule.id, run_id: run.id, error, - }); - captureCriticalException(error, { - tags: { module: 'scheduleDispatch', operation: 'handoff' }, - extra: { - tenant_id: run.tenant_id, - schedule_id: schedule.id, - run_id: run.id, - }, + ...reportableErrorFields({ + error, + operation: 'handoff', + extra: { + tenant_id: run.tenant_id, + schedule_id: schedule.id, + run_id: run.id, + }, + }), }); await finishScheduledRun({ store, @@ -329,14 +330,15 @@ export async function dispatchScheduledRuns(params: { schedule_id: run.schedule_id, run_id: run.id, error, - }); - captureCriticalException(error, { - tags: { module: 'scheduleDispatch', operation: 'processRun' }, - extra: { - tenant_id: run.tenant_id, - schedule_id: run.schedule_id, - run_id: run.id, - }, + ...reportableErrorFields({ + error, + operation: 'processRun', + extra: { + tenant_id: run.tenant_id, + schedule_id: run.schedule_id, + run_id: run.id, + }, + }), }); } } diff --git a/packages/trueforge/src/logger.ts b/packages/trueforge/src/logger.ts index fcbdc6d24d..c2350b39de 100644 --- a/packages/trueforge/src/logger.ts +++ b/packages/trueforge/src/logger.ts @@ -7,7 +7,6 @@ */ import { AsyncLocalStorage } from 'node:async_hooks'; import winston, { type Logger } from 'winston'; -import { reportMarkedLogFormat } from './sentry/reportMarkedLog'; const STANDALONE_META_SKIP = new Set(['level', 'message', 'timestamp', 'component', 'stack', 'splat']); @@ -133,8 +132,7 @@ function jsonFormat(): winston.Logform.Format { } function serverLogFormat(options: { standalone: boolean }): winston.Logform.Format { - const output = options.standalone ? standaloneFormat(shouldColorize()) : jsonFormat(); - return winston.format.combine(reportMarkedLogFormat(), output); + return options.standalone ? standaloneFormat(shouldColorize()) : jsonFormat(); } function createLogger(options: { level: string; standalone: boolean; component: 'server' | 'controller' }): Logger { diff --git a/packages/trueforge/src/main.ts b/packages/trueforge/src/main.ts index a2edcfa705..95dd74dd37 100644 --- a/packages/trueforge/src/main.ts +++ b/packages/trueforge/src/main.ts @@ -331,8 +331,9 @@ function buildResolveAgentStore(options: { persistenceStore: PostgresAgentStore; db: Kysely; client: TrueFoundryServiceFoundryServerClient | undefined; + logger: Logger; }): (rc: RequestContext) => IAgentStore> { - const { persistenceStore, client, db } = options; + const { persistenceStore, client, db, logger } = options; return rc => { if (client) { return new TrueFoundryAgentStore({ @@ -340,6 +341,7 @@ function buildResolveAgentStore(options: { client, context: rc, db, + logger, }); } return persistenceStore; @@ -569,6 +571,7 @@ async function createDistributedPersistence(options: { persistenceStore: agentStore, db, client: serviceFoundryClient, + logger, }); const resolveSandboxProviderStore = buildResolveSandboxProviderStore({ persistenceStore: sandboxProviderStore, diff --git a/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts b/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts index 6a803aa4d6..de273ab89b 100644 --- a/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts +++ b/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts @@ -1,5 +1,9 @@ /** Progress pending env version: snapshot build only (secrets sync on PUT). */ -import { DAYTONA_SNAPSHOT_NOT_STARTED_REASON, withTimeout } from '@truefoundry/trueforge-core/core'; +import { + DAYTONA_SNAPSHOT_NOT_STARTED_REASON, + reportableErrorFields, + withTimeout, +} from '@truefoundry/trueforge-core/core'; import { HTTPException } from 'hono/http-exception'; import type { Logger } from 'winston'; import type { ISandboxEnvironmentStore } from '../db/sandboxEnvironmentStore'; @@ -11,7 +15,6 @@ import { toSandboxEnvironment, toSandboxStatus, } from '../sandbox/providerUtils'; -import { captureCriticalException } from '../sentry'; export async function progressSandboxEnvironmentVersion({ sandboxEnvironmentStore, @@ -27,9 +30,13 @@ export async function progressSandboxEnvironmentVersion({ const pending = await sandboxEnvironmentStore.getSandboxEnvironmentVersion({ environment_version_id }); if (pending === undefined) { const error = new HTTPException(404, { message: 'Sandbox environment version not found' }); - captureCriticalException(error, { - tags: { module: 'progressSandboxEnvironmentVersion', operation: 'notFound' }, - extra: { environment_version_id }, + logger.warn('Sandbox environment version not found', { + environment_version_id, + ...reportableErrorFields({ + error, + operation: 'notFound', + extra: { environment_version_id }, + }), }); throw error; } @@ -76,17 +83,14 @@ export async function progressSandboxEnvironmentVersion({ logger.warn('Sandbox environment build failed authz', { environment_version_id, status_reason, - }); - captureCriticalException(error, { - tags: { module: 'progressSandboxEnvironmentVersion', operation: 'authz' }, - extra: { environment_version_id, tenant_id: pending.tenant_id, status_reason }, + ...reportableErrorFields({ + error, + operation: 'authz', + extra: { environment_version_id, tenant_id: pending.tenant_id, status_reason }, + }), }); return; } - captureCriticalException(error, { - tags: { module: 'progressSandboxEnvironmentVersion', operation: 'build' }, - extra: { environment_version_id, tenant_id: pending.tenant_id }, - }); throw error; } } diff --git a/packages/trueforge/src/sandbox/providerUtils.ts b/packages/trueforge/src/sandbox/providerUtils.ts index fc74a13653..9dbfefc05a 100644 --- a/packages/trueforge/src/sandbox/providerUtils.ts +++ b/packages/trueforge/src/sandbox/providerUtils.ts @@ -16,7 +16,6 @@ import { type SandboxStatus, type StoredSandboxProviderManifest, } from '../schemas/sandboxProvider'; -import { captureCriticalException } from '../sentry'; /** Bound Daytona RPCs so API requests and controller ticks cannot hang indefinitely. */ export const DAYTONA_RPC_TIMEOUT_MS = 5_000; @@ -110,11 +109,6 @@ export function toDaytonaSandboxProvider({ tenantName: tenant_id, fileMaxBytesForDownload: configuration.SANDBOX_FILE_MAX_BYTES_FOR_DOWNLOAD, logger, - onCriticalAlert: error => { - captureCriticalException(error, { - tags: { module: 'DaytonaProvider', operation: 'sandboxCreate', priority: 'P1' }, - }); - }, }); } diff --git a/packages/trueforge/src/sentry/captureCriticalException.ts b/packages/trueforge/src/sentry/captureCriticalException.ts index 7331f6c1e1..4830c0ce13 100644 --- a/packages/trueforge/src/sentry/captureCriticalException.ts +++ b/packages/trueforge/src/sentry/captureCriticalException.ts @@ -1,14 +1,32 @@ import * as Sentry from '@sentry/node'; -import { rememberReportedError } from '@truefoundry/trueforge-core/core'; + +import { currentLogContext, type LogContext } from '../logger'; + +const LOG_CONTEXT_TAG_KEYS: (keyof LogContext)[] = ['tenant_id', 'request_id', 'session_id', 'turn_id']; + +function logContextTags(): Record { + const context = currentLogContext(); + if (context === undefined) { + return {}; + } + const tags: Record = {}; + for (const key of LOG_CONTEXT_TAG_KEYS) { + const value = context[key]; + if (value !== undefined) { + tags[key] = value; + } + } + return tags; +} export function captureCriticalException( err: unknown, options?: { tags?: Record; extra?: Record }, ): void { - rememberReportedError(err); + const tags: Record = { ...logContextTags(), ...options?.tags }; Sentry.withScope(scope => { - if (options?.tags) { - scope.setTags(options.tags); + if (Object.keys(tags).length > 0) { + scope.setTags(tags); } if (options?.extra) { scope.setExtras(options.extra); diff --git a/packages/trueforge/src/sentry/initSentry.ts b/packages/trueforge/src/sentry/initSentry.ts index 0fd48a8384..b3d4719391 100644 --- a/packages/trueforge/src/sentry/initSentry.ts +++ b/packages/trueforge/src/sentry/initSentry.ts @@ -1,9 +1,10 @@ import * as Sentry from '@sentry/node'; -import type { Logger } from 'winston'; +import winston, { type Logger } from 'winston'; import { isTrueFoundryModeEnabled, type ServerConfiguration } from '../config'; import { PACKAGE_VERSION } from '../packageVersion'; import { initTrueFoundrySentry } from '../truefoundry/initTrueFoundrySentry'; +import { reportMarkedLogFormat } from './reportMarkedLog'; function isLocalLikeEnv(nodeEnv: string | undefined): boolean { return nodeEnv === 'development' || nodeEnv === 'test' || nodeEnv === 'local'; @@ -20,9 +21,13 @@ export interface InitSentryOptions { tags?: Record; } +function attachMarkedLogReporting(logger: Pick): void { + logger.format = winston.format.combine(reportMarkedLogFormat(), logger.format); +} + export async function initSentry( config: ServerConfiguration, - logger: Pick, + logger: Pick, options?: InitSentryOptions, ): Promise { if (!config.SENTRY_ENABLED || isLocalLikeEnv(config.NODE_ENV)) { @@ -38,7 +43,10 @@ export async function initSentry( }; if (isTrueFoundryModeEnabled(config)) { - await initTrueFoundrySentry({ logger, tags: globalTags }); + const ready = await initTrueFoundrySentry({ logger, tags: globalTags }); + if (ready) { + attachMarkedLogReporting(logger); + } return; } @@ -54,5 +62,6 @@ export async function initSentry( integrations: [], }); applyGlobalTags(globalTags); + attachMarkedLogReporting(logger); logger.info('Sentry initialised'); } diff --git a/packages/trueforge/src/sentry/reportMarkedLog.ts b/packages/trueforge/src/sentry/reportMarkedLog.ts index 53ae003957..f9e290c8a3 100644 --- a/packages/trueforge/src/sentry/reportMarkedLog.ts +++ b/packages/trueforge/src/sentry/reportMarkedLog.ts @@ -1,14 +1,6 @@ import winston from 'winston'; import { captureCriticalException } from './captureCriticalException'; -const REPORT_THROTTLE_MS = 60_000; - -const reportedAt = new Map(); - -export function resetMarkedLogReports(): void { - reportedAt.clear(); -} - function readExtra(value: unknown): Record | undefined { if (typeof value !== 'object' || value === null || Array.isArray(value)) { return undefined; @@ -37,31 +29,27 @@ function readReportableError( }; } -function toError(value: unknown): Error { +function toError(value: unknown): Error | undefined { if (value instanceof Error) { return value; } - const message = typeof value === 'string' && value.length > 0 ? value : 'Reportable error'; - return new Error(message, { cause: value }); -} - -function reserveReport(operation: string, nowMs: number): boolean { - const previous = reportedAt.get(operation); - if (previous !== undefined && nowMs - previous < REPORT_THROTTLE_MS) { - return false; + if (typeof value !== 'string' || value.length === 0) { + return undefined; } - reportedAt.set(operation, nowMs); - return true; + return new Error(value, { cause: value }); } export function reportMarkedLogFormat(): winston.Logform.Format { return winston.format(info => { const marked = readReportableError(info['reportable_error']); - if (marked !== undefined && reserveReport(marked.operation, Date.now())) { - captureCriticalException(toError(marked.error), { - tags: { operation: marked.operation }, - ...(marked.extra !== undefined ? { extra: marked.extra } : {}), - }); + if (marked !== undefined) { + const error = toError(marked.error); + if (error !== undefined) { + captureCriticalException(error, { + tags: { operation: marked.operation }, + ...(marked.extra !== undefined ? { extra: marked.extra } : {}), + }); + } } Reflect.deleteProperty(info, 'reportable_error'); return info; diff --git a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts index c6b7464ad9..6e56308daa 100644 --- a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts +++ b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts @@ -1,5 +1,7 @@ import type { AgentSpec, TokenPagination } from '@truefoundry/trueforge-core/agent-session'; +import { extractErrorLogFields } from '@truefoundry/trueforge-core/core'; import { sql, type Kysely, type Transaction } from 'kysely'; +import type { Logger } from 'winston'; import type { RequestContext } from '../auth/identity'; import { type AgentCatalogUsageRow, @@ -19,7 +21,6 @@ import { import { PostgresAgentStore } from '../db/postgres/agent-store/PostgresAgentStore'; import type { Database } from '../db/postgres/types'; import { AGENT_DESCRIPTION_MAX_LENGTH } from '../schemas/agent'; -import { captureCriticalException } from '../sentry'; import { callerAccessToken, type ResolveServiceFoundryAuthorization } from './accessToken'; import { TrueFoundryServiceFoundryServerClient, @@ -80,17 +81,20 @@ export class TrueFoundryAgentStore implements IAgentStore> readonly #client: TrueFoundryServiceFoundryServerClient; readonly #resolveAccessToken: ResolveServiceFoundryAuthorization; readonly #db: Kysely; + readonly #logger: Logger; constructor(input: { inner: PostgresAgentStore; client: TrueFoundryServiceFoundryServerClient; context: RequestContext; db: Kysely; + logger: Logger; }) { this.#inner = input.inner; this.#client = input.client; this.#resolveAccessToken = callerAccessToken(input.context); this.#db = input.db; + this.#logger = input.logger; } listAgents( @@ -190,14 +194,12 @@ export class TrueFoundryAgentStore implements IAgentStore> const aggregate = new AggregateError(failures, 'createAgent failed and cleanup also failed', { cause: error, }); - captureCriticalException(aggregate, { - tags: { module: 'TrueFoundryAgentStore', operation: 'dualWrite' }, - extra: { - agent_id: created.id, - agent_name: created.name, - tenant_id: input.tenant_id, - external_id: externalId, - }, + this.#logger.error('createAgent failed and cleanup also failed', { + ...extractErrorLogFields(aggregate), + agent_id: created.id, + agent_name: created.name, + tenant_id: input.tenant_id, + external_id: externalId, }); throw aggregate; } @@ -256,14 +258,12 @@ export class TrueFoundryAgentStore implements IAgentStore> 'updateAgent failed and ServiceFoundry restore also failed', { cause: restoreError }, ); - captureCriticalException(aggregate, { - tags: { module: 'TrueFoundryAgentStore', operation: 'dualWrite' }, - extra: { - agent_id: input.id, - agent_name: previous.name, - tenant_id: input.tenant_id, - external_id: previous.external_id, - }, + this.#logger.error('updateAgent failed and ServiceFoundry restore also failed', { + ...extractErrorLogFields(aggregate), + agent_id: input.id, + agent_name: previous.name, + tenant_id: input.tenant_id, + external_id: previous.external_id, }); throw aggregate; } diff --git a/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts b/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts index 6dea95f99c..3267a26bfb 100644 --- a/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts +++ b/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts @@ -360,7 +360,6 @@ export class TrueFoundryServiceFoundryServerClient { } catch (error) { this.#logger.error('TrueFoundry ServiceFoundry resolve agent-skill-versions returned an unexpected response', { ...extractErrorLogFields(error), - ...reportableErrorFields({ error, operation: 'resolveAgentSkillVersions' }), }); throw new HTTPException(500, { message: 'TrueFoundry ServiceFoundry resolve agent-skill-versions returned an unexpected response', @@ -702,7 +701,6 @@ export class TrueFoundryServiceFoundryServerClient { durationMs: Date.now() - startedAt, timedOut, ...extractErrorLogFields(error), - ...reportableErrorFields({ error, operation: 'serviceFoundryRequest', extra: { timedOut } }), }); throw new HTTPException(500, { message: timedOut diff --git a/packages/trueforge/src/truefoundry/errors.ts b/packages/trueforge/src/truefoundry/errors.ts index b0233b148f..22da3c7322 100644 --- a/packages/trueforge/src/truefoundry/errors.ts +++ b/packages/trueforge/src/truefoundry/errors.ts @@ -1,6 +1,5 @@ import { HTTPException } from 'hono/http-exception'; import type { AgentRecord } from '../db/agentStore'; -import { captureCriticalException } from '../sentry'; export const AGENT_EXTERNAL_ID_REQUIRED = 'Agent is missing a TrueFoundry external id'; export const TRUEFOUNDRY_MANAGED_STATUS = 424 as const; @@ -11,16 +10,7 @@ export function requireTrueFoundryAgentExternalId( agent: Pick, ): string { if (agent.external_id === null) { - const error = new HTTPException(500, { message: AGENT_EXTERNAL_ID_REQUIRED }); - captureCriticalException(error, { - tags: { module: 'truefoundry', operation: 'requireExternalId' }, - extra: { - agent_id: agent.id, - tenant_id: agent.tenant_id, - agent_name: agent.name, - }, - }); - throw error; + throw new HTTPException(500, { message: AGENT_EXTERNAL_ID_REQUIRED }); } return agent.external_id; } diff --git a/packages/trueforge/tests/unit/apis/auth.test.ts b/packages/trueforge/tests/unit/apis/auth.test.ts index 93f0122354..522abc5837 100644 --- a/packages/trueforge/tests/unit/apis/auth.test.ts +++ b/packages/trueforge/tests/unit/apis/auth.test.ts @@ -477,41 +477,6 @@ describe('auth router (auth enabled)', () => { errorLog.mockRestore(); }); - it('GET /callback marks a token-endpoint network failure for Sentry', async () => { - const errorLog = jest.spyOn(logger, 'error'); - const router = createTestAuthRouter({ oidcClient }); - const loginRes = await router.request('/login?return_to=/', { redirect: 'manual' }); - const stateCookieRaw = cookieValue(setCookies(loginRes), STATE_COOKIE) ?? ''; - const authorizationUrl = new URL(loginRes.headers.get('location') ?? ''); - const state = authorizationUrl.searchParams.get('state') ?? ''; - - const fetchStub = globalThis.fetch; - const failingFetch: typeof fetch = async (input, init) => { - if (String(input) === `${ISSUER}/token` && init?.method === 'POST') { - throw new TypeError('fetch failed', { - cause: Object.assign(new Error('connect ECONNREFUSED'), { code: 'ECONNREFUSED' }), - }); - } - return fetchStub(input, init); - }; - globalThis.fetch = failingFetch; - - const res = await router.request(`/callback?code=abc&state=${state}&iss=${encodeURIComponent(ISSUER)}`, { - redirect: 'manual', - headers: { Cookie: `${STATE_COOKIE}=${stateCookieRaw}` }, - }); - - expect(res.status).toBe(302); - expect(res.headers.get('location')).toBe('/?error=login_failed'); - expect(errorLog).toHaveBeenCalledWith( - 'Failed to exchange authorization code', - expect.objectContaining({ - reportable_error: expect.objectContaining({ operation: 'oidcExchange' }), - }), - ); - errorLog.mockRestore(); - }); - it('GET /callback keeps login_failed when exchange fails and the leftover cookie is blocked', async () => { const restrictedClient = await initOidc({ ...configuredOidc, diff --git a/packages/trueforge/tests/unit/appErrorHandler.test.ts b/packages/trueforge/tests/unit/appErrorHandler.test.ts index 3e4711fda6..2564c950c3 100644 --- a/packages/trueforge/tests/unit/appErrorHandler.test.ts +++ b/packages/trueforge/tests/unit/appErrorHandler.test.ts @@ -66,11 +66,10 @@ describe('createAppErrorHandler', () => { }); }); - it('does not mark a 500 whose cause was already marked', async () => { + it('marks a 500 that wraps an earlier error', async () => { const logger = winston.createLogger({ silent: true }); const errorLog = jest.spyOn(logger, 'error'); const cause = new Error('socket hang up'); - reportableErrorFields({ error: cause, operation: 'serviceFoundryRequest' }); const thrown = new HTTPException(500, { message: 'Service unavailable', cause }); const response = await appThrowing(logger, thrown).request('/test'); @@ -82,6 +81,7 @@ describe('createAppErrorHandler', () => { status: 500, error: `${thrown.message}: ${cause.message}`, stack: `${thrown.stack ?? ''}\nCaused by: ${cause.stack ?? ''}`, + ...reportableErrorFields({ error: thrown, operation: 'serverApiError' }), }); }); diff --git a/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts b/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts index 2aa4849de5..a2b9f6b10f 100644 --- a/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts +++ b/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts @@ -2,21 +2,12 @@ import { TrueForgeApi } from '@truefoundry/trueforge-sdk'; import { createLogger } from 'winston'; import { internalTrueForgeClient } from '../../../src/controller/internalTrueForgeClient'; import { dispatchSandboxEnvBuilds, sandboxEnvBuildLoop } from '../../../src/controller/sandboxEnvBuild'; -import * as sentry from '../../../src/sentry'; - -jest.mock('../../../src/sentry', () => ({ - captureCriticalException: jest.fn(), -})); function fakeLogger() { return { error: jest.fn(), debug: jest.fn(), info: jest.fn(), warn: jest.fn() }; } describe('sandboxEnvBuildLoop', () => { - beforeEach(() => { - jest.mocked(sentry.captureCriticalException).mockClear(); - }); - afterEach(() => { jest.restoreAllMocks(); }); @@ -50,20 +41,29 @@ describe('sandboxEnvBuildLoop', () => { await dispatchSandboxEnvBuilds({ logger: logger as never }); expect(progress).toHaveBeenCalledTimes(2); - expect(logger.warn).toHaveBeenCalled(); - expect(sentry.captureCriticalException).toHaveBeenCalledWith(expect.any(TrueForgeApi.NotFoundError), { - tags: { module: 'sandboxEnvBuild', operation: 'progressNotFound' }, - extra: { environment_version_id: 'gone' }, - }); + expect(logger.warn).toHaveBeenCalledWith( + 'Sandbox environment version not found; skipping', + expect.objectContaining({ + environment_version_id: 'gone', + reportable_error: expect.objectContaining({ operation: 'progressNotFound' }), + }), + ); }); it('reports a progress failure once', async () => { jest.spyOn(internalTrueForgeClient, 'listPendingSandboxEnvironmentVersions').mockResolvedValue(['ver-1']); jest.spyOn(internalTrueForgeClient, 'progressSandboxEnvironmentVersion').mockRejectedValue(new Error('down')); + const logger = createLogger({ silent: true }); + const error = jest.spyOn(logger, 'error'); - await dispatchSandboxEnvBuilds({ logger: createLogger({ silent: true }) }); + await dispatchSandboxEnvBuilds({ logger }); - expect(sentry.captureCriticalException).toHaveBeenCalledTimes(1); + expect(error).toHaveBeenCalledWith( + 'Sandbox environment version progress failed', + expect.objectContaining({ + reportable_error: expect.objectContaining({ operation: 'progress' }), + }), + ); }); it('does not tick when aborted', async () => { diff --git a/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts b/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts index 43ae9eb92b..04095d7956 100644 --- a/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts +++ b/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts @@ -6,11 +6,6 @@ import type { ISandboxEnvironmentStore } from '../../../src/db/sandboxEnvironmen import type { ISandboxProviderStore } from '../../../src/db/sandboxProviderStore'; import { progressSandboxEnvironmentVersion } from '../../../src/sandbox/progressSandboxEnvironmentVersion'; import * as providerUtils from '../../../src/sandbox/providerUtils'; -import * as sentry from '../../../src/sentry'; - -jest.mock('../../../src/sentry', () => ({ - captureCriticalException: jest.fn(), -})); jest.mock('../../../src/sandbox/providerUtils', () => { const actual = jest.requireActual( @@ -77,11 +72,11 @@ function mockProvider(methods: Record): void { describe('progressSandboxEnvironmentVersion', () => { beforeEach(() => { jest.mocked(providerUtils.toDaytonaSandboxProvider).mockReset(); - jest.mocked(sentry.captureCriticalException).mockClear(); }); it('throws 404 when the version is missing', async () => { const { sandboxEnvironmentStore, sandboxProviderStore } = makeStores({ pending: null }); + const warn = jest.spyOn(logger, 'warn'); await expect( progressSandboxEnvironmentVersion({ sandboxEnvironmentStore, @@ -90,18 +85,22 @@ describe('progressSandboxEnvironmentVersion', () => { logger, }), ).rejects.toMatchObject({ status: 404 } satisfies Partial); - expect(sentry.captureCriticalException).toHaveBeenCalledWith(expect.objectContaining({ status: 404 }), { - tags: { module: 'progressSandboxEnvironmentVersion', operation: 'notFound' }, - extra: { environment_version_id: 'missing' }, - }); + expect(warn).toHaveBeenCalledWith( + 'Sandbox environment version not found', + expect.objectContaining({ + environment_version_id: 'missing', + reportable_error: expect.objectContaining({ operation: 'notFound' }), + }), + ); }); - it('reports a build failure from this layer', async () => { + it('rethrows a build failure without reporting it from this layer', async () => { const { sandboxEnvironmentStore, sandboxProviderStore } = makeStores(); mockProvider({ getBuildStatus: jest.fn().mockRejectedValue(new Error('network down')), build: jest.fn(), }); + const errorLog = jest.spyOn(logger, 'error'); await expect( progressSandboxEnvironmentVersion({ @@ -111,15 +110,13 @@ describe('progressSandboxEnvironmentVersion', () => { logger, }), ).rejects.toThrow('network down'); - expect(sentry.captureCriticalException).toHaveBeenCalledWith(expect.any(Error), { - tags: { module: 'progressSandboxEnvironmentVersion', operation: 'build' }, - extra: { environment_version_id: 'ver-1', tenant_id: 'tenant-1' }, - }); + expect(errorLog).not.toHaveBeenCalled(); }); it('reports an authz failure and marks the version failed', async () => { const { sandboxEnvironmentStore, sandboxProviderStore, envStore } = makeStores(); const authError = new DaytonaError('Unauthorized', 401); + const warn = jest.spyOn(logger, 'warn'); mockProvider({ getBuildStatus: jest.fn().mockRejectedValue(authError), build: jest.fn(), @@ -136,14 +133,12 @@ describe('progressSandboxEnvironmentVersion', () => { environment_version_id: 'ver-1', status_reason: 'Sandbox provider rejected the API key — check the credentials', }); - expect(sentry.captureCriticalException).toHaveBeenCalledWith(authError, { - tags: { module: 'progressSandboxEnvironmentVersion', operation: 'authz' }, - extra: { - environment_version_id: 'ver-1', - tenant_id: 'tenant-1', - status_reason: 'Sandbox provider rejected the API key — check the credentials', - }, - }); + expect(warn).toHaveBeenCalledWith( + 'Sandbox environment build failed authz', + expect.objectContaining({ + reportable_error: expect.objectContaining({ operation: 'authz' }), + }), + ); }); it('marks ready when getBuildStatus reports ready', async () => { diff --git a/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts b/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts index 8bdbc56024..cccc8ef5d4 100644 --- a/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts +++ b/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts @@ -2,7 +2,7 @@ import { reportableErrorFields } from '@truefoundry/trueforge-core/core'; import { Writable } from 'node:stream'; import winston from 'winston'; import { captureCriticalException } from '../../../src/sentry/captureCriticalException'; -import { reportMarkedLogFormat, resetMarkedLogReports } from '../../../src/sentry/reportMarkedLog'; +import { reportMarkedLogFormat } from '../../../src/sentry/reportMarkedLog'; jest.mock('../../../src/sentry/captureCriticalException', () => ({ captureCriticalException: jest.fn(), @@ -25,13 +25,7 @@ function loggedLines(): { logger: winston.Logger; lines: string[] } { describe('reportMarkedLogFormat', () => { beforeEach(() => { - resetMarkedLogReports(); jest.mocked(captureCriticalException).mockClear(); - jest.spyOn(Date, 'now').mockReturnValue(1_000); - }); - - afterEach(() => { - jest.restoreAllMocks(); }); it('reports a marked record and removes the marker from the printed line', () => { @@ -57,15 +51,11 @@ describe('reportMarkedLogFormat', () => { expect(captureCriticalException).not.toHaveBeenCalled(); }); - it('reports the first repeated operation and the next one after 60 seconds', () => { + it('reports each repeated operation', () => { const { logger } = loggedLines(); const failure = new Error('socket closed'); logger.error('[Redis] Client error', reportableErrorFields({ error: failure, operation: 'redisClient' })); logger.error('[Redis] Client error', reportableErrorFields({ error: failure, operation: 'redisClient' })); - expect(captureCriticalException).toHaveBeenCalledTimes(1); - - jest.spyOn(Date, 'now').mockReturnValue(1_000 + 60_000); - logger.error('[Redis] Client error', reportableErrorFields({ error: failure, operation: 'redisClient' })); expect(captureCriticalException).toHaveBeenCalledTimes(2); }); }); diff --git a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts index 9d66285fe9..7c39dcc7eb 100644 --- a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts +++ b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts @@ -106,6 +106,7 @@ function tfStore(input: { user_credential: input.accessToken ?? TOKEN, }, db: input.db ?? mockDb(), + logger: LOGGER, }); } From 31f1a557528ac0771d128c4700351db75a82e042 Mon Sep 17 00:00:00 2001 From: Raman Tehlan Date: Fri, 23 Oct 2026 18:42:16 +0530 Subject: [PATCH 05/13] review comments --- .changeset/sentry-allowlisted-logs.md | 2 +- .../src/agent-session/SessionHandle.ts | 2 +- .../src/agent-session/TurnHandle.ts | 2 +- .../builtins/LargeToolResponse.ts | 2 +- .../src/core/llm/VercelAILLM.ts | 8 +-- .../src/core/sandbox/Sandbox.ts | 4 +- .../sandbox/codeMode/CodeModeDispatcher.ts | 4 +- .../codeMode/nats/CodeModeNatsTransport.ts | 12 ++-- .../core/sandbox/provider/DaytonaProvider.ts | 9 ++- .../sandbox/provider/TFYSandboxProvider.ts | 5 +- .../src/core/util/promiseUtils.ts | 2 +- .../src/core/util/reportableErrorLog.ts | 58 ++++++++++++++++++- .../src/request-reply/executor.ts | 13 ----- packages/trueforge/src/apis/auth.ts | 6 +- packages/trueforge/src/apis/mcpOAuth.ts | 2 +- packages/trueforge/src/apis/mcpServers.ts | 2 +- packages/trueforge/src/apis/sessions.ts | 4 +- packages/trueforge/src/apis/turns.ts | 6 +- packages/trueforge/src/app.ts | 7 +-- packages/trueforge/src/controller.ts | 1 - .../trueforge/src/controller/Controller.ts | 2 +- .../src/controller/sandboxEnvBuild.ts | 2 - .../src/controller/scheduleDispatch.ts | 2 - packages/trueforge/src/main.ts | 3 +- packages/trueforge/src/runtime/redis.ts | 6 +- .../progressSandboxEnvironmentVersion.ts | 2 - .../trueforge/src/sentry/reportMarkedLog.ts | 23 +++++--- .../TrueFoundryServiceFoundryServerClient.ts | 16 ++--- .../tests/unit/appErrorHandler.test.ts | 16 +++-- .../unit/controller/sandboxEnvBuild.test.ts | 4 +- .../progressSandboxEnvironmentVersion.test.ts | 4 +- .../tests/unit/sentry/reportMarkedLog.test.ts | 10 ++-- 32 files changed, 137 insertions(+), 104 deletions(-) diff --git a/.changeset/sentry-allowlisted-logs.md b/.changeset/sentry-allowlisted-logs.md index 9e5267e82a..b7e0183ef8 100644 --- a/.changeset/sentry-allowlisted-logs.md +++ b/.changeset/sentry-allowlisted-logs.md @@ -3,4 +3,4 @@ '@truefoundry/trueforge-core': patch --- -Report actionable network, timeout, and internal failures to Sentry when the log is explicitly marked. Those events include the active tenant, request, session, and turn ids. Provider inference errors, invalid credentials, and client disconnects stay in the logs. Existing direct Sentry captures are unchanged. +Report actionable network, timeout, and internal failures to Sentry when the log is explicitly marked. Those events include the active tenant, request, session, and turn ids, and are tagged with the file that logged them. Provider inference errors, invalid credentials, and client disconnects stay in the logs. Existing direct Sentry captures are unchanged. diff --git a/packages/trueforge-core/src/agent-session/SessionHandle.ts b/packages/trueforge-core/src/agent-session/SessionHandle.ts index ef04b1b63d..4c54fd1d2f 100644 --- a/packages/trueforge-core/src/agent-session/SessionHandle.ts +++ b/packages/trueforge-core/src/agent-session/SessionHandle.ts @@ -337,7 +337,7 @@ export class SessionHandle< await input.resolver.close().catch((closeError: unknown) => { input.resolver.logger.warn('TurnResourceResolver.close() failed after createTurn() error', { err: closeError, - ...reportableErrorFields({ error: closeError, operation: 'resolverCloseAfterCreateTurn' }), + ...reportableErrorFields({ error: closeError }), }); }); throw error; diff --git a/packages/trueforge-core/src/agent-session/TurnHandle.ts b/packages/trueforge-core/src/agent-session/TurnHandle.ts index 41490af2b8..895099190f 100644 --- a/packages/trueforge-core/src/agent-session/TurnHandle.ts +++ b/packages/trueforge-core/src/agent-session/TurnHandle.ts @@ -377,7 +377,7 @@ export class TurnHandle> { await resolver.close().catch((err: unknown) => { resolver.logger.warn('TurnResourceResolver.close() failed', { err, - ...reportableErrorFields({ error: err, operation: 'resolverClose' }), + ...reportableErrorFields({ error: err }), }); }); diff --git a/packages/trueforge-core/src/core/capabilities/builtins/LargeToolResponse.ts b/packages/trueforge-core/src/core/capabilities/builtins/LargeToolResponse.ts index 9fbbfebbd9..2eb03d29f1 100644 --- a/packages/trueforge-core/src/core/capabilities/builtins/LargeToolResponse.ts +++ b/packages/trueforge-core/src/core/capabilities/builtins/LargeToolResponse.ts @@ -184,7 +184,7 @@ export class LargeToolResponseProcessor implements ToolResponseProcessor { } catch (error) { this.logger.error('Failed to upload to sandbox', { ...extractErrorLogFields(error), - ...reportableErrorFields({ error, operation: 'sandboxUpload' }), + ...reportableErrorFields({ error }), }); return undefined; } diff --git a/packages/trueforge-core/src/core/llm/VercelAILLM.ts b/packages/trueforge-core/src/core/llm/VercelAILLM.ts index 1eedf9efd6..4aaac4ae9f 100644 --- a/packages/trueforge-core/src/core/llm/VercelAILLM.ts +++ b/packages/trueforge-core/src/core/llm/VercelAILLM.ts @@ -1458,9 +1458,7 @@ export class VercelAILLM implements ILLM { } else { this.logger.error('Error creating streaming chat completion', { ...extractErrorLogFields(streamError), - ...(isReportableLlmTransportFailure(error) - ? reportableErrorFields({ error: streamError, operation: 'llmStreamCreate' }) - : {}), + ...(isReportableLlmTransportFailure(error) ? reportableErrorFields({ error: streamError }) : {}), }); } throw streamError; @@ -1502,9 +1500,7 @@ export class VercelAILLM implements ILLM { } else { this.logger.error('Error reading LLM stream', { ...extractErrorLogFields(streamError), - ...(isReportableLlmTransportFailure(error) - ? reportableErrorFields({ error: streamError, operation: 'llmStreamRead' }) - : {}), + ...(isReportableLlmTransportFailure(error) ? reportableErrorFields({ error: streamError }) : {}), }); } throw streamError; diff --git a/packages/trueforge-core/src/core/sandbox/Sandbox.ts b/packages/trueforge-core/src/core/sandbox/Sandbox.ts index 0626774dd0..4c6daf3dcf 100644 --- a/packages/trueforge-core/src/core/sandbox/Sandbox.ts +++ b/packages/trueforge-core/src/core/sandbox/Sandbox.ts @@ -544,7 +544,7 @@ export class Sandbox extends LocalToolMCP { } catch (e) { this.logger.error('Sandbox initialization failed', { ...extractErrorLogFields(e), - ...reportableErrorFields({ error: e, operation: 'sandboxInit' }), + ...reportableErrorFields({ error: e }), }); const message = e instanceof Error ? e.message : 'Sandbox initialization failed'; const fallback = this.existingSandboxInfo; @@ -805,7 +805,7 @@ export class Sandbox extends LocalToolMCP { this.logger.error('Code Mode transport start failed', { ...extractErrorLogFields(e), sandboxId, - ...reportableErrorFields({ error: e, operation: 'codeModeTransportStart', extra: { sandboxId } }), + ...reportableErrorFields({ error: e, extra: { sandboxId } }), }); return undefined; } diff --git a/packages/trueforge-core/src/core/sandbox/codeMode/CodeModeDispatcher.ts b/packages/trueforge-core/src/core/sandbox/codeMode/CodeModeDispatcher.ts index e8497a5355..04df28c24c 100644 --- a/packages/trueforge-core/src/core/sandbox/codeMode/CodeModeDispatcher.ts +++ b/packages/trueforge-core/src/core/sandbox/codeMode/CodeModeDispatcher.ts @@ -90,9 +90,7 @@ export class CodeModeDispatcher { const errorMessage = e instanceof Error ? e.message : 'Unknown Code Mode dispatch error'; this.logger.error(`Code Mode dispatch failed (source=${source})`, { ...extractErrorLogFields(e), - ...(source === 'caller' - ? {} - : reportableErrorFields({ error: e, operation: 'codeModeDispatch', extra: { source } })), + ...(source === 'caller' ? {} : reportableErrorFields({ error: e, extra: { source } })), }); return { ok: false, error: errorMessage, source }; } diff --git a/packages/trueforge-core/src/core/sandbox/codeMode/nats/CodeModeNatsTransport.ts b/packages/trueforge-core/src/core/sandbox/codeMode/nats/CodeModeNatsTransport.ts index 10929366a5..9d1e93b451 100644 --- a/packages/trueforge-core/src/core/sandbox/codeMode/nats/CodeModeNatsTransport.ts +++ b/packages/trueforge-core/src/core/sandbox/codeMode/nats/CodeModeNatsTransport.ts @@ -96,7 +96,7 @@ export class CodeModeNatsTransport implements CodeModeTransport { } catch (e) { this.logger.warn('Code Mode NATS drain failed/timed out; forcing close', { ...extractErrorLogFields(e), - ...reportableErrorFields({ error: e, operation: 'codeModeNatsDrain' }), + ...reportableErrorFields({ error: e }), }); await nc.close().catch(() => { /* no-op */ @@ -166,7 +166,7 @@ export class CodeModeNatsTransport implements CodeModeTransport { if (err) { this.logger.error('Code Mode NATS subscription error', { ...extractErrorLogFields(err), - ...reportableErrorFields({ error: err, operation: 'codeModeNatsSubscription' }), + ...reportableErrorFields({ error: err }), }); return; } @@ -182,7 +182,7 @@ export class CodeModeNatsTransport implements CodeModeTransport { } catch (e) { this.logger.warn(`Failed to send dispatcher-missing reply on '${msg.subject}'`, { ...extractErrorLogFields(e), - ...reportableErrorFields({ error: e, operation: 'codeModeNatsMissingDispatcherReply' }), + ...reportableErrorFields({ error: e }), }); } return; @@ -204,14 +204,14 @@ export class CodeModeNatsTransport implements CodeModeTransport { } catch (e) { this.logger.error(`Failed to decode Code Mode NATS request on '${msg.subject}'`, { ...extractErrorLogFields(e), - ...reportableErrorFields({ error: e, operation: 'codeModeNatsDecode' }), + ...reportableErrorFields({ error: e }), }); try { msg.respond(this.encode({ ok: false, error: 'Malformed Code Mode request', source: 'caller' })); } catch (respondErr) { this.logger.warn(`Failed to send decode-error reply on '${msg.subject}'`, { ...extractErrorLogFields(respondErr), - ...reportableErrorFields({ error: respondErr, operation: 'codeModeNatsDecodeReply' }), + ...reportableErrorFields({ error: respondErr }), }); } return; @@ -223,7 +223,7 @@ export class CodeModeNatsTransport implements CodeModeTransport { } catch (e) { this.logger.error('Failed to publish Code Mode NATS reply', { ...extractErrorLogFields(e), - ...reportableErrorFields({ error: e, operation: 'codeModeNatsPublish' }), + ...reportableErrorFields({ error: e }), }); } } diff --git a/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts b/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts index ac270724a8..664ef95697 100644 --- a/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts +++ b/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts @@ -359,7 +359,6 @@ export class DaytonaSandboxProvider implements SandboxProvider= 500 ? reportableErrorFields({ error: new Error(`Sandbox server returned ${String(response.status)}`), - operation: 'sandboxServer5xx', extra: { status: response.status }, }) : {}), @@ -194,7 +193,7 @@ export class TFYSandboxProvider implements SandboxProvider { this.logger.error(`Sandbox exec timed out after ${String(timeoutSeconds)}s`, { url: execUrl, ...extractErrorLogFields(e), - ...reportableErrorFields({ error: e, operation: 'sandboxExecTimeout' }), + ...reportableErrorFields({ error: e }), }); return { success: false, @@ -204,7 +203,7 @@ export class TFYSandboxProvider implements SandboxProvider { this.logger.error('Sandbox exec failed', { url: execUrl, ...extractErrorLogFields(e), - ...reportableErrorFields({ error: e, operation: 'sandboxExec' }), + ...reportableErrorFields({ error: e }), }); return { success: false, diff --git a/packages/trueforge-core/src/core/util/promiseUtils.ts b/packages/trueforge-core/src/core/util/promiseUtils.ts index d2f791970f..ba2cc8ef63 100644 --- a/packages/trueforge-core/src/core/util/promiseUtils.ts +++ b/packages/trueforge-core/src/core/util/promiseUtils.ts @@ -46,7 +46,7 @@ export async function* mergeAsyncGenerators( nextPromise.catch((error: unknown) => { logger.error(`Unexpected error in mergeAsyncGenerators generator ${String(idx)}`, { ...extractErrorLogFields(error), - ...reportableErrorFields({ error, operation: 'mergeAsyncGenerators', extra: { generator: idx } }), + ...reportableErrorFields({ error, extra: { generator: idx } }), }); }); return nextPromise; diff --git a/packages/trueforge-core/src/core/util/reportableErrorLog.ts b/packages/trueforge-core/src/core/util/reportableErrorLog.ts index 03a98acd74..1aa3fcec80 100644 --- a/packages/trueforge-core/src/core/util/reportableErrorLog.ts +++ b/packages/trueforge-core/src/core/util/reportableErrorLog.ts @@ -1,18 +1,70 @@ export interface ReportableErrorLog { error: unknown; - operation: string; extra: Record | undefined; + filepath: string | undefined; +} + +function isCallSite(value: unknown): value is NodeJS.CallSite { + return ( + typeof value === 'object' && + value !== null && + 'getFileName' in value && + typeof Reflect.get(value, 'getFileName') === 'function' + ); +} + +function packageRelativePath(fileName: string): string { + const marker = '/packages/'; + const index = fileName.lastIndexOf(marker); + if (index === -1) { + return fileName; + } + return fileName.slice(index + marker.length); +} + +function callerFilePath(): string | undefined { + const previous: unknown = Reflect.get(Error, 'prepareStackTrace'); + Reflect.set(Error, 'prepareStackTrace', (_error: Error, stack: NodeJS.CallSite[]) => stack); + try { + const captured = new Error(); + const stack: unknown = captured.stack; + if (!Array.isArray(stack)) { + return undefined; + } + for (const site of stack) { + if (!isCallSite(site)) { + continue; + } + const fileName = site.getFileName(); + if ( + fileName === null || + fileName.startsWith('node:') || + fileName.includes('node_modules') || + fileName.includes('reportableErrorLog') + ) { + continue; + } + return packageRelativePath(fileName); + } + return undefined; + } finally { + if (typeof previous === 'function') { + Reflect.set(Error, 'prepareStackTrace', previous); + } else { + Reflect.deleteProperty(Error, 'prepareStackTrace'); + } + } } /** Metadata the Sentry log format uses to report a Winston line. */ -export function reportableErrorFields(input: { error: unknown; operation: string; extra?: Record }): { +export function reportableErrorFields(input: { error: unknown; extra?: Record }): { reportable_error: ReportableErrorLog; } { return { reportable_error: { error: input.error, - operation: input.operation, extra: input.extra, + filepath: callerFilePath(), }, }; } diff --git a/packages/trueforge-core/src/request-reply/executor.ts b/packages/trueforge-core/src/request-reply/executor.ts index 9560e36920..2ec69efd79 100644 --- a/packages/trueforge-core/src/request-reply/executor.ts +++ b/packages/trueforge-core/src/request-reply/executor.ts @@ -109,7 +109,6 @@ export class RequestReplyExecutor { ...extractErrorLogFields(err), ...reportableErrorFields({ error: err, - operation: 'requestReplySubscriber', extra: { executorId: this.executorId }, }), }); @@ -121,13 +120,6 @@ export class RequestReplyExecutor { this.subscriberClient.on('end', () => { this.logger.warn('[RequestReplyExecutor] Subscriber connection ended', { executorId: this.executorId, - ...(this.closed - ? {} - : reportableErrorFields({ - error: new Error('Subscriber connection ended'), - operation: 'requestReplySubscriberEnded', - extra: { executorId: this.executorId }, - })), }); this.stopHeartbeat(); }); @@ -160,7 +152,6 @@ export class RequestReplyExecutor { ...extractErrorLogFields(error), ...reportableErrorFields({ error, - operation: 'requestReplySubscribe', extra: { executorId: this.executorId }, }), }); @@ -179,7 +170,6 @@ export class RequestReplyExecutor { ...extractErrorLogFields(err), ...reportableErrorFields({ error: err, - operation: 'requestReplyHeartbeat', extra: { executorId: this.executorId }, }), }); @@ -218,7 +208,6 @@ export class RequestReplyExecutor { executorId: this.executorId, ...reportableErrorFields({ error: err, - operation: 'requestReplyInvalidJson', extra: { executorId: this.executorId }, }), }); @@ -243,7 +232,6 @@ export class RequestReplyExecutor { zod: zodError, ...reportableErrorFields({ error: parsedRequest.error, - operation: 'requestReplyInvalidShape', extra: { executorId: this.executorId }, }), }); @@ -280,7 +268,6 @@ export class RequestReplyExecutor { ...extractErrorLogFields(err), ...reportableErrorFields({ error: err, - operation: 'requestReplyProcessing', extra: { executorId: this.executorId }, }), }); diff --git a/packages/trueforge/src/apis/auth.ts b/packages/trueforge/src/apis/auth.ts index c90c55bc15..f95ddbd178 100644 --- a/packages/trueforge/src/apis/auth.ts +++ b/packages/trueforge/src/apis/auth.ts @@ -67,7 +67,7 @@ export function createAuthRouter(params: { } catch (error) { params.logger.error('Failed to build login authorization', { ...extractErrorLogFields(error), - ...reportableErrorFields({ error, operation: 'loginAuthorization' }), + ...reportableErrorFields({ error }), }); return c.redirect(oauthErrorRedirect('login_failed'), 302); } @@ -79,7 +79,7 @@ export function createAuthRouter(params: { } catch (error) { params.logger.error('Failed to build TrueFoundry external login URL', { ...extractErrorLogFields(error), - ...reportableErrorFields({ error, operation: 'externalLoginUrl' }), + ...reportableErrorFields({ error }), }); return c.redirect(oauthErrorRedirect('login_failed'), 302); } @@ -126,7 +126,7 @@ export function createAuthRouter(params: { } catch (error) { params.logger.error('Failed to exchange authorization code', { ...extractErrorLogFields(error), - ...(isReportableTransportFailure(error) ? reportableErrorFields({ error, operation: 'oidcExchange' }) : {}), + ...(isReportableTransportFailure(error) ? reportableErrorFields({ error }) : {}), }); const soft = await redirectIfAlreadyAuthenticated({ context: c, diff --git a/packages/trueforge/src/apis/mcpOAuth.ts b/packages/trueforge/src/apis/mcpOAuth.ts index 2ec48b96ae..f150a6b12a 100644 --- a/packages/trueforge/src/apis/mcpOAuth.ts +++ b/packages/trueforge/src/apis/mcpOAuth.ts @@ -121,7 +121,7 @@ export function createMcpOAuthRouter(deps: McpOAuthRouterDeps(deps: McpServersRouterDeps< } deps.logger.error(`MCP authorize unexpected failure for "${name}"`, { ...extractErrorLogFields(error), - ...reportableErrorFields({ error, operation: 'mcpAuthorizeUnexpected', extra: { mcp_server_name: name } }), + ...reportableErrorFields({ error, extra: { mcp_server_name: name } }), }); return c.json({ error: { message: 'Internal server error' } }, 500); } diff --git a/packages/trueforge/src/apis/sessions.ts b/packages/trueforge/src/apis/sessions.ts index 1925890464..e288e85d72 100644 --- a/packages/trueforge/src/apis/sessions.ts +++ b/packages/trueforge/src/apis/sessions.ts @@ -204,12 +204,12 @@ export async function cancelSessionTurn( if (error instanceof RequestTimeoutError) { deps.logger.warn('Timed out waiting for owning executor to cancel; freezing the running turn', { ...fields, - ...reportableErrorFields({ error, operation: 'cancelPeerTimeout', extra: { turn_id: turnId, owner } }), + ...reportableErrorFields({ error, extra: { turn_id: turnId, owner } }), }); } else { deps.logger.warn('Failed to reach owning executor over Redis; freezing the running turn', { ...fields, - ...reportableErrorFields({ error, operation: 'cancelPeerRedis', extra: { turn_id: turnId, owner } }), + ...reportableErrorFields({ error, extra: { turn_id: turnId, owner } }), }); } } diff --git a/packages/trueforge/src/apis/turns.ts b/packages/trueforge/src/apis/turns.ts index e2984aee10..0ca1fd22d1 100644 --- a/packages/trueforge/src/apis/turns.ts +++ b/packages/trueforge/src/apis/turns.ts @@ -405,7 +405,7 @@ export async function drainTurnEvents(input: { } else { logger.error('Unexpected error in turn event drain', { ...extractErrorLogFields(error), - ...reportableErrorFields({ error, operation: 'eventDrain' }), + ...reportableErrorFields({ error }), }); } } finally { @@ -729,7 +729,7 @@ export function createTurnsRouter(deps: TurnsRouterDeps) { ...extractErrorLogFields(error), sandboxId, path, - ...reportableErrorFields({ error, operation: 'sandboxFileDownload', extra: { sandboxId, path } }), + ...reportableErrorFields({ error, extra: { sandboxId, path } }), }); return c.json({ error: { message: 'Failed to download file from sandbox' } }, 424); } @@ -975,7 +975,7 @@ export function createTurnsRouter(deps: TurnsRouterDeps) { if (!subscribeAbort.signal.aborted) { deps.logger.error('Unexpected error in turn subscribe SSE loop', { ...extractErrorLogFields(error), - ...reportableErrorFields({ error, operation: 'subscribeSse' }), + ...reportableErrorFields({ error }), }); } } finally { diff --git a/packages/trueforge/src/app.ts b/packages/trueforge/src/app.ts index 9e519d2376..cf0ad99aea 100644 --- a/packages/trueforge/src/app.ts +++ b/packages/trueforge/src/app.ts @@ -159,12 +159,7 @@ export function createAppErrorHandler(params: { logger: Logger }): ErrorHandler path: c.req.path, status, ...extractErrorLogFields(error), - ...(status >= 500 - ? reportableErrorFields({ - error, - operation: message === 'Unhandled error' ? 'unhandled' : 'serverApiError', - }) - : {}), + ...(status >= 500 ? reportableErrorFields({ error }) : {}), }; if (status >= 500) { params.logger.error(message, fields); diff --git a/packages/trueforge/src/controller.ts b/packages/trueforge/src/controller.ts index 092ac34f7c..da7f3c2bd4 100644 --- a/packages/trueforge/src/controller.ts +++ b/packages/trueforge/src/controller.ts @@ -58,7 +58,6 @@ export function runController(params: { logger.warn(`Controller drain timed out after ${String(gracefulTimeoutSeconds)}s, exiting`, { ...reportableErrorFields({ error: new Error(`Controller drain timed out after ${String(gracefulTimeoutSeconds)}s, exiting`), - operation: 'controllerDrainTimeout', }), }); process.exit(1); diff --git a/packages/trueforge/src/controller/Controller.ts b/packages/trueforge/src/controller/Controller.ts index c41aeb0960..57088e0363 100644 --- a/packages/trueforge/src/controller/Controller.ts +++ b/packages/trueforge/src/controller/Controller.ts @@ -123,7 +123,7 @@ export class Controller { this.#logger.error('Control loop pass failed', { loop: loop.name, error, - ...reportableErrorFields({ error, operation: 'controlLoop', extra: { loop: loop.name } }), + ...reportableErrorFields({ error, extra: { loop: loop.name } }), }); } })(); diff --git a/packages/trueforge/src/controller/sandboxEnvBuild.ts b/packages/trueforge/src/controller/sandboxEnvBuild.ts index bad1080880..4d8b9b4a60 100644 --- a/packages/trueforge/src/controller/sandboxEnvBuild.ts +++ b/packages/trueforge/src/controller/sandboxEnvBuild.ts @@ -26,7 +26,6 @@ export async function dispatchSandboxEnvBuilds({ logger }: { logger: Logger }): environment_version_id: environmentVersionId, ...reportableErrorFields({ error, - operation: 'progressNotFound', extra: { environment_version_id: environmentVersionId }, }), }); @@ -37,7 +36,6 @@ export async function dispatchSandboxEnvBuilds({ logger }: { logger: Logger }): error: error instanceof Error ? error.message : String(error), ...reportableErrorFields({ error, - operation: 'progress', extra: { environment_version_id: environmentVersionId }, }), }); diff --git a/packages/trueforge/src/controller/scheduleDispatch.ts b/packages/trueforge/src/controller/scheduleDispatch.ts index 40f7eae7b1..57523e8563 100644 --- a/packages/trueforge/src/controller/scheduleDispatch.ts +++ b/packages/trueforge/src/controller/scheduleDispatch.ts @@ -297,7 +297,6 @@ export async function dispatchScheduledRuns(params: { error, ...reportableErrorFields({ error, - operation: 'handoff', extra: { tenant_id: run.tenant_id, schedule_id: schedule.id, @@ -332,7 +331,6 @@ export async function dispatchScheduledRuns(params: { error, ...reportableErrorFields({ error, - operation: 'processRun', extra: { tenant_id: run.tenant_id, schedule_id: run.schedule_id, diff --git a/packages/trueforge/src/main.ts b/packages/trueforge/src/main.ts index 95dd74dd37..22fea88ba6 100644 --- a/packages/trueforge/src/main.ts +++ b/packages/trueforge/src/main.ts @@ -868,7 +868,7 @@ try { requestReplySubscriber.on('error', (error: Error) => { logger.error('[RedisSubscriber] Client error', { ...extractErrorLogFields(error), - ...reportableErrorFields({ error, operation: 'redisSubscriber' }), + ...reportableErrorFields({ error }), }); }); await requestReplySubscriber.connect(); @@ -931,7 +931,6 @@ try { logger.warn(`Drain timed out after ${String(configuration.GRACEFUL_TIMEOUT_SECONDS)}s, exiting`, { ...reportableErrorFields({ error: new Error(`Drain timed out after ${String(configuration.GRACEFUL_TIMEOUT_SECONDS)}s, exiting`), - operation: 'serverDrainTimeout', }), }); process.exit(1); diff --git a/packages/trueforge/src/runtime/redis.ts b/packages/trueforge/src/runtime/redis.ts index 6f8116618c..b6b06b8817 100644 --- a/packages/trueforge/src/runtime/redis.ts +++ b/packages/trueforge/src/runtime/redis.ts @@ -154,7 +154,7 @@ async function connectSentinelWithRetry(input: { client.on('error', (error: Error) => { input.logger.error('[Redis] Client error', { ...extractErrorLogFields(error), - ...reportableErrorFields({ error, operation: 'redisClient' }), + ...reportableErrorFields({ error }), }); }); try { @@ -174,7 +174,7 @@ async function connectSentinelWithRetry(input: { input.logger.error('[Redis] Sentinel connect failed, rebuilding and retrying', { attempt, ...extractErrorLogFields(error), - ...reportableErrorFields({ error, operation: 'redisSentinelConnect', extra: { attempt } }), + ...reportableErrorFields({ error, extra: { attempt } }), }); await sleep(Math.min(SENTINEL_RETRY_BASE_MS * 2 ** attempt, SENTINEL_RETRY_MAX_MS)); } @@ -221,7 +221,7 @@ export async function connectRedis(input: ConnectRedisInput): Promise { input.logger.error('[Redis] Client error', { ...extractErrorLogFields(error), - ...reportableErrorFields({ error, operation: 'redisClient' }), + ...reportableErrorFields({ error }), }); }); await created.connect(); diff --git a/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts b/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts index de273ab89b..14f929f620 100644 --- a/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts +++ b/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts @@ -34,7 +34,6 @@ export async function progressSandboxEnvironmentVersion({ environment_version_id, ...reportableErrorFields({ error, - operation: 'notFound', extra: { environment_version_id }, }), }); @@ -85,7 +84,6 @@ export async function progressSandboxEnvironmentVersion({ status_reason, ...reportableErrorFields({ error, - operation: 'authz', extra: { environment_version_id, tenant_id: pending.tenant_id, status_reason }, }), }); diff --git a/packages/trueforge/src/sentry/reportMarkedLog.ts b/packages/trueforge/src/sentry/reportMarkedLog.ts index f9e290c8a3..aad5236729 100644 --- a/packages/trueforge/src/sentry/reportMarkedLog.ts +++ b/packages/trueforge/src/sentry/reportMarkedLog.ts @@ -12,20 +12,27 @@ function readExtra(value: unknown): Record | undefined { return extra; } -function readReportableError( - value: unknown, -): { error: unknown; operation: string; extra: Record | undefined } | undefined { - if (typeof value !== 'object' || value === null || !('operation' in value) || !('error' in value)) { +function readFilePath(value: unknown): string | undefined { + if (typeof value !== 'object' || value === null || !('filepath' in value)) { + return undefined; + } + const filepath = Reflect.get(value, 'filepath'); + if (typeof filepath !== 'string' || filepath.length === 0) { return undefined; } - const operation = Reflect.get(value, 'operation'); - if (typeof operation !== 'string' || operation.length === 0) { + return filepath; +} + +function readReportableError( + value: unknown, +): { error: unknown; extra: Record | undefined; filepath: string | undefined } | undefined { + if (typeof value !== 'object' || value === null || !('error' in value)) { return undefined; } return { error: Reflect.get(value, 'error'), - operation, extra: readExtra(Reflect.get(value, 'extra')), + filepath: readFilePath(value), }; } @@ -46,7 +53,7 @@ export function reportMarkedLogFormat(): winston.Logform.Format { const error = toError(marked.error); if (error !== undefined) { captureCriticalException(error, { - tags: { operation: marked.operation }, + ...(marked.filepath !== undefined ? { tags: { filepath: marked.filepath } } : {}), ...(marked.extra !== undefined ? { extra: marked.extra } : {}), }); } diff --git a/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts b/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts index 3267a26bfb..44d7976de6 100644 --- a/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts +++ b/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts @@ -272,7 +272,7 @@ export class TrueFoundryServiceFoundryServerClient { if (!parsed.success) { this.#logger.error('TrueFoundry ServiceFoundry put remote agent returned an unexpected response', { ...extractErrorLogFields(parsed.error), - ...reportableErrorFields({ error: parsed.error, operation: 'putRemoteAgent' }), + ...reportableErrorFields({ error: parsed.error }), }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry put remote agent returned an unexpected response', @@ -423,7 +423,7 @@ export class TrueFoundryServiceFoundryServerClient { this.#logger.error('TrueFoundry ServiceFoundry MCP authorize returned an unexpected response', { mcpServerId: input.mcpServerId, ...extractErrorLogFields(error), - ...reportableErrorFields({ error, operation: 'mcpAuthorize', extra: { mcp_server_id: input.mcpServerId } }), + ...reportableErrorFields({ error, extra: { mcp_server_id: input.mcpServerId } }), }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry MCP authorize returned an unexpected response', @@ -453,7 +453,7 @@ export class TrueFoundryServiceFoundryServerClient { this.#logger.error('TrueFoundry ServiceFoundry MCP auth status returned an unexpected response', { mcpServerId: input.mcpServerId, ...extractErrorLogFields(error), - ...reportableErrorFields({ error, operation: 'mcpAuthStatus', extra: { mcp_server_id: input.mcpServerId } }), + ...reportableErrorFields({ error, extra: { mcp_server_id: input.mcpServerId } }), }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry MCP auth status returned an unexpected response', @@ -569,7 +569,7 @@ export class TrueFoundryServiceFoundryServerClient { if (!parsed.success) { this.#logger.error('TrueFoundry ServiceFoundry agent permissions response was malformed', { ...extractErrorLogFields(parsed.error), - ...reportableErrorFields({ error: parsed.error, operation: 'agentPermissions' }), + ...reportableErrorFields({ error: parsed.error }), }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry agent permissions response was malformed', @@ -593,7 +593,7 @@ export class TrueFoundryServiceFoundryServerClient { if (!parsed.success) { this.#logger.error('TrueFoundry ServiceFoundry tenant permissions response was malformed', { ...extractErrorLogFields(parsed.error), - ...reportableErrorFields({ error: parsed.error, operation: 'tenantPermissions' }), + ...reportableErrorFields({ error: parsed.error }), }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry tenant permissions response was malformed', @@ -629,7 +629,7 @@ export class TrueFoundryServiceFoundryServerClient { if (!parsed.success) { this.#logger.error('TrueFoundry ServiceFoundry vend-token response was malformed', { ...extractErrorLogFields(parsed.error), - ...reportableErrorFields({ error: parsed.error, operation: 'vendToken' }), + ...reportableErrorFields({ error: parsed.error }), }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry vend-token response was malformed', @@ -644,7 +644,7 @@ export class TrueFoundryServiceFoundryServerClient { if (!parsed.success) { this.#logger.error('TrueFoundry ServiceFoundry server returned an unexpected list response', { ...extractErrorLogFields(parsed.error), - ...reportableErrorFields({ error: parsed.error, operation: 'unexpectedList' }), + ...reportableErrorFields({ error: parsed.error }), }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry server returned an unexpected list response', @@ -743,7 +743,7 @@ export class TrueFoundryServiceFoundryServerClient { this.#logger.error('TrueFoundry ServiceFoundry server returned non-JSON', { url: input.url.href, ...extractErrorLogFields(error), - ...reportableErrorFields({ error, operation: 'serviceFoundryNonJson' }), + ...reportableErrorFields({ error }), }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry server returned non-JSON', diff --git a/packages/trueforge/tests/unit/appErrorHandler.test.ts b/packages/trueforge/tests/unit/appErrorHandler.test.ts index 2564c950c3..9e3659d92b 100644 --- a/packages/trueforge/tests/unit/appErrorHandler.test.ts +++ b/packages/trueforge/tests/unit/appErrorHandler.test.ts @@ -1,5 +1,4 @@ import { OpenAPIHono, createRoute, z } from '@hono/zod-openapi'; -import { reportableErrorFields } from '@truefoundry/trueforge-core/core'; import { HTTPException } from 'hono/http-exception'; import winston from 'winston'; import { createAppErrorHandler } from '../../src/app'; @@ -42,7 +41,10 @@ describe('createAppErrorHandler', () => { status: 500, error: thrown.message, stack: thrown.stack, - ...reportableErrorFields({ error: thrown, operation: 'unhandled' }), + reportable_error: expect.objectContaining({ + error: thrown, + filepath: expect.stringContaining('trueforge/src/app.ts'), + }), }); }); @@ -62,7 +64,10 @@ describe('createAppErrorHandler', () => { status, error: thrown.message, stack: thrown.stack, - ...reportableErrorFields({ error: thrown, operation: 'serverApiError' }), + reportable_error: expect.objectContaining({ + error: thrown, + filepath: expect.stringContaining('trueforge/src/app.ts'), + }), }); }); @@ -81,7 +86,10 @@ describe('createAppErrorHandler', () => { status: 500, error: `${thrown.message}: ${cause.message}`, stack: `${thrown.stack ?? ''}\nCaused by: ${cause.stack ?? ''}`, - ...reportableErrorFields({ error: thrown, operation: 'serverApiError' }), + reportable_error: expect.objectContaining({ + error: thrown, + filepath: expect.stringContaining('trueforge/src/app.ts'), + }), }); }); diff --git a/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts b/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts index a2b9f6b10f..1432d7c6b4 100644 --- a/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts +++ b/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts @@ -45,7 +45,7 @@ describe('sandboxEnvBuildLoop', () => { 'Sandbox environment version not found; skipping', expect.objectContaining({ environment_version_id: 'gone', - reportable_error: expect.objectContaining({ operation: 'progressNotFound' }), + reportable_error: expect.objectContaining({ error: expect.any(TrueForgeApi.NotFoundError) }), }), ); }); @@ -61,7 +61,7 @@ describe('sandboxEnvBuildLoop', () => { expect(error).toHaveBeenCalledWith( 'Sandbox environment version progress failed', expect.objectContaining({ - reportable_error: expect.objectContaining({ operation: 'progress' }), + reportable_error: expect.objectContaining({ error: expect.any(Error) }), }), ); }); diff --git a/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts b/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts index 04095d7956..d607903e2b 100644 --- a/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts +++ b/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts @@ -89,7 +89,7 @@ describe('progressSandboxEnvironmentVersion', () => { 'Sandbox environment version not found', expect.objectContaining({ environment_version_id: 'missing', - reportable_error: expect.objectContaining({ operation: 'notFound' }), + reportable_error: expect.objectContaining({ error: expect.any(HTTPException) }), }), ); }); @@ -136,7 +136,7 @@ describe('progressSandboxEnvironmentVersion', () => { expect(warn).toHaveBeenCalledWith( 'Sandbox environment build failed authz', expect.objectContaining({ - reportable_error: expect.objectContaining({ operation: 'authz' }), + reportable_error: expect.objectContaining({ error: authError }), }), ); }); diff --git a/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts b/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts index cccc8ef5d4..7ba0b66cb5 100644 --- a/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts +++ b/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts @@ -33,11 +33,11 @@ describe('reportMarkedLogFormat', () => { const { logger, lines } = loggedLines(); logger.error('Unhandled error', { error: 'boom', - ...reportableErrorFields({ error: failure, operation: 'unhandled', extra: { path: '/api' } }), + ...reportableErrorFields({ error: failure, extra: { path: '/api' } }), }); expect(captureCriticalException).toHaveBeenCalledWith(failure, { - tags: { operation: 'unhandled' }, + tags: { filepath: expect.stringContaining('reportMarkedLog.test.ts') }, extra: { path: '/api' }, }); expect(lines.join('')).not.toContain('reportable_error'); @@ -51,11 +51,11 @@ describe('reportMarkedLogFormat', () => { expect(captureCriticalException).not.toHaveBeenCalled(); }); - it('reports each repeated operation', () => { + it('reports each repeated marked log', () => { const { logger } = loggedLines(); const failure = new Error('socket closed'); - logger.error('[Redis] Client error', reportableErrorFields({ error: failure, operation: 'redisClient' })); - logger.error('[Redis] Client error', reportableErrorFields({ error: failure, operation: 'redisClient' })); + logger.error('[Redis] Client error', reportableErrorFields({ error: failure })); + logger.error('[Redis] Client error', reportableErrorFields({ error: failure })); expect(captureCriticalException).toHaveBeenCalledTimes(2); }); }); From cde911509f76fc52588639c49094083346eca216 Mon Sep 17 00:00:00 2001 From: Raman Tehlan Date: Sat, 24 Oct 2026 15:59:55 +0530 Subject: [PATCH 06/13] review comments --- .changeset/sentry-allowlisted-logs.md | 2 +- packages/trueforge-core/src/core/index.ts | 2 +- .../src/core/llm/VercelAILLM.ts | 14 +-- .../sandbox/codeMode/CodeModeDispatcher.ts | 2 +- .../src/core/util/errorLogFields.ts | 65 ++++------- .../tests/core/llm/VercelAILLM.stream.test.ts | 27 +++-- .../tests/core/util/errorLogFields.test.ts | 31 +++++- packages/trueforge/src/apis/auth.ts | 8 +- packages/trueforge/src/apis/turns.ts | 27 +++++ packages/trueforge/src/sentry/initSentry.ts | 2 +- .../trueforge/src/sentry/reportMarkedLog.ts | 27 ++++- .../src/truefoundry/initTrueFoundrySentry.ts | 2 +- .../trueforge/tests/unit/apis/turns.test.ts | 101 +++++++++++++++++- .../tests/unit/sentry/reportMarkedLog.test.ts | 13 ++- 14 files changed, 239 insertions(+), 84 deletions(-) diff --git a/.changeset/sentry-allowlisted-logs.md b/.changeset/sentry-allowlisted-logs.md index b7e0183ef8..f77ddff47e 100644 --- a/.changeset/sentry-allowlisted-logs.md +++ b/.changeset/sentry-allowlisted-logs.md @@ -3,4 +3,4 @@ '@truefoundry/trueforge-core': patch --- -Report actionable network, timeout, and internal failures to Sentry when the log is explicitly marked. Those events include the active tenant, request, session, and turn ids, and are tagged with the file that logged them. Provider inference errors, invalid credentials, and client disconnects stay in the logs. Existing direct Sentry captures are unchanged. +Report logged failures to Sentry except HTTP statuses from 200 through 499. The log message is the Sentry title, and the thrown error is the cause and stack. Those events include the active tenant, request, session, and turn ids, and are tagged with the file that logged them. Existing direct Sentry captures are unchanged. diff --git a/packages/trueforge-core/src/core/index.ts b/packages/trueforge-core/src/core/index.ts index 6033fa4381..a31d0119c4 100644 --- a/packages/trueforge-core/src/core/index.ts +++ b/packages/trueforge-core/src/core/index.ts @@ -158,7 +158,7 @@ export type { // Errors / utils export { AgentHarnessError, McpConnectionError, McpDcrConfigurationError } from './errors'; export { REDIS_KEY_NAMESPACE, redisKey } from './redisKeys'; -export { describeUnknownError, extractErrorLogFields, isReportableTransportFailure } from './util/errorLogFields'; +export { describeUnknownError, extractErrorLogFields, isReportableError } from './util/errorLogFields'; export { DEFAULT_MCP_HTTP_BODY_TIMEOUT_MS, DEFAULT_MCP_HTTP_CONNECT_TIMEOUT_MS, diff --git a/packages/trueforge-core/src/core/llm/VercelAILLM.ts b/packages/trueforge-core/src/core/llm/VercelAILLM.ts index 4aaac4ae9f..d991527a6e 100644 --- a/packages/trueforge-core/src/core/llm/VercelAILLM.ts +++ b/packages/trueforge-core/src/core/llm/VercelAILLM.ts @@ -35,7 +35,7 @@ import type { } from 'openai/resources/chat'; import type { Logger } from 'winston'; import { ModelProviderError } from '../errors'; -import { describeUnknownError, extractErrorLogFields, isReportableTransportFailure } from '../util/errorLogFields'; +import { describeUnknownError, extractErrorLogFields, isReportableError } from '../util/errorLogFields'; import { modelSsrfFetch } from '../util/outboundFetch'; import { reportableErrorFields } from '../util/reportableErrorLog'; import type { ILLM, LLMCreateParams, LLMCreateParamsStreaming } from './ILLM'; @@ -993,14 +993,6 @@ export function normalizeUsage(usage: { }; } -/** Provider HTTP responses stay in the log. Only a dropped connection or timeout is reportable. */ -export function isReportableLlmTransportFailure(error: unknown): boolean { - if (APICallError.isInstance(error) && error.statusCode != null) { - return false; - } - return isReportableTransportFailure(error); -} - /** * Prefer AI SDK `APICallError` (`message` / `statusCode`): * https://ai-sdk.dev/docs/reference/ai-sdk-errors/ai-api-call-error#properties @@ -1458,7 +1450,7 @@ export class VercelAILLM implements ILLM { } else { this.logger.error('Error creating streaming chat completion', { ...extractErrorLogFields(streamError), - ...(isReportableLlmTransportFailure(error) ? reportableErrorFields({ error: streamError }) : {}), + ...(isReportableError(error) ? reportableErrorFields({ error: streamError }) : {}), }); } throw streamError; @@ -1500,7 +1492,7 @@ export class VercelAILLM implements ILLM { } else { this.logger.error('Error reading LLM stream', { ...extractErrorLogFields(streamError), - ...(isReportableLlmTransportFailure(error) ? reportableErrorFields({ error: streamError }) : {}), + ...(isReportableError(error) ? reportableErrorFields({ error: streamError }) : {}), }); } throw streamError; diff --git a/packages/trueforge-core/src/core/sandbox/codeMode/CodeModeDispatcher.ts b/packages/trueforge-core/src/core/sandbox/codeMode/CodeModeDispatcher.ts index 04df28c24c..108f957705 100644 --- a/packages/trueforge-core/src/core/sandbox/codeMode/CodeModeDispatcher.ts +++ b/packages/trueforge-core/src/core/sandbox/codeMode/CodeModeDispatcher.ts @@ -90,7 +90,7 @@ export class CodeModeDispatcher { const errorMessage = e instanceof Error ? e.message : 'Unknown Code Mode dispatch error'; this.logger.error(`Code Mode dispatch failed (source=${source})`, { ...extractErrorLogFields(e), - ...(source === 'caller' ? {} : reportableErrorFields({ error: e, extra: { source } })), + ...(source === 'internal' ? reportableErrorFields({ error: e, extra: { source } }) : {}), }); return { ok: false, error: errorMessage, source }; } diff --git a/packages/trueforge-core/src/core/util/errorLogFields.ts b/packages/trueforge-core/src/core/util/errorLogFields.ts index f9abe4600b..05b81f49d5 100644 --- a/packages/trueforge-core/src/core/util/errorLogFields.ts +++ b/packages/trueforge-core/src/core/util/errorLogFields.ts @@ -107,63 +107,38 @@ function stackWithCauses(error: Error): string | undefined { return stacks.length > 0 ? stacks.join('\nCaused by: ') : undefined; } -const REPORTABLE_TRANSPORT_ERROR_NAMES = new Set([ - 'AbortError', - 'TimeoutError', - 'ConnectTimeoutError', - 'HeadersTimeoutError', - 'BodyTimeoutError', -]); - -const REPORTABLE_TRANSPORT_ERROR_CODES = new Set([ - 'ECONNRESET', - 'ECONNREFUSED', - 'ETIMEDOUT', - 'ENOTFOUND', - 'EAI_AGAIN', - 'EPIPE', - 'EHOSTUNREACH', - 'ENETUNREACH', - 'ECONNABORTED', - 'UND_ERR_CONNECT_TIMEOUT', - 'UND_ERR_HEADERS_TIMEOUT', - 'UND_ERR_BODY_TIMEOUT', - 'UND_ERR_SOCKET', - 'CERT_HAS_EXPIRED', - 'UNABLE_TO_VERIFY_LEAF_SIGNATURE', - 'DEPTH_ZERO_SELF_SIGNED_CERT', - 'ERR_TLS_CERT_ALTNAME_INVALID', -]); - -function transportErrorCode(error: object): string | undefined { - if (!('code' in error)) { +function readIntegerProperty(error: object, key: string): number | undefined { + if (!(key in error)) { return undefined; } - const code: unknown = Reflect.get(error, 'code'); - return typeof code === 'string' ? code : undefined; + const value: unknown = Reflect.get(error, key); + return typeof value === 'number' && Number.isInteger(value) ? value : undefined; } -function hasReportableTransportSignal(error: unknown, seen: Set): boolean { +/** `statusCode` (AI SDK, MCP) or `status` (OAuth, HTTPException). */ +function readHttpStatus(error: object): number | undefined { + return readIntegerProperty(error, 'statusCode') ?? readIntegerProperty(error, 'status'); +} + +function httpStatusInChain(error: unknown, seen: Set): number | undefined { if (typeof error !== 'object' || error === null || seen.has(error)) { - return false; + return undefined; } seen.add(error); - if (error instanceof Error && REPORTABLE_TRANSPORT_ERROR_NAMES.has(error.name)) { - return true; - } - const code = transportErrorCode(error); - if (code !== undefined && REPORTABLE_TRANSPORT_ERROR_CODES.has(code)) { - return true; + const status = readHttpStatus(error); + if (status !== undefined) { + return status; } if (!('cause' in error)) { - return false; + return undefined; } - return hasReportableTransportSignal(Reflect.get(error, 'cause'), seen); + return httpStatusInChain(Reflect.get(error, 'cause'), seen); } -/** Timeout, DNS, TLS, and socket failures, including those nested on `error.cause`. */ -export function isReportableTransportFailure(error: unknown): boolean { - return hasReportableTransportSignal(error, new Set()); +/** Every failure is reported except an HTTP status from 200 through 499, including on `cause`. */ +export function isReportableError(error: unknown): boolean { + const status = httpStatusInChain(error, new Set()); + return status === undefined || status < 200 || status > 499; } export function extractErrorLogFields(error: unknown): ErrorLogFields { diff --git a/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts b/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts index f3d9c8f90f..b196c98935 100644 --- a/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts +++ b/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts @@ -15,12 +15,12 @@ import { ModelProviderError } from '../../../src/core/errors'; import type { ExtendedChatCompletionChunk, RawAssistantMessageWithUsage } from '../../../src/core/llm/LLMTypes'; import { describeStreamError, - isReportableLlmTransportFailure, mapFinishReason, mapStreamToChunks, normalizeUsage, toStreamError, } from '../../../src/core/llm/VercelAILLM'; +import { isReportableError } from '../../../src/core/util/errorLogFields'; // ─────────── Helpers ─────────── @@ -687,8 +687,8 @@ describe('mapStreamToChunks', () => { }); }); -describe('isReportableLlmTransportFailure', () => { - it('reports timeouts and dropped connections', () => { +describe('isReportableError', () => { + it('reports timeouts, dropped connections, and provider 5xx', () => { const timedOut = new Error('timed out'); timedOut.name = 'TimeoutError'; const refused = new TypeError('fetch failed', { @@ -700,16 +700,24 @@ describe('isReportableLlmTransportFailure', () => { requestBodyValues: {}, cause: Object.assign(new Error('socket hang up'), { code: 'ECONNRESET' }), }); + const providerOutage = new APICallError({ + message: 'provider unavailable', + url: 'https://example.com', + requestBodyValues: {}, + statusCode: 503, + }); - expect(isReportableLlmTransportFailure(timedOut)).toBe(true); - expect(isReportableLlmTransportFailure(refused)).toBe(true); - expect(isReportableLlmTransportFailure(providerNetwork)).toBe(true); + expect(isReportableError(timedOut)).toBe(true); + expect(isReportableError(refused)).toBe(true); + expect(isReportableError(providerNetwork)).toBe(true); + expect(isReportableError(providerOutage)).toBe(true); + expect(isReportableError(new Error('model produced an invalid tool call'))).toBe(true); }); - it('skips provider HTTP responses, including rate limits and provider 5xx', () => { - for (const statusCode of [401, 429, 503]) { + it('skips provider responses from 200 through 499', () => { + for (const statusCode of [200, 401, 429, 499]) { expect( - isReportableLlmTransportFailure( + isReportableError( new APICallError({ message: 'provider rejected the request', url: 'https://example.com', @@ -720,6 +728,5 @@ describe('isReportableLlmTransportFailure', () => { ), ).toBe(false); } - expect(isReportableLlmTransportFailure(new Error('model produced an invalid tool call'))).toBe(false); }); }); diff --git a/packages/trueforge-core/tests/core/util/errorLogFields.test.ts b/packages/trueforge-core/tests/core/util/errorLogFields.test.ts index 2a9e89a730..3ec584f88e 100644 --- a/packages/trueforge-core/tests/core/util/errorLogFields.test.ts +++ b/packages/trueforge-core/tests/core/util/errorLogFields.test.ts @@ -1,4 +1,4 @@ -import { describeUnknownError, extractErrorLogFields } from '../../../src/core/util/errorLogFields'; +import { describeUnknownError, extractErrorLogFields, isReportableError } from '../../../src/core/util/errorLogFields'; describe('extractErrorLogFields', () => { it('returns the message and stack for a plain error', () => { @@ -53,6 +53,35 @@ describe('extractErrorLogFields', () => { }); }); +describe('isReportableError', () => { + it('reports errors with no HTTP status', () => { + expect(isReportableError(new Error('boom'))).toBe(true); + expect(isReportableError('plain string')).toBe(true); + }); + + it('skips statuses from 200 through 499 on the error or its cause', () => { + expect(isReportableError(Object.assign(new Error('unauthorized'), { statusCode: 401 }))).toBe(false); + expect(isReportableError(Object.assign(new Error('invalid grant'), { status: 400 }))).toBe(false); + const wrapped = new Error('fetch failed', { + cause: Object.assign(new Error('bad request'), { status: 400 }), + }); + expect(isReportableError(wrapped)).toBe(false); + }); + + it('reports statuses outside 200 through 499, using the outermost status', () => { + expect(isReportableError(Object.assign(new Error('unavailable'), { statusCode: 503 }))).toBe(true); + expect(isReportableError(Object.assign(new Error('bad gateway'), { status: 502 }))).toBe(true); + expect( + isReportableError( + Object.assign(new Error('unavailable'), { + statusCode: 503, + cause: Object.assign(new Error('unauthorized'), { statusCode: 401 }), + }), + ), + ).toBe(true); + }); +}); + describe('describeUnknownError', () => { it('joins the cause chain', () => { const error = new Error('fetch failed', { cause: new Error('ECONNREFUSED') }); diff --git a/packages/trueforge/src/apis/auth.ts b/packages/trueforge/src/apis/auth.ts index f95ddbd178..e75963ed65 100644 --- a/packages/trueforge/src/apis/auth.ts +++ b/packages/trueforge/src/apis/auth.ts @@ -1,9 +1,5 @@ import { OpenAPIHono } from '@hono/zod-openapi'; -import { - extractErrorLogFields, - isReportableTransportFailure, - reportableErrorFields, -} from '@truefoundry/trueforge-core/core'; +import { extractErrorLogFields, isReportableError, reportableErrorFields } from '@truefoundry/trueforge-core/core'; import type { Context, MiddlewareHandler } from 'hono'; import type { Configuration } from 'openid-client'; import type { Logger } from 'winston'; @@ -126,7 +122,7 @@ export function createAuthRouter(params: { } catch (error) { params.logger.error('Failed to exchange authorization code', { ...extractErrorLogFields(error), - ...(isReportableTransportFailure(error) ? reportableErrorFields({ error }) : {}), + ...(isReportableError(error) ? reportableErrorFields({ error }) : {}), }); const soft = await redirectIfAlreadyAuthenticated({ context: c, diff --git a/packages/trueforge/src/apis/turns.ts b/packages/trueforge/src/apis/turns.ts index 0ca1fd22d1..17ee40c0ee 100644 --- a/packages/trueforge/src/apis/turns.ts +++ b/packages/trueforge/src/apis/turns.ts @@ -378,6 +378,32 @@ export function turnStreamId(tenantId: string, sessionId: string, turnId: string return redisKey('agent', 'turn', tenantId, sessionId, turnId, 'stream'); } +/** Client cancel and a turn replaced by the next one are expected endings. */ +function reportUnexpectedTurnDone({ event, logger }: { event: TurnStreamingEvent; logger: Logger }): void { + if (event.type !== EventType.TURN_DONE) { + return; + } + const { state } = event; + if (state.status === 'error') { + const error = new Error(state.message); + logger.error('turn.done error', { + ...extractErrorLogFields(error), + ...reportableErrorFields({ error }), + }); + return; + } + if ( + state.status === 'cancelled' && + (state.reason === CancellationReason.Abandoned || state.reason === CancellationReason.ServerExecutionTimeout) + ) { + const error = new Error(state.reason); + logger.error('turn.done cancelled', { + ...extractErrorLogFields(error), + ...reportableErrorFields({ error, extra: { reason: state.reason } }), + }); + } +} + /** * Dual-write turn events to the resumable subscription registry, then optionally * forward each sequenced event (SSE path). Shared by streaming and non-streaming @@ -393,6 +419,7 @@ export async function drainTurnEvents(input: { const { trackedStream, turnEventStream, maxExecutionTimer, logger, onEvent } = input; try { for await (const event of trackedStream) { + reportUnexpectedTurnDone({ event, logger }); // Dual-write before any client sink so subscribers can resume after disconnect. const sequenceNumber = await turnEventStream.put(event, { streamTTLSeconds: streamTTLSecondsFor(event), diff --git a/packages/trueforge/src/sentry/initSentry.ts b/packages/trueforge/src/sentry/initSentry.ts index b3d4719391..53c5df64b8 100644 --- a/packages/trueforge/src/sentry/initSentry.ts +++ b/packages/trueforge/src/sentry/initSentry.ts @@ -59,7 +59,7 @@ export async function initSentry( dsn, includeLocalVariables: false, defaultIntegrations: false, - integrations: [], + integrations: [Sentry.linkedErrorsIntegration()], }); applyGlobalTags(globalTags); attachMarkedLogReporting(logger); diff --git a/packages/trueforge/src/sentry/reportMarkedLog.ts b/packages/trueforge/src/sentry/reportMarkedLog.ts index aad5236729..c83c05a80f 100644 --- a/packages/trueforge/src/sentry/reportMarkedLog.ts +++ b/packages/trueforge/src/sentry/reportMarkedLog.ts @@ -46,13 +46,34 @@ function toError(value: unknown): Error | undefined { return new Error(value, { cause: value }); } +/** Log message is the Sentry title. The thrown error stays the cause and supplies the stack frames. */ +function errorWithLogTitle({ message, cause }: { message: string; cause: Error }): Error { + const titled = new Error(message, { cause }); + const causeStack = cause.stack; + if (causeStack === undefined) { + titled.stack = `${titled.name}: ${message}`; + return titled; + } + const frameStart = causeStack.indexOf('\n'); + const frames = frameStart === -1 ? '' : causeStack.slice(frameStart); + titled.stack = `${titled.name}: ${message}${frames}`; + return titled; +} + +function reportedError({ message, cause }: { message: unknown; cause: Error }): Error { + if (typeof message === 'string' && message.length > 0) { + return errorWithLogTitle({ message, cause }); + } + return cause; +} + export function reportMarkedLogFormat(): winston.Logform.Format { return winston.format(info => { const marked = readReportableError(info['reportable_error']); if (marked !== undefined) { - const error = toError(marked.error); - if (error !== undefined) { - captureCriticalException(error, { + const cause = toError(marked.error); + if (cause !== undefined) { + captureCriticalException(reportedError({ message: info.message, cause }), { ...(marked.filepath !== undefined ? { tags: { filepath: marked.filepath } } : {}), ...(marked.extra !== undefined ? { extra: marked.extra } : {}), }); diff --git a/packages/trueforge/src/truefoundry/initTrueFoundrySentry.ts b/packages/trueforge/src/truefoundry/initTrueFoundrySentry.ts index e160ac0c3c..cf33ee6629 100644 --- a/packages/trueforge/src/truefoundry/initTrueFoundrySentry.ts +++ b/packages/trueforge/src/truefoundry/initTrueFoundrySentry.ts @@ -75,7 +75,7 @@ export async function initTrueFoundrySentry(input: { Object.assign(sentryInitOptions, authData); sentryInitOptions.includeLocalVariables = false; sentryInitOptions.defaultIntegrations = false; - sentryInitOptions.integrations = []; + sentryInitOptions.integrations = [Sentry.linkedErrorsIntegration()]; Sentry.init(sentryInitOptions); Sentry.getGlobalScope().setTag('TRUEFORGE_VERSION', PACKAGE_VERSION); if (tags) { diff --git a/packages/trueforge/tests/unit/apis/turns.test.ts b/packages/trueforge/tests/unit/apis/turns.test.ts index 961cededf6..6bd6ba5ef3 100644 --- a/packages/trueforge/tests/unit/apis/turns.test.ts +++ b/packages/trueforge/tests/unit/apis/turns.test.ts @@ -1,14 +1,17 @@ import { OpenAPIHono } from '@hono/zod-openapi'; import { AgentSpecSchema, + CancellationReason, + EventType, Sessions, TurnNotFoundError, + type TerminalTurnState, type TurnInputItem, type TurnStreamingEvent, } from '@truefoundry/trueforge-core/agent-session'; import type { Kysely } from 'kysely'; import { createLogger } from 'winston'; -import { createTurnsRouter, deriveSessionTitle, turnStreamId } from '../../../src/apis/turns'; +import { createTurnsRouter, deriveSessionTitle, drainTurnEvents, turnStreamId } from '../../../src/apis/turns'; import { TrueForgeAuthorizer, type Authorizer } from '../../../src/auth/authorizer'; import { STANDALONE_REQUEST_CONTEXT } from '../../../src/auth/identity'; import { McpServerWithAuthStore } from '../../../src/db/McpServerWithAuthStore'; @@ -26,7 +29,7 @@ import type { Database } from '../../../src/db/sqlite/types'; import { SqliteWebSearchProviderStore } from '../../../src/db/sqlite/web-search-provider-store/SqliteWebSearchProviderStore'; import { runWithLogContext } from '../../../src/logger'; import { ActiveTurnRegistry } from '../../../src/runtime/activeTurns'; -import { EventSubscriptionRegistry } from '../../../src/runtime/event-subscription/index.js'; +import { EventSubscriptionRegistry, type EventSubscription } from '../../../src/runtime/event-subscription/index.js'; function mcpServerStoreWithAuth(db: Kysely, tokenStore: SqliteOAuthTokenStore) { return new McpServerWithAuthStore({ @@ -90,6 +93,100 @@ describe('turns', () => { }); }); + describe('drainTurnEvents', () => { + const completedAt = '2026-01-01T00:00:00.000Z'; + + function turnDone(state: TerminalTurnState): TurnStreamingEvent { + return { + type: EventType.TURN_DONE, + id: 'evt-done', + created_at: completedAt, + thread_id: null, + state, + }; + } + + function subscription(): EventSubscription { + return { + put: async () => 1, + assertSubscribable: async () => undefined, + async *poll() { + /* drain does not read the subscription */ + }, + }; + } + + async function drain(event: TurnStreamingEvent): Promise { + const logger = createLogger({ silent: true }); + const errorLog = jest.spyOn(logger, 'error'); + const timer = setTimeout(() => undefined, 60_000); + await drainTurnEvents({ + trackedStream: (async function* () { + yield event; + })(), + turnEventStream: subscription(), + maxExecutionTimer: timer, + logger, + }); + return errorLog; + } + + function reportedError(errorLog: jest.SpyInstance): { message: unknown; error: unknown; reason: unknown } { + const [message, fields] = errorLog.mock.calls[0] ?? []; + const marked: unknown = + typeof fields === 'object' && fields !== null ? Reflect.get(fields, 'reportable_error') : undefined; + const error: unknown = typeof marked === 'object' && marked !== null ? Reflect.get(marked, 'error') : undefined; + const extra: unknown = typeof marked === 'object' && marked !== null ? Reflect.get(marked, 'extra') : undefined; + const reason: unknown = typeof extra === 'object' && extra !== null ? Reflect.get(extra, 'reason') : undefined; + return { message, error, reason }; + } + + it('reports turn.done error, abandoned, and server-execution-timeout', async () => { + const errorLog = await drain(turnDone({ status: 'error', message: 'model failed', completed_at: completedAt })); + expect(reportedError(errorLog)).toMatchObject({ + message: 'turn.done error', + error: expect.objectContaining({ message: 'model failed' }), + }); + + const abandonedLog = await drain( + turnDone({ status: 'cancelled', reason: CancellationReason.Abandoned, completed_at: completedAt }), + ); + expect(reportedError(abandonedLog)).toMatchObject({ + message: 'turn.done cancelled', + error: expect.objectContaining({ message: CancellationReason.Abandoned }), + reason: CancellationReason.Abandoned, + }); + + const timeoutLog = await drain( + turnDone({ + status: 'cancelled', + reason: CancellationReason.ServerExecutionTimeout, + completed_at: completedAt, + }), + ); + expect(reportedError(timeoutLog)).toMatchObject({ + message: 'turn.done cancelled', + error: expect.objectContaining({ message: CancellationReason.ServerExecutionTimeout }), + reason: CancellationReason.ServerExecutionTimeout, + }); + }); + + it('does not report client-cancelled, cancelled-for-next-turn, or a finished turn', async () => { + for (const event of [ + turnDone({ status: 'cancelled', reason: CancellationReason.ClientCancelled, completed_at: completedAt }), + turnDone({ + status: 'cancelled', + reason: CancellationReason.CancelledForNextTurn, + completed_at: completedAt, + }), + turnDone({ status: 'done', output: null, required_actions: [], completed_at: completedAt }), + ]) { + const errorLog = await drain(event); + expect(errorLog).not.toHaveBeenCalled(); + } + }); + }); + describe('turn ownership', () => { it('returns 403 for all turn routes when the caller is not the session creator', async () => { const db = createSqliteDb(':memory:'); diff --git a/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts b/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts index 7ba0b66cb5..bdea411c2b 100644 --- a/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts +++ b/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts @@ -36,10 +36,21 @@ describe('reportMarkedLogFormat', () => { ...reportableErrorFields({ error: failure, extra: { path: '/api' } }), }); - expect(captureCriticalException).toHaveBeenCalledWith(failure, { + expect(captureCriticalException).toHaveBeenCalledWith(expect.any(Error), { tags: { filepath: expect.stringContaining('reportMarkedLog.test.ts') }, extra: { path: '/api' }, }); + const reported: unknown = jest.mocked(captureCriticalException).mock.calls[0]?.[0]; + expect(reported).toBeInstanceOf(Error); + if (!(reported instanceof Error)) { + return; + } + expect(reported.message).toBe('Unhandled error'); + expect(reported.cause).toBe(failure); + const reportedFrames = reported.stack?.slice(reported.stack.indexOf('\n') + 1); + const causeFrames = failure.stack?.slice((failure.stack?.indexOf('\n') ?? -1) + 1); + expect(reportedFrames).toBe(causeFrames); + expect(failure.message).toBe('boom'); expect(lines.join('')).not.toContain('reportable_error'); }); From f35cf619becc1d1df56c28461c22dba02d21b57a Mon Sep 17 00:00:00 2001 From: Raman Tehlan Date: Sat, 24 Oct 2026 16:13:54 +0530 Subject: [PATCH 07/13] fix: strip Sentry markers from logs and tag the file that logged The marker stayed on the printed line whenever Sentry did not start, and production tags collapsed to the server bundle or skipped core. Wrapped provider responses from 200 through 499 stay in the logs. Signed-off-by: Raman Tehlan --- .changeset/sentry-allowlisted-logs.md | 2 +- .../src/core/util/reportableErrorLog.ts | 205 ++++++++++++++++-- .../tests/core/llm/VercelAILLM.stream.test.ts | 44 ++++ packages/trueforge/src/logger.ts | 13 +- packages/trueforge/tests/unit/logger.test.ts | 32 ++- 5 files changed, 272 insertions(+), 24 deletions(-) diff --git a/.changeset/sentry-allowlisted-logs.md b/.changeset/sentry-allowlisted-logs.md index f77ddff47e..2001938ad8 100644 --- a/.changeset/sentry-allowlisted-logs.md +++ b/.changeset/sentry-allowlisted-logs.md @@ -3,4 +3,4 @@ '@truefoundry/trueforge-core': patch --- -Report logged failures to Sentry except HTTP statuses from 200 through 499. The log message is the Sentry title, and the thrown error is the cause and stack. Those events include the active tenant, request, session, and turn ids, and are tagged with the file that logged them. Existing direct Sentry captures are unchanged. +Report logged failures to Sentry except HTTP statuses from 200 through 499, including when that status sits on a wrapped cause. The log message is the Sentry title, and the thrown error is the cause and stack. Those events include the active tenant, request, session, and turn ids, and are tagged with the file that logged them. Printed lines omit that marker even when Sentry is off. diff --git a/packages/trueforge-core/src/core/util/reportableErrorLog.ts b/packages/trueforge-core/src/core/util/reportableErrorLog.ts index 1aa3fcec80..b24c3812f7 100644 --- a/packages/trueforge-core/src/core/util/reportableErrorLog.ts +++ b/packages/trueforge-core/src/core/util/reportableErrorLog.ts @@ -1,3 +1,8 @@ +import { readFileSync } from 'node:fs'; +import { SourceMap, type SourceMapPayload, type SourceOrigin } from 'node:module'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + export interface ReportableErrorLog { error: unknown; extra: Record | undefined; @@ -13,16 +18,164 @@ function isCallSite(value: unknown): value is NodeJS.CallSite { ); } -function packageRelativePath(fileName: string): string { - const marker = '/packages/'; - const index = fileName.lastIndexOf(marker); +function withSlashes(fileName: string): string { + return fileName.replaceAll('\\', '/'); +} + +/** Third-party frames. Published core and the server bundle stay, so the log site is not skipped. */ +function isThirdPartyFrame(fileName: string): boolean { + if (fileName.startsWith('node:')) { + return true; + } + const normalized = withSlashes(fileName); + const marker = '/node_modules/'; + const index = normalized.lastIndexOf(marker); if (index === -1) { + return false; + } + const rest = normalized.slice(index + marker.length); + return !rest.startsWith('@truefoundry/trueforge-core/') && !rest.startsWith('@truefoundry/trueforge/'); +} + +function filesystemPath(fileName: string): string { + if (fileName.startsWith('file:')) { + return fileURLToPath(fileName); + } + return fileName; +} + +function packageRelativePath(fileName: string): string { + const normalized = withSlashes(fileName); + const packagesMarker = '/packages/'; + const packagesIndex = normalized.lastIndexOf(packagesMarker); + if (packagesIndex !== -1) { + return normalized.slice(packagesIndex + packagesMarker.length); + } + const scopedMarker = '/@truefoundry/'; + const scopedIndex = normalized.lastIndexOf(scopedMarker); + if (scopedIndex !== -1) { + return normalized.slice(scopedIndex + scopedMarker.length); + } + return normalized; +} + +function isBundledEntry(fileName: string): boolean { + return /\/dist\/(?:main|controller-main|cli)\.js$/.test(withSlashes(fileName)); +} + +function readStringArray(value: unknown): string[] | undefined { + if (!Array.isArray(value)) { + return undefined; + } + const items: string[] = []; + for (const item of value) { + if (typeof item !== 'string') { + return undefined; + } + items.push(item); + } + return items; +} + +function readSourcesContent(value: unknown): string[] { + if (!Array.isArray(value)) { + return []; + } + const items: string[] = []; + for (const item of value) { + if (typeof item === 'string') { + items.push(item); + continue; + } + if (item === null) { + items.push(''); + continue; + } + return []; + } + return items; +} + +function readSourceMapPayload(value: unknown): SourceMapPayload | undefined { + if (typeof value !== 'object' || value === null) { + return undefined; + } + const version: unknown = Reflect.get(value, 'version'); + const sources = readStringArray(Reflect.get(value, 'sources')); + const mappings: unknown = Reflect.get(value, 'mappings'); + const names = readStringArray(Reflect.get(value, 'names')); + if (typeof version !== 'number' || sources === undefined || typeof mappings !== 'string' || names === undefined) { + return undefined; + } + const file: unknown = Reflect.get(value, 'file'); + const sourceRoot: unknown = Reflect.get(value, 'sourceRoot'); + const sourcesContent: unknown = Reflect.get(value, 'sourcesContent'); + return { + file: typeof file === 'string' ? file : 'generated.js', + version, + sources, + sourcesContent: readSourcesContent(sourcesContent), + names, + mappings, + sourceRoot: typeof sourceRoot === 'string' ? sourceRoot : '', + }; +} + +const sourceMaps = new Map(); + +function sourceMapFor(generatedPath: string): SourceMap | undefined { + if (sourceMaps.has(generatedPath)) { + return sourceMaps.get(generatedPath); + } + let map: SourceMap | undefined; + try { + const raw: unknown = JSON.parse(readFileSync(`${generatedPath}.map`, 'utf8')); + const payload = readSourceMapPayload(raw); + map = payload === undefined ? undefined : new SourceMap(payload); + } catch { + map = undefined; + } + sourceMaps.set(generatedPath, map); + return map; +} + +function originalFileName(origin: SourceOrigin | Record): string | undefined { + if (!('fileName' in origin)) { + return undefined; + } + const fileName: unknown = origin.fileName; + return typeof fileName === 'string' && fileName.length > 0 ? fileName : undefined; +} + +/** Bundle frames are all `dist/main.js`. The sibling map names the file that logged. */ +function originalSourceFile({ + generated, + line, + column, +}: { + generated: string; + line: number; + column: number; +}): string | undefined { + const generatedPath = filesystemPath(generated); + const map = sourceMapFor(generatedPath); + if (map === undefined) { + return undefined; + } + const fileName = originalFileName(map.findOrigin(line, column)); + if (fileName === undefined) { + return undefined; + } + if (fileName.startsWith('file:')) { + return filesystemPath(fileName); + } + if (path.isAbsolute(fileName)) { return fileName; } - return fileName.slice(index + marker.length); + return path.resolve(path.dirname(generatedPath), fileName); } -function callerFilePath(): string | undefined { +function captureCallSites(): NodeJS.CallSite[] | undefined { const previous: unknown = Reflect.get(Error, 'prepareStackTrace'); Reflect.set(Error, 'prepareStackTrace', (_error: Error, stack: NodeJS.CallSite[]) => stack); try { @@ -31,22 +184,7 @@ function callerFilePath(): string | undefined { if (!Array.isArray(stack)) { return undefined; } - for (const site of stack) { - if (!isCallSite(site)) { - continue; - } - const fileName = site.getFileName(); - if ( - fileName === null || - fileName.startsWith('node:') || - fileName.includes('node_modules') || - fileName.includes('reportableErrorLog') - ) { - continue; - } - return packageRelativePath(fileName); - } - return undefined; + return stack.filter(isCallSite); } finally { if (typeof previous === 'function') { Reflect.set(Error, 'prepareStackTrace', previous); @@ -56,6 +194,31 @@ function callerFilePath(): string | undefined { } } +function callerFilePath(): string | undefined { + const sites = captureCallSites(); + if (sites === undefined) { + return undefined; + } + for (const site of sites) { + const generated = site.getFileName(); + if (generated === null || isThirdPartyFrame(generated)) { + continue; + } + const line = site.getLineNumber(); + const column = site.getColumnNumber(); + const original = line !== null && column !== null ? originalSourceFile({ generated, line, column }) : undefined; + const fileName = original ?? filesystemPath(generated); + if (fileName.includes('reportableErrorLog') || isThirdPartyFrame(fileName)) { + continue; + } + if (original === undefined && isBundledEntry(fileName)) { + return line === null ? packageRelativePath(fileName) : `${packageRelativePath(fileName)}:${String(line)}`; + } + return packageRelativePath(fileName); + } + return undefined; +} + /** Metadata the Sentry log format uses to report a Winston line. */ export function reportableErrorFields(input: { error: unknown; extra?: Record }): { reportable_error: ReportableErrorLog; diff --git a/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts b/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts index b196c98935..d2d5e5cbc1 100644 --- a/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts +++ b/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts @@ -729,4 +729,48 @@ describe('isReportableError', () => { ).toBe(false); } }); + + it('skips a stream error part that wraps a provider 4xx with a socket cause', async () => { + const cause = new APICallError({ + message: 'rate limit', + url: 'https://example.com', + requestBodyValues: {}, + statusCode: 429, + cause: Object.assign(new Error('socket hang up'), { code: 'ECONNRESET' }), + }); + let thrown: unknown; + try { + await drainStream( + mapStreamToChunks({ + stream: makeStream([{ type: 'error', error: cause }]), + chunkMeta: CHUNK_META, + }), + ); + } catch (error) { + thrown = error; + } + expect(isReportableError(thrown)).toBe(false); + }); + + it('reports a stream error part that wraps a provider 5xx', async () => { + const cause = new APICallError({ + message: 'provider unavailable', + url: 'https://example.com', + requestBodyValues: {}, + statusCode: 503, + cause: Object.assign(new Error('socket hang up'), { code: 'ECONNRESET' }), + }); + let thrown: unknown; + try { + await drainStream( + mapStreamToChunks({ + stream: makeStream([{ type: 'error', error: cause }]), + chunkMeta: CHUNK_META, + }), + ); + } catch (error) { + thrown = error; + } + expect(isReportableError(thrown)).toBe(true); + }); }); diff --git a/packages/trueforge/src/logger.ts b/packages/trueforge/src/logger.ts index c2350b39de..6a1ddcb98a 100644 --- a/packages/trueforge/src/logger.ts +++ b/packages/trueforge/src/logger.ts @@ -131,8 +131,19 @@ function jsonFormat(): winston.Logform.Format { ); } +/** Logging metadata. Removed before print so a line never shows it when Sentry does not start. */ +function omitReportableErrorFormat(): winston.Logform.Format { + return winston.format(info => { + Reflect.deleteProperty(info, 'reportable_error'); + return info; + })(); +} + function serverLogFormat(options: { standalone: boolean }): winston.Logform.Format { - return options.standalone ? standaloneFormat(shouldColorize()) : jsonFormat(); + return winston.format.combine( + omitReportableErrorFormat(), + options.standalone ? standaloneFormat(shouldColorize()) : jsonFormat(), + ); } function createLogger(options: { level: string; standalone: boolean; component: 'server' | 'controller' }): Logger { diff --git a/packages/trueforge/tests/unit/logger.test.ts b/packages/trueforge/tests/unit/logger.test.ts index ffb93efca8..f29e2713a6 100644 --- a/packages/trueforge/tests/unit/logger.test.ts +++ b/packages/trueforge/tests/unit/logger.test.ts @@ -1,4 +1,6 @@ -import { currentLogContext, runWithLogContext } from '../../src/logger'; +import { Writable } from 'node:stream'; +import winston from 'winston'; +import { createServerLogger, currentLogContext, runWithLogContext } from '../../src/logger'; describe('log context AsyncLocalStorage', () => { it('is empty when an AbortSignal listener runs after the request store exits', async () => { @@ -34,3 +36,31 @@ describe('log context AsyncLocalStorage', () => { }); }); }); + +describe('printed logs', () => { + function captureLines(standalone: boolean): { logger: winston.Logger; lines: string[] } { + const lines: string[] = []; + const stream = new Writable({ + write(chunk: Buffer | string, _encoding: BufferEncoding, callback: (error?: Error | null) => void) { + lines.push(chunk.toString()); + callback(); + }, + }); + const logger = createServerLogger({ level: 'info', standalone }); + logger.clear(); + logger.add(new winston.transports.Stream({ stream })); + return { logger, lines }; + } + + it('omits reportable_error when Sentry has not started', () => { + for (const standalone of [false, true]) { + const { logger, lines } = captureLines(standalone); + logger.error('Unhandled error', { + error: 'boom', + reportable_error: { error: new Error('boom'), extra: undefined, filepath: 'trueforge/src/app.ts' }, + }); + expect(lines.join('')).not.toContain('reportable_error'); + expect(lines.join('')).toContain('boom'); + } + }); +}); From 3d1ee60c344e48cb3ea6b3162adc1f348462c189 Mon Sep 17 00:00:00 2001 From: Raman Tehlan Date: Sat, 24 Oct 2026 16:29:21 +0530 Subject: [PATCH 08/13] fix: keep expected failures and duplicate reports out of Sentry Dual-write cleanup keeps the agent identity on the event. A turn.done error only has a message, so provider failures stay classified where they are thrown. Missing sandbox versions, rejected credentials, and API responses the server already logged stay in the logs. Signed-off-by: Raman Tehlan Co-authored-by: Cursor --- .../src/core/runtime/AgentThread.ts | 11 ++- .../agentThreadExecutionFailure.test.ts | 70 +++++++++++++++++++ packages/trueforge/src/apis/turns.ts | 7 +- .../src/controller/sandboxEnvBuild.ts | 18 ++--- .../progressSandboxEnvironmentVersion.ts | 14 +--- .../src/truefoundry/TrueFoundryAgentStore.ts | 20 +++++- .../trueforge/tests/unit/apis/turns.test.ts | 10 +-- .../unit/controller/sandboxEnvBuild.test.ts | 44 +++++++++--- .../progressSandboxEnvironmentVersion.test.ts | 22 +++--- .../truefoundry/TrueFoundryAgentStore.test.ts | 32 +++++++++ 10 files changed, 190 insertions(+), 58 deletions(-) create mode 100644 packages/trueforge-core/tests/core/runtime/agentThreadExecutionFailure.test.ts diff --git a/packages/trueforge-core/src/core/runtime/AgentThread.ts b/packages/trueforge-core/src/core/runtime/AgentThread.ts index a8338ae850..e4143e587e 100644 --- a/packages/trueforge-core/src/core/runtime/AgentThread.ts +++ b/packages/trueforge-core/src/core/runtime/AgentThread.ts @@ -12,7 +12,7 @@ import type { } from '../capabilities/AgentContextProcessor'; import { SUB_AGENT_IDENTITY } from '../capabilities/builtins/DynamicSubAgents'; import type { ToolResponseProcessor } from '../capabilities/ToolResponseProcessor'; -import { AgentHarnessError, InvalidAgentSendInputError } from '../errors'; +import { AgentHarnessError, InvalidAgentSendInputError, ModelProviderError } from '../errors'; import type { RegisteredPassthroughEvent } from '../events/PassthroughEvents'; import { EventType, @@ -55,7 +55,8 @@ import { executeToolCalls } from '../mcp/executeToolCalls'; import type { IToolSet, MCPAuthRequired } from '../mcp/IMCPServer'; import type { HarnessSandbox, SandboxInfo } from '../sandbox/Sandbox'; import type { AgentTracing } from '../tracing/AgentTracing'; -import { describeUnknownError, extractErrorLogFields } from '../util/errorLogFields'; +import { describeUnknownError, extractErrorLogFields, isReportableError } from '../util/errorLogFields'; +import { reportableErrorFields } from '../util/reportableErrorLog'; import type { AgentDefinition } from './AgentDefinition'; import type { AgentThreadConstructorInput, @@ -1516,7 +1517,11 @@ export class AgentThread { } catch (error) { // Providers / transports often reject with plain objects; instanceof Error would // otherwise collapse those into an opaque "Unknown error occurred" in Agent Steps. - this.logger.error('Agent thread execution failed', extractErrorLogFields(error)); + // Model-provider failures are classified at the LLM call. turn.done only keeps the message. + this.logger.error('Agent thread execution failed', { + ...extractErrorLogFields(error), + ...(error instanceof ModelProviderError || !isReportableError(error) ? {} : reportableErrorFields({ error })), + }); yield this.generateErrorEvent(describeUnknownError(error)); } finally { this.contextBusy = false; diff --git a/packages/trueforge-core/tests/core/runtime/agentThreadExecutionFailure.test.ts b/packages/trueforge-core/tests/core/runtime/agentThreadExecutionFailure.test.ts new file mode 100644 index 0000000000..82da288d03 --- /dev/null +++ b/packages/trueforge-core/tests/core/runtime/agentThreadExecutionFailure.test.ts @@ -0,0 +1,70 @@ +import { ModelProviderError } from '../../../src/core/errors'; +import type { ILLM } from '../../../src/core/llm/ILLM'; +import { AgentThread } from '../../../src/core/runtime/AgentThread'; +import { NOOP_AGENT_TRACING } from '../../../src/core/tracing/NoopAgentTracing'; +import '../harnessMocks'; +import { makeSilentLogger, TEST_MODEL_NAME } from '../harnessMocks'; + +class RateLimited extends Error { + readonly statusCode = 429; +} + +function failingClient(error: unknown): ILLM { + return { + create: () => { + throw error; + }, + createNonStream: jest.fn(), + }; +} + +async function run(error: unknown): Promise { + const logger = makeSilentLogger(); + const errorLog = jest.spyOn(logger, 'error'); + const thread = new AgentThread({ + threadId: 'main', + title: 'Main', + tracing: NOOP_AGENT_TRACING, + logger, + definition: { + modelClient: failingClient(error), + modelName: TEST_MODEL_NAME, + instruction: 'test', + toolSets: [], + }, + context: [{ role: 'user', content: 'hi' }], + }); + for await (const event of thread.execute({ signal: new AbortController().signal })) { + void event; + } + return errorLog; +} + +describe('AgentThread execution failure', () => { + it('reports a failure that is not a model-provider response', async () => { + const failure = new Error('disk full'); + const errorLog = await run(failure); + expect(errorLog).toHaveBeenCalledWith( + 'Agent thread execution failed', + expect.objectContaining({ + reportable_error: expect.objectContaining({ error: failure }), + }), + ); + }); + + it('does not report a model-provider failure or an HTTP client error', async () => { + const provider = new ModelProviderError('Request failed (401): bad credentials'); + const providerLog = await run(provider); + expect(providerLog).toHaveBeenCalledWith( + 'Agent thread execution failed', + expect.not.objectContaining({ reportable_error: expect.anything() }), + ); + + const limited = new RateLimited('slow down'); + const limitedLog = await run(limited); + expect(limitedLog).toHaveBeenCalledWith( + 'Agent thread execution failed', + expect.not.objectContaining({ reportable_error: expect.anything() }), + ); + }); +}); diff --git a/packages/trueforge/src/apis/turns.ts b/packages/trueforge/src/apis/turns.ts index 982380c7ec..629d3a038e 100644 --- a/packages/trueforge/src/apis/turns.ts +++ b/packages/trueforge/src/apis/turns.ts @@ -385,11 +385,8 @@ function reportUnexpectedTurnDone({ event, logger }: { event: TurnStreamingEvent } const { state } = event; if (state.status === 'error') { - const error = new Error(state.message); - logger.error('turn.done error', { - ...extractErrorLogFields(error), - ...reportableErrorFields({ error }), - }); + // Only the message remains here. The thrown error was already classified, including provider 4xx. + logger.error('turn.done error', extractErrorLogFields(new Error(state.message))); return; } if ( diff --git a/packages/trueforge/src/controller/sandboxEnvBuild.ts b/packages/trueforge/src/controller/sandboxEnvBuild.ts index 4d8b9b4a60..a7fecd63d4 100644 --- a/packages/trueforge/src/controller/sandboxEnvBuild.ts +++ b/packages/trueforge/src/controller/sandboxEnvBuild.ts @@ -2,7 +2,7 @@ * Control loop: list pending sandbox-env versions and hand each to the server over HTTP. */ import { reportableErrorFields } from '@truefoundry/trueforge-core/core'; -import { TrueForgeApi } from '@truefoundry/trueforge-sdk'; +import { TrueForgeApi, TrueForgeError } from '@truefoundry/trueforge-sdk'; import type { Logger } from 'winston'; import type { ControlLoop } from './Controller'; import { internalTrueForgeClient } from './internalTrueForgeClient'; @@ -24,20 +24,20 @@ export async function dispatchSandboxEnvBuilds({ logger }: { logger: Logger }): if (error instanceof TrueForgeApi.NotFoundError) { logger.warn('Sandbox environment version not found; skipping', { environment_version_id: environmentVersionId, - ...reportableErrorFields({ - error, - extra: { environment_version_id: environmentVersionId }, - }), }); continue; } + // A status means the API handler already logged the failure. Report a call that got no response. + const serverResponded = error instanceof TrueForgeError && typeof error.statusCode === 'number'; logger.error('Sandbox environment version progress failed', { environment_version_id: environmentVersionId, error: error instanceof Error ? error.message : String(error), - ...reportableErrorFields({ - error, - extra: { environment_version_id: environmentVersionId }, - }), + ...(serverResponded + ? {} + : reportableErrorFields({ + error, + extra: { environment_version_id: environmentVersionId }, + })), }); } } diff --git a/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts b/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts index 14f929f620..0e345a33e8 100644 --- a/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts +++ b/packages/trueforge/src/sandbox/progressSandboxEnvironmentVersion.ts @@ -1,9 +1,5 @@ /** Progress pending env version: snapshot build only (secrets sync on PUT). */ -import { - DAYTONA_SNAPSHOT_NOT_STARTED_REASON, - reportableErrorFields, - withTimeout, -} from '@truefoundry/trueforge-core/core'; +import { DAYTONA_SNAPSHOT_NOT_STARTED_REASON, withTimeout } from '@truefoundry/trueforge-core/core'; import { HTTPException } from 'hono/http-exception'; import type { Logger } from 'winston'; import type { ISandboxEnvironmentStore } from '../db/sandboxEnvironmentStore'; @@ -32,10 +28,6 @@ export async function progressSandboxEnvironmentVersion({ const error = new HTTPException(404, { message: 'Sandbox environment version not found' }); logger.warn('Sandbox environment version not found', { environment_version_id, - ...reportableErrorFields({ - error, - extra: { environment_version_id }, - }), }); throw error; } @@ -82,10 +74,6 @@ export async function progressSandboxEnvironmentVersion({ logger.warn('Sandbox environment build failed authz', { environment_version_id, status_reason, - ...reportableErrorFields({ - error, - extra: { environment_version_id, tenant_id: pending.tenant_id, status_reason }, - }), }); return; } diff --git a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts index 6e56308daa..1ce29faa95 100644 --- a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts +++ b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts @@ -1,5 +1,5 @@ import type { AgentSpec, TokenPagination } from '@truefoundry/trueforge-core/agent-session'; -import { extractErrorLogFields } from '@truefoundry/trueforge-core/core'; +import { extractErrorLogFields, reportableErrorFields } from '@truefoundry/trueforge-core/core'; import { sql, type Kysely, type Transaction } from 'kysely'; import type { Logger } from 'winston'; import type { RequestContext } from '../auth/identity'; @@ -200,6 +200,15 @@ export class TrueFoundryAgentStore implements IAgentStore> agent_name: created.name, tenant_id: input.tenant_id, external_id: externalId, + ...reportableErrorFields({ + error: aggregate, + extra: { + agent_id: created.id, + agent_name: created.name, + tenant_id: input.tenant_id, + external_id: externalId, + }, + }), }); throw aggregate; } @@ -264,6 +273,15 @@ export class TrueFoundryAgentStore implements IAgentStore> agent_name: previous.name, tenant_id: input.tenant_id, external_id: previous.external_id, + ...reportableErrorFields({ + error: aggregate, + extra: { + agent_id: input.id, + agent_name: previous.name, + tenant_id: input.tenant_id, + external_id: previous.external_id, + }, + }), }); throw aggregate; } diff --git a/packages/trueforge/tests/unit/apis/turns.test.ts b/packages/trueforge/tests/unit/apis/turns.test.ts index 6bd6ba5ef3..9c48ffa353 100644 --- a/packages/trueforge/tests/unit/apis/turns.test.ts +++ b/packages/trueforge/tests/unit/apis/turns.test.ts @@ -141,12 +141,12 @@ describe('turns', () => { return { message, error, reason }; } - it('reports turn.done error, abandoned, and server-execution-timeout', async () => { + it('logs turn.done error without reporting it, and reports abandoned and server-execution-timeout', async () => { const errorLog = await drain(turnDone({ status: 'error', message: 'model failed', completed_at: completedAt })); - expect(reportedError(errorLog)).toMatchObject({ - message: 'turn.done error', - error: expect.objectContaining({ message: 'model failed' }), - }); + expect(errorLog).toHaveBeenCalledWith( + 'turn.done error', + expect.not.objectContaining({ reportable_error: expect.anything() }), + ); const abandonedLog = await drain( turnDone({ status: 'cancelled', reason: CancellationReason.Abandoned, completed_at: completedAt }), diff --git a/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts b/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts index 1432d7c6b4..09795abb79 100644 --- a/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts +++ b/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts @@ -1,4 +1,4 @@ -import { TrueForgeApi } from '@truefoundry/trueforge-sdk'; +import { TrueForgeApi, TrueForgeError } from '@truefoundry/trueforge-sdk'; import { createLogger } from 'winston'; import { internalTrueForgeClient } from '../../../src/controller/internalTrueForgeClient'; import { dispatchSandboxEnvBuilds, sandboxEnvBuildLoop } from '../../../src/controller/sandboxEnvBuild'; @@ -41,13 +41,9 @@ describe('sandboxEnvBuildLoop', () => { await dispatchSandboxEnvBuilds({ logger: logger as never }); expect(progress).toHaveBeenCalledTimes(2); - expect(logger.warn).toHaveBeenCalledWith( - 'Sandbox environment version not found; skipping', - expect.objectContaining({ - environment_version_id: 'gone', - reportable_error: expect.objectContaining({ error: expect.any(TrueForgeApi.NotFoundError) }), - }), - ); + expect(logger.warn).toHaveBeenCalledWith('Sandbox environment version not found; skipping', { + environment_version_id: 'gone', + }); }); it('reports a progress failure once', async () => { @@ -66,6 +62,38 @@ describe('sandboxEnvBuildLoop', () => { ); }); + it('does not report a progress failure the API already answered', async () => { + jest.spyOn(internalTrueForgeClient, 'listPendingSandboxEnvironmentVersions').mockResolvedValue(['ver-1']); + jest + .spyOn(internalTrueForgeClient, 'progressSandboxEnvironmentVersion') + .mockRejectedValue(new TrueForgeApi.InternalServerError({ error: { message: 'down' } })); + const logger = fakeLogger(); + + await dispatchSandboxEnvBuilds({ logger: logger as never }); + + expect(logger.error).toHaveBeenCalledWith( + 'Sandbox environment version progress failed', + expect.not.objectContaining({ reportable_error: expect.anything() }), + ); + }); + + it('reports a progress failure that never received a response', async () => { + jest.spyOn(internalTrueForgeClient, 'listPendingSandboxEnvironmentVersions').mockResolvedValue(['ver-1']); + jest + .spyOn(internalTrueForgeClient, 'progressSandboxEnvironmentVersion') + .mockRejectedValue(new TrueForgeError({ message: 'socket hang up' })); + const logger = fakeLogger(); + + await dispatchSandboxEnvBuilds({ logger: logger as never }); + + expect(logger.error).toHaveBeenCalledWith( + 'Sandbox environment version progress failed', + expect.objectContaining({ + reportable_error: expect.objectContaining({ error: expect.any(TrueForgeError) }), + }), + ); + }); + it('does not tick when aborted', async () => { const list = jest.spyOn(internalTrueForgeClient, 'listPendingSandboxEnvironmentVersions'); const controller = new AbortController(); diff --git a/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts b/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts index d607903e2b..4fd43932b2 100644 --- a/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts +++ b/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts @@ -85,13 +85,9 @@ describe('progressSandboxEnvironmentVersion', () => { logger, }), ).rejects.toMatchObject({ status: 404 } satisfies Partial); - expect(warn).toHaveBeenCalledWith( - 'Sandbox environment version not found', - expect.objectContaining({ - environment_version_id: 'missing', - reportable_error: expect.objectContaining({ error: expect.any(HTTPException) }), - }), - ); + expect(warn).toHaveBeenCalledWith('Sandbox environment version not found', { + environment_version_id: 'missing', + }); }); it('rethrows a build failure without reporting it from this layer', async () => { @@ -113,7 +109,7 @@ describe('progressSandboxEnvironmentVersion', () => { expect(errorLog).not.toHaveBeenCalled(); }); - it('reports an authz failure and marks the version failed', async () => { + it('logs an authz failure without reporting it and marks the version failed', async () => { const { sandboxEnvironmentStore, sandboxProviderStore, envStore } = makeStores(); const authError = new DaytonaError('Unauthorized', 401); const warn = jest.spyOn(logger, 'warn'); @@ -133,12 +129,10 @@ describe('progressSandboxEnvironmentVersion', () => { environment_version_id: 'ver-1', status_reason: 'Sandbox provider rejected the API key — check the credentials', }); - expect(warn).toHaveBeenCalledWith( - 'Sandbox environment build failed authz', - expect.objectContaining({ - reportable_error: expect.objectContaining({ error: authError }), - }), - ); + expect(warn).toHaveBeenCalledWith('Sandbox environment build failed authz', { + environment_version_id: 'ver-1', + status_reason: 'Sandbox provider rejected the API key — check the credentials', + }); }); it('marks ready when getBuildStatus reports ready', async () => { diff --git a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts index 7c39dcc7eb..e7b2433033 100644 --- a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts +++ b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts @@ -369,6 +369,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ createAgent, updateAgent, deleteAgent }), client: mockClient({ deleteRemoteAgent }), }); + const errorLog = jest.spyOn(LOGGER, 'error'); await expect( store.createAgent( @@ -390,6 +391,21 @@ describe('TrueFoundryAgentStore', () => { expect.objectContaining({ message: 'local cleanup failed' }), ], }); + expect(errorLog).toHaveBeenCalledWith( + 'createAgent failed and cleanup also failed', + expect.objectContaining({ + agent_id: local.id, + agent_name: local.name, + external_id: 'sf-1', + reportable_error: expect.objectContaining({ + extra: expect.objectContaining({ + agent_id: local.id, + agent_name: local.name, + external_id: 'sf-1', + }), + }), + }), + ); }); it('updateAgent without manifest passes through to the inner store', async () => { @@ -586,6 +602,7 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, updateAgent }), client: mockClient({ putRemoteAgent }), }); + const errorLog = jest.spyOn(LOGGER, 'error'); await expect( store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: manifest({ instructions: 'Updated.' }) }), @@ -596,6 +613,21 @@ describe('TrueFoundryAgentStore', () => { expect.objectContaining({ message: 'sf restore failed' }), ], }); + expect(errorLog).toHaveBeenCalledWith( + 'updateAgent failed and ServiceFoundry restore also failed', + expect.objectContaining({ + agent_id: previous.id, + agent_name: previous.name, + external_id: previous.external_id, + reportable_error: expect.objectContaining({ + extra: expect.objectContaining({ + agent_id: previous.id, + agent_name: previous.name, + external_id: previous.external_id, + }), + }), + }), + ); }); it('deleteAgent reads the agent inside the transaction', async () => { From 17db699c4dd90e1a0f447fe34e845d2cee8b8a41 Mon Sep 17 00:00:00 2001 From: Raman Tehlan Date: Sat, 24 Oct 2026 17:49:02 +0530 Subject: [PATCH 09/13] fix: report a cleanup failure once The agent store log already carries the agent identity. The HTTP 500 handler skips an error that was already marked, so the same cleanup failure is not sent again. Signed-off-by: Raman Tehlan Co-authored-by: Cursor --- packages/trueforge-core/src/core/index.ts | 2 +- .../src/core/util/reportableErrorLog.ts | 10 ++++++++++ packages/trueforge/src/app.ts | 4 ++-- .../trueforge/tests/unit/appErrorHandler.test.ts | 16 ++++++++++++++++ 4 files changed, 29 insertions(+), 3 deletions(-) diff --git a/packages/trueforge-core/src/core/index.ts b/packages/trueforge-core/src/core/index.ts index a31d0119c4..609e9b05ce 100644 --- a/packages/trueforge-core/src/core/index.ts +++ b/packages/trueforge-core/src/core/index.ts @@ -181,7 +181,7 @@ export { ssrfFetch, } from './util/outboundFetch'; export { PromiseTimeoutError, withTimeout } from './util/promiseUtils'; -export { reportableErrorFields, type ReportableErrorLog } from './util/reportableErrorLog'; +export { errorAlreadyMarked, reportableErrorFields, type ReportableErrorLog } from './util/reportableErrorLog'; export { assertSafeOutboundUrl, configureOutboundUrlGuard, diff --git a/packages/trueforge-core/src/core/util/reportableErrorLog.ts b/packages/trueforge-core/src/core/util/reportableErrorLog.ts index b24c3812f7..56a2d55414 100644 --- a/packages/trueforge-core/src/core/util/reportableErrorLog.ts +++ b/packages/trueforge-core/src/core/util/reportableErrorLog.ts @@ -219,10 +219,20 @@ function callerFilePath(): string | undefined { return undefined; } +const markedErrors = new WeakSet(); + +/** True when this error object was already attached to a Sentry log line. */ +export function errorAlreadyMarked(error: unknown): boolean { + return typeof error === 'object' && error !== null && markedErrors.has(error); +} + /** Metadata the Sentry log format uses to report a Winston line. */ export function reportableErrorFields(input: { error: unknown; extra?: Record }): { reportable_error: ReportableErrorLog; } { + if (typeof input.error === 'object' && input.error !== null) { + markedErrors.add(input.error); + } return { reportable_error: { error: input.error, diff --git a/packages/trueforge/src/app.ts b/packages/trueforge/src/app.ts index cf0ad99aea..2e08efbc68 100644 --- a/packages/trueforge/src/app.ts +++ b/packages/trueforge/src/app.ts @@ -2,7 +2,7 @@ import { swaggerUI } from '@hono/swagger-ui'; import { OpenAPIHono, z } from '@hono/zod-openapi'; import type { ISessionStore, Sessions, TurnStreamingEvent } from '@truefoundry/trueforge-core/agent-session'; -import { extractErrorLogFields, reportableErrorFields } from '@truefoundry/trueforge-core/core'; +import { errorAlreadyMarked, extractErrorLogFields, reportableErrorFields } from '@truefoundry/trueforge-core/core'; import type { RedisClient, RequestReplyRouter } from '@truefoundry/trueforge-core/request-reply'; import type { Context, ErrorHandler, MiddlewareHandler } from 'hono'; import { bodyLimit } from 'hono/body-limit'; @@ -159,7 +159,7 @@ export function createAppErrorHandler(params: { logger: Logger }): ErrorHandler path: c.req.path, status, ...extractErrorLogFields(error), - ...(status >= 500 ? reportableErrorFields({ error }) : {}), + ...(status >= 500 && !errorAlreadyMarked(error) ? reportableErrorFields({ error }) : {}), }; if (status >= 500) { params.logger.error(message, fields); diff --git a/packages/trueforge/tests/unit/appErrorHandler.test.ts b/packages/trueforge/tests/unit/appErrorHandler.test.ts index 9e3659d92b..73728bf416 100644 --- a/packages/trueforge/tests/unit/appErrorHandler.test.ts +++ b/packages/trueforge/tests/unit/appErrorHandler.test.ts @@ -1,4 +1,5 @@ import { OpenAPIHono, createRoute, z } from '@hono/zod-openapi'; +import { reportableErrorFields } from '@truefoundry/trueforge-core/core'; import { HTTPException } from 'hono/http-exception'; import winston from 'winston'; import { createAppErrorHandler } from '../../src/app'; @@ -71,6 +72,21 @@ describe('createAppErrorHandler', () => { }); }); + it('does not mark a 500 whose error was already marked', async () => { + const logger = winston.createLogger({ silent: true }); + const errorLog = jest.spyOn(logger, 'error'); + const thrown = new AggregateError([new Error('db update failed')], 'createAgent failed and cleanup also failed'); + reportableErrorFields({ error: thrown, extra: { agent_id: 'agent-1' } }); + + const response = await appThrowing(logger, thrown).request('/test'); + + expect(response.status).toBe(500); + expect(errorLog).toHaveBeenCalledWith( + 'Unhandled error', + expect.not.objectContaining({ reportable_error: expect.anything() }), + ); + }); + it('marks a 500 that wraps an earlier error', async () => { const logger = winston.createLogger({ silent: true }); const errorLog = jest.spyOn(logger, 'error'); From 252d77973c471c42fc2717b228432dbf16d87b69 Mon Sep 17 00:00:00 2001 From: Raman Tehlan Date: Sat, 24 Oct 2026 20:43:20 +0530 Subject: [PATCH 10/13] fix: remove the wrapper function for reportable_error --- .changeset/sentry-allowlisted-logs.md | 2 +- .../src/agent-session/SessionHandle.ts | 3 +- .../src/agent-session/TurnHandle.ts | 3 +- .../builtins/LargeToolResponse.ts | 3 +- packages/trueforge-core/src/core/index.ts | 2 +- .../src/core/llm/VercelAILLM.ts | 5 +- .../src/core/runtime/AgentThread.ts | 3 +- .../src/core/sandbox/Sandbox.ts | 5 +- .../sandbox/codeMode/CodeModeDispatcher.ts | 3 +- .../codeMode/nats/CodeModeNatsTransport.ts | 13 +- .../core/sandbox/provider/DaytonaProvider.ts | 13 +- .../sandbox/provider/TFYSandboxProvider.ts | 15 +- .../src/core/util/promiseUtils.ts | 3 +- .../src/core/util/reportableErrorLog.ts | 242 +----------------- .../src/request-reply/executor.ts | 25 +- packages/trueforge/src/apis/auth.ts | 8 +- packages/trueforge/src/apis/mcpOAuth.ts | 4 +- packages/trueforge/src/apis/mcpServers.ts | 3 +- packages/trueforge/src/apis/sessions.ts | 6 +- packages/trueforge/src/apis/turns.ts | 9 +- packages/trueforge/src/app.ts | 4 +- packages/trueforge/src/controller.ts | 5 +- .../trueforge/src/controller/Controller.ts | 3 +- .../src/controller/sandboxEnvBuild.ts | 11 +- .../src/controller/scheduleDispatch.ts | 9 +- packages/trueforge/src/main.ts | 7 +- packages/trueforge/src/runtime/redis.ts | 8 +- .../src/truefoundry/TrueFoundryAgentStore.ts | 10 +- .../TrueFoundryServiceFoundryServerClient.ts | 18 +- .../tests/unit/appErrorHandler.test.ts | 19 -- .../tests/unit/sentry/reportMarkedLog.test.ts | 8 +- 31 files changed, 99 insertions(+), 373 deletions(-) diff --git a/.changeset/sentry-allowlisted-logs.md b/.changeset/sentry-allowlisted-logs.md index 2001938ad8..5d00375096 100644 --- a/.changeset/sentry-allowlisted-logs.md +++ b/.changeset/sentry-allowlisted-logs.md @@ -3,4 +3,4 @@ '@truefoundry/trueforge-core': patch --- -Report logged failures to Sentry except HTTP statuses from 200 through 499, including when that status sits on a wrapped cause. The log message is the Sentry title, and the thrown error is the cause and stack. Those events include the active tenant, request, session, and turn ids, and are tagged with the file that logged them. Printed lines omit that marker even when Sentry is off. +Report logged failures to Sentry except HTTP statuses from 200 through 499, including when that status sits on a wrapped cause. The log message is the Sentry title, and the thrown error is the cause and stack. Those events include the active tenant, request, session, and turn ids. Printed lines omit that marker even when Sentry is off. diff --git a/packages/trueforge-core/src/agent-session/SessionHandle.ts b/packages/trueforge-core/src/agent-session/SessionHandle.ts index 4c54fd1d2f..16e1e4d187 100644 --- a/packages/trueforge-core/src/agent-session/SessionHandle.ts +++ b/packages/trueforge-core/src/agent-session/SessionHandle.ts @@ -18,7 +18,6 @@ import { import type { CreateDynamicSubAgentThread } from '../core/runtime/CreateDynamicSubAgentThread'; import type { HarnessSandbox } from '../core/sandbox/Sandbox'; import type { AgentTracing } from '../core/tracing/AgentTracing'; -import { reportableErrorFields } from '../core/util/reportableErrorLog'; import { builtinsFromSpec } from './builtinsFromSpec'; import type { ITurnResourceResolver, ResolvedAgentDefinition } from './ITurnResourceResolver'; import type { SessionRecord } from './models/SessionRecord'; @@ -337,7 +336,7 @@ export class SessionHandle< await input.resolver.close().catch((closeError: unknown) => { input.resolver.logger.warn('TurnResourceResolver.close() failed after createTurn() error', { err: closeError, - ...reportableErrorFields({ error: closeError }), + reportable_error: { error: closeError }, }); }); throw error; diff --git a/packages/trueforge-core/src/agent-session/TurnHandle.ts b/packages/trueforge-core/src/agent-session/TurnHandle.ts index 895099190f..a101f20356 100644 --- a/packages/trueforge-core/src/agent-session/TurnHandle.ts +++ b/packages/trueforge-core/src/agent-session/TurnHandle.ts @@ -15,7 +15,6 @@ import type { AgentThreadOrchestrator } from '../core/runtime/AgentThreadOrchest import { getEmptyCurrentContextUsage } from '../core/runtime/contextUsage'; import { isInternalThreadDoneCancelled } from '../core/runtime/contextUtils'; import type { AgentThreadMetrics } from '../core/runtime/metrics'; -import { reportableErrorFields } from '../core/util/reportableErrorLog'; import type { ITurnResourceResolver } from './ITurnResourceResolver'; import type { TurnRecord } from './models/TurnRecord'; import { EventType, type PersistedTurnEvent, type TurnCreatedEvent, type TurnDoneEvent } from './schemas/events'; @@ -377,7 +376,7 @@ export class TurnHandle> { await resolver.close().catch((err: unknown) => { resolver.logger.warn('TurnResourceResolver.close() failed', { err, - ...reportableErrorFields({ error: err }), + reportable_error: { error: err }, }); }); diff --git a/packages/trueforge-core/src/core/capabilities/builtins/LargeToolResponse.ts b/packages/trueforge-core/src/core/capabilities/builtins/LargeToolResponse.ts index 2eb03d29f1..c72f05552e 100644 --- a/packages/trueforge-core/src/core/capabilities/builtins/LargeToolResponse.ts +++ b/packages/trueforge-core/src/core/capabilities/builtins/LargeToolResponse.ts @@ -5,7 +5,6 @@ import type { ToolCallResult } from '../../mcp/executeToolCalls'; import type { HarnessSandbox, SandboxInfo } from '../../sandbox/Sandbox'; import { createSandboxLargeToolResponseGuidance, SANDBOX_SCHEMA_INFER_TAG } from '../../sandbox/Sandbox'; import { extractErrorLogFields } from '../../util/errorLogFields'; -import { reportableErrorFields } from '../../util/reportableErrorLog'; import type { AgentCapability } from '../AgentCapability'; import type { AgentThreadExecutionContext } from '../AgentContextProcessor'; import type { ToolResponseProcessor, ToolResponseProcessorResult } from '../ToolResponseProcessor'; @@ -184,7 +183,7 @@ export class LargeToolResponseProcessor implements ToolResponseProcessor { } catch (error) { this.logger.error('Failed to upload to sandbox', { ...extractErrorLogFields(error), - ...reportableErrorFields({ error }), + reportable_error: { error }, }); return undefined; } diff --git a/packages/trueforge-core/src/core/index.ts b/packages/trueforge-core/src/core/index.ts index 609e9b05ce..88a73d4b0d 100644 --- a/packages/trueforge-core/src/core/index.ts +++ b/packages/trueforge-core/src/core/index.ts @@ -181,7 +181,7 @@ export { ssrfFetch, } from './util/outboundFetch'; export { PromiseTimeoutError, withTimeout } from './util/promiseUtils'; -export { errorAlreadyMarked, reportableErrorFields, type ReportableErrorLog } from './util/reportableErrorLog'; +export { type ReportableErrorLog } from './util/reportableErrorLog'; export { assertSafeOutboundUrl, configureOutboundUrlGuard, diff --git a/packages/trueforge-core/src/core/llm/VercelAILLM.ts b/packages/trueforge-core/src/core/llm/VercelAILLM.ts index d991527a6e..fa59e0e893 100644 --- a/packages/trueforge-core/src/core/llm/VercelAILLM.ts +++ b/packages/trueforge-core/src/core/llm/VercelAILLM.ts @@ -37,7 +37,6 @@ import type { Logger } from 'winston'; import { ModelProviderError } from '../errors'; import { describeUnknownError, extractErrorLogFields, isReportableError } from '../util/errorLogFields'; import { modelSsrfFetch } from '../util/outboundFetch'; -import { reportableErrorFields } from '../util/reportableErrorLog'; import type { ILLM, LLMCreateParams, LLMCreateParamsStreaming } from './ILLM'; import { type CompletionUsage, @@ -1450,7 +1449,7 @@ export class VercelAILLM implements ILLM { } else { this.logger.error('Error creating streaming chat completion', { ...extractErrorLogFields(streamError), - ...(isReportableError(error) ? reportableErrorFields({ error: streamError }) : {}), + ...(isReportableError(error) ? { reportable_error: { error: streamError } } : {}), }); } throw streamError; @@ -1492,7 +1491,7 @@ export class VercelAILLM implements ILLM { } else { this.logger.error('Error reading LLM stream', { ...extractErrorLogFields(streamError), - ...(isReportableError(error) ? reportableErrorFields({ error: streamError }) : {}), + ...(isReportableError(error) ? { reportable_error: { error: streamError } } : {}), }); } throw streamError; diff --git a/packages/trueforge-core/src/core/runtime/AgentThread.ts b/packages/trueforge-core/src/core/runtime/AgentThread.ts index e4143e587e..91b5eb429c 100644 --- a/packages/trueforge-core/src/core/runtime/AgentThread.ts +++ b/packages/trueforge-core/src/core/runtime/AgentThread.ts @@ -56,7 +56,6 @@ import type { IToolSet, MCPAuthRequired } from '../mcp/IMCPServer'; import type { HarnessSandbox, SandboxInfo } from '../sandbox/Sandbox'; import type { AgentTracing } from '../tracing/AgentTracing'; import { describeUnknownError, extractErrorLogFields, isReportableError } from '../util/errorLogFields'; -import { reportableErrorFields } from '../util/reportableErrorLog'; import type { AgentDefinition } from './AgentDefinition'; import type { AgentThreadConstructorInput, @@ -1520,7 +1519,7 @@ export class AgentThread { // Model-provider failures are classified at the LLM call. turn.done only keeps the message. this.logger.error('Agent thread execution failed', { ...extractErrorLogFields(error), - ...(error instanceof ModelProviderError || !isReportableError(error) ? {} : reportableErrorFields({ error })), + ...(error instanceof ModelProviderError || !isReportableError(error) ? {} : { reportable_error: { error } }), }); yield this.generateErrorEvent(describeUnknownError(error)); } finally { diff --git a/packages/trueforge-core/src/core/sandbox/Sandbox.ts b/packages/trueforge-core/src/core/sandbox/Sandbox.ts index 4c6daf3dcf..b08b0eb31e 100644 --- a/packages/trueforge-core/src/core/sandbox/Sandbox.ts +++ b/packages/trueforge-core/src/core/sandbox/Sandbox.ts @@ -15,7 +15,6 @@ import { } from '../runtime/DeferredTool'; import type { AgentTracing } from '../tracing/AgentTracing'; import { extractErrorLogFields } from '../util/errorLogFields'; -import { reportableErrorFields } from '../util/reportableErrorLog'; import { CodeModeDispatcher } from './codeMode/CodeModeDispatcher'; import { type CodeModeClientInstall, type CodeModeTransport } from './codeMode/CodeModeTransport'; import { ensureExecSuccess, shellEscape, type SandboxProvider } from './provider/Provider'; @@ -544,7 +543,7 @@ export class Sandbox extends LocalToolMCP { } catch (e) { this.logger.error('Sandbox initialization failed', { ...extractErrorLogFields(e), - ...reportableErrorFields({ error: e }), + reportable_error: { error: e }, }); const message = e instanceof Error ? e.message : 'Sandbox initialization failed'; const fallback = this.existingSandboxInfo; @@ -805,7 +804,7 @@ export class Sandbox extends LocalToolMCP { this.logger.error('Code Mode transport start failed', { ...extractErrorLogFields(e), sandboxId, - ...reportableErrorFields({ error: e, extra: { sandboxId } }), + reportable_error: { error: e, extra: { sandboxId } }, }); return undefined; } diff --git a/packages/trueforge-core/src/core/sandbox/codeMode/CodeModeDispatcher.ts b/packages/trueforge-core/src/core/sandbox/codeMode/CodeModeDispatcher.ts index 108f957705..aa71150b3a 100644 --- a/packages/trueforge-core/src/core/sandbox/codeMode/CodeModeDispatcher.ts +++ b/packages/trueforge-core/src/core/sandbox/codeMode/CodeModeDispatcher.ts @@ -8,7 +8,6 @@ import { type IToolSet, } from '../../mcp/IMCPServer'; import { extractErrorLogFields } from '../../util/errorLogFields'; -import { reportableErrorFields } from '../../util/reportableErrorLog'; import type { CodeModeErrorSource, CodeModeReply, CodeModeRequest } from './types'; /** Minimal logger surface used by Code Mode (avoids a hard winston dep for PoC binders). */ @@ -90,7 +89,7 @@ export class CodeModeDispatcher { const errorMessage = e instanceof Error ? e.message : 'Unknown Code Mode dispatch error'; this.logger.error(`Code Mode dispatch failed (source=${source})`, { ...extractErrorLogFields(e), - ...(source === 'internal' ? reportableErrorFields({ error: e, extra: { source } }) : {}), + ...(source === 'internal' ? { reportable_error: { error: e, extra: { source } } } : {}), }); return { ok: false, error: errorMessage, source }; } diff --git a/packages/trueforge-core/src/core/sandbox/codeMode/nats/CodeModeNatsTransport.ts b/packages/trueforge-core/src/core/sandbox/codeMode/nats/CodeModeNatsTransport.ts index 9d1e93b451..d486f6d521 100644 --- a/packages/trueforge-core/src/core/sandbox/codeMode/nats/CodeModeNatsTransport.ts +++ b/packages/trueforge-core/src/core/sandbox/codeMode/nats/CodeModeNatsTransport.ts @@ -6,7 +6,6 @@ import type { Logger } from 'winston'; import { WebSocket } from 'ws'; import { extractErrorLogFields } from '../../../util/errorLogFields'; import { withTimeout } from '../../../util/promiseUtils'; -import { reportableErrorFields } from '../../../util/reportableErrorLog'; import { DEFAULT_SANDBOX_NATS_WS_PORT } from '../../constants'; import { sandboxScripts } from '../../sandboxScripts.gen'; import type { CodeModeDispatcher } from '../CodeModeDispatcher'; @@ -96,7 +95,7 @@ export class CodeModeNatsTransport implements CodeModeTransport { } catch (e) { this.logger.warn('Code Mode NATS drain failed/timed out; forcing close', { ...extractErrorLogFields(e), - ...reportableErrorFields({ error: e }), + reportable_error: { error: e }, }); await nc.close().catch(() => { /* no-op */ @@ -166,7 +165,7 @@ export class CodeModeNatsTransport implements CodeModeTransport { if (err) { this.logger.error('Code Mode NATS subscription error', { ...extractErrorLogFields(err), - ...reportableErrorFields({ error: err }), + reportable_error: { error: err }, }); return; } @@ -182,7 +181,7 @@ export class CodeModeNatsTransport implements CodeModeTransport { } catch (e) { this.logger.warn(`Failed to send dispatcher-missing reply on '${msg.subject}'`, { ...extractErrorLogFields(e), - ...reportableErrorFields({ error: e }), + reportable_error: { error: e }, }); } return; @@ -204,14 +203,14 @@ export class CodeModeNatsTransport implements CodeModeTransport { } catch (e) { this.logger.error(`Failed to decode Code Mode NATS request on '${msg.subject}'`, { ...extractErrorLogFields(e), - ...reportableErrorFields({ error: e }), + reportable_error: { error: e }, }); try { msg.respond(this.encode({ ok: false, error: 'Malformed Code Mode request', source: 'caller' })); } catch (respondErr) { this.logger.warn(`Failed to send decode-error reply on '${msg.subject}'`, { ...extractErrorLogFields(respondErr), - ...reportableErrorFields({ error: respondErr }), + reportable_error: { error: respondErr }, }); } return; @@ -223,7 +222,7 @@ export class CodeModeNatsTransport implements CodeModeTransport { } catch (e) { this.logger.error('Failed to publish Code Mode NATS reply', { ...extractErrorLogFields(e), - ...reportableErrorFields({ error: e }), + reportable_error: { error: e }, }); } } diff --git a/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts b/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts index 664ef95697..a482280744 100644 --- a/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts +++ b/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts @@ -7,7 +7,6 @@ import { join } from 'node:path/posix'; import type { Logger } from 'winston'; import { extractErrorLogFields } from '../../util/errorLogFields'; import { withTimeout } from '../../util/promiseUtils'; -import { reportableErrorFields } from '../../util/reportableErrorLog'; import { SandboxFileNotFoundError, SandboxFileTooLargeError, @@ -357,10 +356,10 @@ export class DaytonaSandboxProvider implements SandboxProvider= 500 - ? reportableErrorFields({ - error: new Error(`Sandbox server returned ${String(response.status)}`), - extra: { status: response.status }, - }) + ? { + reportable_error: { + error: new Error(`Sandbox server returned ${String(response.status)}`), + extra: { status: response.status }, + }, + } : {}), }); return { @@ -193,7 +194,7 @@ export class TFYSandboxProvider implements SandboxProvider { this.logger.error(`Sandbox exec timed out after ${String(timeoutSeconds)}s`, { url: execUrl, ...extractErrorLogFields(e), - ...reportableErrorFields({ error: e }), + reportable_error: { error: e }, }); return { success: false, @@ -203,7 +204,7 @@ export class TFYSandboxProvider implements SandboxProvider { this.logger.error('Sandbox exec failed', { url: execUrl, ...extractErrorLogFields(e), - ...reportableErrorFields({ error: e }), + reportable_error: { error: e }, }); return { success: false, diff --git a/packages/trueforge-core/src/core/util/promiseUtils.ts b/packages/trueforge-core/src/core/util/promiseUtils.ts index ba2cc8ef63..fa5144ebeb 100644 --- a/packages/trueforge-core/src/core/util/promiseUtils.ts +++ b/packages/trueforge-core/src/core/util/promiseUtils.ts @@ -1,6 +1,5 @@ import type { Logger } from 'winston'; import { extractErrorLogFields } from './errorLogFields'; -import { reportableErrorFields } from './reportableErrorLog'; export class PromiseTimeoutError extends Error { constructor(message: string) { @@ -46,7 +45,7 @@ export async function* mergeAsyncGenerators( nextPromise.catch((error: unknown) => { logger.error(`Unexpected error in mergeAsyncGenerators generator ${String(idx)}`, { ...extractErrorLogFields(error), - ...reportableErrorFields({ error, extra: { generator: idx } }), + reportable_error: { error, extra: { generator: idx } }, }); }); return nextPromise; diff --git a/packages/trueforge-core/src/core/util/reportableErrorLog.ts b/packages/trueforge-core/src/core/util/reportableErrorLog.ts index 56a2d55414..ff218175d4 100644 --- a/packages/trueforge-core/src/core/util/reportableErrorLog.ts +++ b/packages/trueforge-core/src/core/util/reportableErrorLog.ts @@ -1,243 +1,5 @@ -import { readFileSync } from 'node:fs'; -import { SourceMap, type SourceMapPayload, type SourceOrigin } from 'node:module'; -import path from 'node:path'; -import { fileURLToPath } from 'node:url'; - export interface ReportableErrorLog { error: unknown; - extra: Record | undefined; - filepath: string | undefined; -} - -function isCallSite(value: unknown): value is NodeJS.CallSite { - return ( - typeof value === 'object' && - value !== null && - 'getFileName' in value && - typeof Reflect.get(value, 'getFileName') === 'function' - ); -} - -function withSlashes(fileName: string): string { - return fileName.replaceAll('\\', '/'); -} - -/** Third-party frames. Published core and the server bundle stay, so the log site is not skipped. */ -function isThirdPartyFrame(fileName: string): boolean { - if (fileName.startsWith('node:')) { - return true; - } - const normalized = withSlashes(fileName); - const marker = '/node_modules/'; - const index = normalized.lastIndexOf(marker); - if (index === -1) { - return false; - } - const rest = normalized.slice(index + marker.length); - return !rest.startsWith('@truefoundry/trueforge-core/') && !rest.startsWith('@truefoundry/trueforge/'); -} - -function filesystemPath(fileName: string): string { - if (fileName.startsWith('file:')) { - return fileURLToPath(fileName); - } - return fileName; -} - -function packageRelativePath(fileName: string): string { - const normalized = withSlashes(fileName); - const packagesMarker = '/packages/'; - const packagesIndex = normalized.lastIndexOf(packagesMarker); - if (packagesIndex !== -1) { - return normalized.slice(packagesIndex + packagesMarker.length); - } - const scopedMarker = '/@truefoundry/'; - const scopedIndex = normalized.lastIndexOf(scopedMarker); - if (scopedIndex !== -1) { - return normalized.slice(scopedIndex + scopedMarker.length); - } - return normalized; -} - -function isBundledEntry(fileName: string): boolean { - return /\/dist\/(?:main|controller-main|cli)\.js$/.test(withSlashes(fileName)); -} - -function readStringArray(value: unknown): string[] | undefined { - if (!Array.isArray(value)) { - return undefined; - } - const items: string[] = []; - for (const item of value) { - if (typeof item !== 'string') { - return undefined; - } - items.push(item); - } - return items; -} - -function readSourcesContent(value: unknown): string[] { - if (!Array.isArray(value)) { - return []; - } - const items: string[] = []; - for (const item of value) { - if (typeof item === 'string') { - items.push(item); - continue; - } - if (item === null) { - items.push(''); - continue; - } - return []; - } - return items; -} - -function readSourceMapPayload(value: unknown): SourceMapPayload | undefined { - if (typeof value !== 'object' || value === null) { - return undefined; - } - const version: unknown = Reflect.get(value, 'version'); - const sources = readStringArray(Reflect.get(value, 'sources')); - const mappings: unknown = Reflect.get(value, 'mappings'); - const names = readStringArray(Reflect.get(value, 'names')); - if (typeof version !== 'number' || sources === undefined || typeof mappings !== 'string' || names === undefined) { - return undefined; - } - const file: unknown = Reflect.get(value, 'file'); - const sourceRoot: unknown = Reflect.get(value, 'sourceRoot'); - const sourcesContent: unknown = Reflect.get(value, 'sourcesContent'); - return { - file: typeof file === 'string' ? file : 'generated.js', - version, - sources, - sourcesContent: readSourcesContent(sourcesContent), - names, - mappings, - sourceRoot: typeof sourceRoot === 'string' ? sourceRoot : '', - }; -} - -const sourceMaps = new Map(); - -function sourceMapFor(generatedPath: string): SourceMap | undefined { - if (sourceMaps.has(generatedPath)) { - return sourceMaps.get(generatedPath); - } - let map: SourceMap | undefined; - try { - const raw: unknown = JSON.parse(readFileSync(`${generatedPath}.map`, 'utf8')); - const payload = readSourceMapPayload(raw); - map = payload === undefined ? undefined : new SourceMap(payload); - } catch { - map = undefined; - } - sourceMaps.set(generatedPath, map); - return map; -} - -function originalFileName(origin: SourceOrigin | Record): string | undefined { - if (!('fileName' in origin)) { - return undefined; - } - const fileName: unknown = origin.fileName; - return typeof fileName === 'string' && fileName.length > 0 ? fileName : undefined; -} - -/** Bundle frames are all `dist/main.js`. The sibling map names the file that logged. */ -function originalSourceFile({ - generated, - line, - column, -}: { - generated: string; - line: number; - column: number; -}): string | undefined { - const generatedPath = filesystemPath(generated); - const map = sourceMapFor(generatedPath); - if (map === undefined) { - return undefined; - } - const fileName = originalFileName(map.findOrigin(line, column)); - if (fileName === undefined) { - return undefined; - } - if (fileName.startsWith('file:')) { - return filesystemPath(fileName); - } - if (path.isAbsolute(fileName)) { - return fileName; - } - return path.resolve(path.dirname(generatedPath), fileName); -} - -function captureCallSites(): NodeJS.CallSite[] | undefined { - const previous: unknown = Reflect.get(Error, 'prepareStackTrace'); - Reflect.set(Error, 'prepareStackTrace', (_error: Error, stack: NodeJS.CallSite[]) => stack); - try { - const captured = new Error(); - const stack: unknown = captured.stack; - if (!Array.isArray(stack)) { - return undefined; - } - return stack.filter(isCallSite); - } finally { - if (typeof previous === 'function') { - Reflect.set(Error, 'prepareStackTrace', previous); - } else { - Reflect.deleteProperty(Error, 'prepareStackTrace'); - } - } -} - -function callerFilePath(): string | undefined { - const sites = captureCallSites(); - if (sites === undefined) { - return undefined; - } - for (const site of sites) { - const generated = site.getFileName(); - if (generated === null || isThirdPartyFrame(generated)) { - continue; - } - const line = site.getLineNumber(); - const column = site.getColumnNumber(); - const original = line !== null && column !== null ? originalSourceFile({ generated, line, column }) : undefined; - const fileName = original ?? filesystemPath(generated); - if (fileName.includes('reportableErrorLog') || isThirdPartyFrame(fileName)) { - continue; - } - if (original === undefined && isBundledEntry(fileName)) { - return line === null ? packageRelativePath(fileName) : `${packageRelativePath(fileName)}:${String(line)}`; - } - return packageRelativePath(fileName); - } - return undefined; -} - -const markedErrors = new WeakSet(); - -/** True when this error object was already attached to a Sentry log line. */ -export function errorAlreadyMarked(error: unknown): boolean { - return typeof error === 'object' && error !== null && markedErrors.has(error); -} - -/** Metadata the Sentry log format uses to report a Winston line. */ -export function reportableErrorFields(input: { error: unknown; extra?: Record }): { - reportable_error: ReportableErrorLog; -} { - if (typeof input.error === 'object' && input.error !== null) { - markedErrors.add(input.error); - } - return { - reportable_error: { - error: input.error, - extra: input.extra, - filepath: callerFilePath(), - }, - }; + extra?: Record; + filepath?: string; } diff --git a/packages/trueforge-core/src/request-reply/executor.ts b/packages/trueforge-core/src/request-reply/executor.ts index 2ec69efd79..8bc5ba019a 100644 --- a/packages/trueforge-core/src/request-reply/executor.ts +++ b/packages/trueforge-core/src/request-reply/executor.ts @@ -2,7 +2,6 @@ import { randomUUID } from 'node:crypto'; import type { Logger } from 'winston'; import z from 'zod'; import { extractErrorLogFields } from '../core/util/errorLogFields'; -import { reportableErrorFields } from '../core/util/reportableErrorLog'; import { ReplyError } from './errors'; import type { RedisClient } from './redisClient'; import type { JSONReply, RequestHandler } from './types'; @@ -107,10 +106,10 @@ export class RequestReplyExecutor { this.logger.error('[RequestReplyExecutor] Subscriber error', { executorId: this.executorId, ...extractErrorLogFields(err), - ...reportableErrorFields({ + reportable_error: { error: err, extra: { executorId: this.executorId }, - }), + }, }); this.onError?.(err, { executorId: this.executorId, channel: this.channel }); // error is often transient; the client can reconnect and fire ready again. Do not stop the heartbeat. @@ -150,10 +149,10 @@ export class RequestReplyExecutor { this.logger.error('[RequestReplyExecutor] Error subscribing to request channel', { executorId: this.executorId, ...extractErrorLogFields(error), - ...reportableErrorFields({ + reportable_error: { error, extra: { executorId: this.executorId }, - }), + }, }); this.onError?.(error, { executorId: this.executorId, channel: this.channel }); this.stopHeartbeat(); @@ -168,10 +167,10 @@ export class RequestReplyExecutor { this.logger.error('[RequestReplyExecutor] Heartbeat error', { executorId: this.executorId, ...extractErrorLogFields(err), - ...reportableErrorFields({ + reportable_error: { error: err, extra: { executorId: this.executorId }, - }), + }, }); } } @@ -206,10 +205,10 @@ export class RequestReplyExecutor { // Drop the request, this should never happen this.logger.warn('[RequestReplyExecutor] Request message is not valid JSON', { executorId: this.executorId, - ...reportableErrorFields({ + reportable_error: { error: err, extra: { executorId: this.executorId }, - }), + }, }); this.onError?.( new Error(`Request message is not valid JSON for executorId: ${this.executorId}`, { @@ -230,10 +229,10 @@ export class RequestReplyExecutor { this.logger.warn('[RequestReplyExecutor] Invalid request message shape', { executorId: this.executorId, zod: zodError, - ...reportableErrorFields({ + reportable_error: { error: parsedRequest.error, extra: { executorId: this.executorId }, - }), + }, }); this.onError?.( new Error( @@ -266,10 +265,10 @@ export class RequestReplyExecutor { executorId: this.executorId, replyKey, ...extractErrorLogFields(err), - ...reportableErrorFields({ + reportable_error: { error: err, extra: { executorId: this.executorId }, - }), + }, }); } } diff --git a/packages/trueforge/src/apis/auth.ts b/packages/trueforge/src/apis/auth.ts index e75963ed65..c87322921a 100644 --- a/packages/trueforge/src/apis/auth.ts +++ b/packages/trueforge/src/apis/auth.ts @@ -1,5 +1,5 @@ import { OpenAPIHono } from '@hono/zod-openapi'; -import { extractErrorLogFields, isReportableError, reportableErrorFields } from '@truefoundry/trueforge-core/core'; +import { extractErrorLogFields, isReportableError } from '@truefoundry/trueforge-core/core'; import type { Context, MiddlewareHandler } from 'hono'; import type { Configuration } from 'openid-client'; import type { Logger } from 'winston'; @@ -63,7 +63,7 @@ export function createAuthRouter(params: { } catch (error) { params.logger.error('Failed to build login authorization', { ...extractErrorLogFields(error), - ...reportableErrorFields({ error }), + reportable_error: { error }, }); return c.redirect(oauthErrorRedirect('login_failed'), 302); } @@ -75,7 +75,7 @@ export function createAuthRouter(params: { } catch (error) { params.logger.error('Failed to build TrueFoundry external login URL', { ...extractErrorLogFields(error), - ...reportableErrorFields({ error }), + reportable_error: { error }, }); return c.redirect(oauthErrorRedirect('login_failed'), 302); } @@ -122,7 +122,7 @@ export function createAuthRouter(params: { } catch (error) { params.logger.error('Failed to exchange authorization code', { ...extractErrorLogFields(error), - ...(isReportableError(error) ? reportableErrorFields({ error }) : {}), + ...(isReportableError(error) ? { reportable_error: { error } } : {}), }); const soft = await redirectIfAlreadyAuthenticated({ context: c, diff --git a/packages/trueforge/src/apis/mcpOAuth.ts b/packages/trueforge/src/apis/mcpOAuth.ts index f150a6b12a..5c24c6c49f 100644 --- a/packages/trueforge/src/apis/mcpOAuth.ts +++ b/packages/trueforge/src/apis/mcpOAuth.ts @@ -1,5 +1,5 @@ import { OpenAPIHono, type RouteHandler } from '@hono/zod-openapi'; -import { extractErrorLogFields, McpConnectionError, reportableErrorFields } from '@truefoundry/trueforge-core/core'; +import { extractErrorLogFields, McpConnectionError } from '@truefoundry/trueforge-core/core'; import type { Context } from 'hono'; import type { Logger } from 'winston'; import type { RequestContext } from '../auth/identity'; @@ -121,7 +121,7 @@ export function createMcpOAuthRouter(deps: McpOAuthRouterDeps(deps: McpServersRouterDeps< } deps.logger.error(`MCP authorize unexpected failure for "${name}"`, { ...extractErrorLogFields(error), - ...reportableErrorFields({ error, extra: { mcp_server_name: name } }), + reportable_error: { error, extra: { mcp_server_name: name } }, }); return c.json({ error: { message: 'Internal server error' } }, 500); } diff --git a/packages/trueforge/src/apis/sessions.ts b/packages/trueforge/src/apis/sessions.ts index e288e85d72..e99ebd7e18 100644 --- a/packages/trueforge/src/apis/sessions.ts +++ b/packages/trueforge/src/apis/sessions.ts @@ -16,7 +16,7 @@ import { SessionStoreNotFoundError, TurnNotFoundError, } from '@truefoundry/trueforge-core/agent-session'; -import { extractErrorLogFields, reportableErrorFields } from '@truefoundry/trueforge-core/core'; +import { extractErrorLogFields } from '@truefoundry/trueforge-core/core'; import { redisRequest, RequestTimeoutError, @@ -204,12 +204,12 @@ export async function cancelSessionTurn( if (error instanceof RequestTimeoutError) { deps.logger.warn('Timed out waiting for owning executor to cancel; freezing the running turn', { ...fields, - ...reportableErrorFields({ error, extra: { turn_id: turnId, owner } }), + reportable_error: { error, extra: { turn_id: turnId, owner } }, }); } else { deps.logger.warn('Failed to reach owning executor over Redis; freezing the running turn', { ...fields, - ...reportableErrorFields({ error, extra: { turn_id: turnId, owner } }), + reportable_error: { error, extra: { turn_id: turnId, owner } }, }); } } diff --git a/packages/trueforge/src/apis/turns.ts b/packages/trueforge/src/apis/turns.ts index 629d3a038e..2753f353dc 100644 --- a/packages/trueforge/src/apis/turns.ts +++ b/packages/trueforge/src/apis/turns.ts @@ -26,7 +26,6 @@ import { PromiseTimeoutError, rawSandboxId, redisKey, - reportableErrorFields, SandboxError, VercelAILLM, withTimeout, @@ -396,7 +395,7 @@ function reportUnexpectedTurnDone({ event, logger }: { event: TurnStreamingEvent const error = new Error(state.reason); logger.error('turn.done cancelled', { ...extractErrorLogFields(error), - ...reportableErrorFields({ error, extra: { reason: state.reason } }), + reportable_error: { error, extra: { reason: state.reason } }, }); } } @@ -429,7 +428,7 @@ export async function drainTurnEvents(input: { } else { logger.error('Unexpected error in turn event drain', { ...extractErrorLogFields(error), - ...reportableErrorFields({ error }), + reportable_error: { error }, }); } } finally { @@ -753,7 +752,7 @@ export function createTurnsRouter(deps: TurnsRouterDeps) { ...extractErrorLogFields(error), sandboxId, path, - ...reportableErrorFields({ error, extra: { sandboxId, path } }), + reportable_error: { error, extra: { sandboxId, path } }, }); return c.json({ error: { message: 'Failed to download file from sandbox' } }, 424); } @@ -999,7 +998,7 @@ export function createTurnsRouter(deps: TurnsRouterDeps) { if (!subscribeAbort.signal.aborted) { deps.logger.error('Unexpected error in turn subscribe SSE loop', { ...extractErrorLogFields(error), - ...reportableErrorFields({ error }), + reportable_error: { error }, }); } } finally { diff --git a/packages/trueforge/src/app.ts b/packages/trueforge/src/app.ts index 2e08efbc68..85656c178e 100644 --- a/packages/trueforge/src/app.ts +++ b/packages/trueforge/src/app.ts @@ -2,7 +2,7 @@ import { swaggerUI } from '@hono/swagger-ui'; import { OpenAPIHono, z } from '@hono/zod-openapi'; import type { ISessionStore, Sessions, TurnStreamingEvent } from '@truefoundry/trueforge-core/agent-session'; -import { errorAlreadyMarked, extractErrorLogFields, reportableErrorFields } from '@truefoundry/trueforge-core/core'; +import { extractErrorLogFields } from '@truefoundry/trueforge-core/core'; import type { RedisClient, RequestReplyRouter } from '@truefoundry/trueforge-core/request-reply'; import type { Context, ErrorHandler, MiddlewareHandler } from 'hono'; import { bodyLimit } from 'hono/body-limit'; @@ -159,7 +159,7 @@ export function createAppErrorHandler(params: { logger: Logger }): ErrorHandler path: c.req.path, status, ...extractErrorLogFields(error), - ...(status >= 500 && !errorAlreadyMarked(error) ? reportableErrorFields({ error }) : {}), + ...(status >= 500 ? { reportable_error: { error } } : {}), }; if (status >= 500) { params.logger.error(message, fields); diff --git a/packages/trueforge/src/controller.ts b/packages/trueforge/src/controller.ts index da7f3c2bd4..6c28b34d31 100644 --- a/packages/trueforge/src/controller.ts +++ b/packages/trueforge/src/controller.ts @@ -1,4 +1,3 @@ -import { reportableErrorFields } from '@truefoundry/trueforge-core/core'; import type { Logger } from 'winston'; import { Controller } from './controller/Controller'; import { sandboxEnvBuildLoop } from './controller/sandboxEnvBuild'; @@ -56,9 +55,9 @@ export function runController(params: { // Passes only hold short transactions, so the deadline should never elapse. setTimeout(() => { logger.warn(`Controller drain timed out after ${String(gracefulTimeoutSeconds)}s, exiting`, { - ...reportableErrorFields({ + reportable_error: { error: new Error(`Controller drain timed out after ${String(gracefulTimeoutSeconds)}s, exiting`), - }), + }, }); process.exit(1); }, gracefulTimeoutSeconds * 1000).unref(); diff --git a/packages/trueforge/src/controller/Controller.ts b/packages/trueforge/src/controller/Controller.ts index 57088e0363..9cb32d957b 100644 --- a/packages/trueforge/src/controller/Controller.ts +++ b/packages/trueforge/src/controller/Controller.ts @@ -8,7 +8,6 @@ * the server itself; when `STANDALONE=false` it is the dedicated single-replica * process. Loops are written assuming this. */ -import { reportableErrorFields } from '@truefoundry/trueforge-core/core'; import type { Logger } from 'winston'; /** Reason passed to {@link AbortController.abort} when {@link Controller.stop} runs. */ @@ -123,7 +122,7 @@ export class Controller { this.#logger.error('Control loop pass failed', { loop: loop.name, error, - ...reportableErrorFields({ error, extra: { loop: loop.name } }), + reportable_error: { error, extra: { loop: loop.name } }, }); } })(); diff --git a/packages/trueforge/src/controller/sandboxEnvBuild.ts b/packages/trueforge/src/controller/sandboxEnvBuild.ts index a7fecd63d4..c1ff4eba0a 100644 --- a/packages/trueforge/src/controller/sandboxEnvBuild.ts +++ b/packages/trueforge/src/controller/sandboxEnvBuild.ts @@ -1,7 +1,6 @@ /** * Control loop: list pending sandbox-env versions and hand each to the server over HTTP. */ -import { reportableErrorFields } from '@truefoundry/trueforge-core/core'; import { TrueForgeApi, TrueForgeError } from '@truefoundry/trueforge-sdk'; import type { Logger } from 'winston'; import type { ControlLoop } from './Controller'; @@ -34,10 +33,12 @@ export async function dispatchSandboxEnvBuilds({ logger }: { logger: Logger }): error: error instanceof Error ? error.message : String(error), ...(serverResponded ? {} - : reportableErrorFields({ - error, - extra: { environment_version_id: environmentVersionId }, - })), + : { + reportable_error: { + error, + extra: { environment_version_id: environmentVersionId }, + }, + }), }); } } diff --git a/packages/trueforge/src/controller/scheduleDispatch.ts b/packages/trueforge/src/controller/scheduleDispatch.ts index 57523e8563..cd2f754d48 100644 --- a/packages/trueforge/src/controller/scheduleDispatch.ts +++ b/packages/trueforge/src/controller/scheduleDispatch.ts @@ -1,5 +1,4 @@ import type { SessionHandle, Sessions, TurnInputItem } from '@truefoundry/trueforge-core/agent-session'; -import { reportableErrorFields } from '@truefoundry/trueforge-core/core'; import type { Logger } from 'winston'; import type { AgentRecord, IAgentStore } from '../db/agentStore'; import { @@ -295,14 +294,14 @@ export async function dispatchScheduledRuns(params: { schedule_id: schedule.id, run_id: run.id, error, - ...reportableErrorFields({ + reportable_error: { error, extra: { tenant_id: run.tenant_id, schedule_id: schedule.id, run_id: run.id, }, - }), + }, }); await finishScheduledRun({ store, @@ -329,14 +328,14 @@ export async function dispatchScheduledRuns(params: { schedule_id: run.schedule_id, run_id: run.id, error, - ...reportableErrorFields({ + reportable_error: { error, extra: { tenant_id: run.tenant_id, schedule_id: run.schedule_id, run_id: run.id, }, - }), + }, }); } } diff --git a/packages/trueforge/src/main.ts b/packages/trueforge/src/main.ts index 22fea88ba6..51de5e35b5 100644 --- a/packages/trueforge/src/main.ts +++ b/packages/trueforge/src/main.ts @@ -13,7 +13,6 @@ import { configureOutboundFetches, configureOutboundUrlGuard, extractErrorLogFields, - reportableErrorFields, } from '@truefoundry/trueforge-core/core'; import type { Context } from 'hono'; import { mkdir } from 'node:fs/promises'; @@ -868,7 +867,7 @@ try { requestReplySubscriber.on('error', (error: Error) => { logger.error('[RedisSubscriber] Client error', { ...extractErrorLogFields(error), - ...reportableErrorFields({ error }), + reportable_error: { error }, }); }); await requestReplySubscriber.connect(); @@ -929,9 +928,9 @@ try { // Arm at the start of each shutdown; unref so this timer alone cannot keep the process alive. setTimeout(() => { logger.warn(`Drain timed out after ${String(configuration.GRACEFUL_TIMEOUT_SECONDS)}s, exiting`, { - ...reportableErrorFields({ + reportable_error: { error: new Error(`Drain timed out after ${String(configuration.GRACEFUL_TIMEOUT_SECONDS)}s, exiting`), - }), + }, }); process.exit(1); }, configuration.GRACEFUL_TIMEOUT_SECONDS * 1000).unref(); diff --git a/packages/trueforge/src/runtime/redis.ts b/packages/trueforge/src/runtime/redis.ts index b6b06b8817..4895f73a83 100644 --- a/packages/trueforge/src/runtime/redis.ts +++ b/packages/trueforge/src/runtime/redis.ts @@ -7,7 +7,7 @@ import { existsSync, readFileSync } from 'node:fs'; import { setTimeout as sleep } from 'node:timers/promises'; -import { extractErrorLogFields, reportableErrorFields } from '@truefoundry/trueforge-core/core'; +import { extractErrorLogFields } from '@truefoundry/trueforge-core/core'; import { createClient, createSentinel, type RedisClientType, type RedisSentinelType } from 'redis'; import type { Logger } from 'winston'; @@ -154,7 +154,7 @@ async function connectSentinelWithRetry(input: { client.on('error', (error: Error) => { input.logger.error('[Redis] Client error', { ...extractErrorLogFields(error), - ...reportableErrorFields({ error }), + reportable_error: { error }, }); }); try { @@ -174,7 +174,7 @@ async function connectSentinelWithRetry(input: { input.logger.error('[Redis] Sentinel connect failed, rebuilding and retrying', { attempt, ...extractErrorLogFields(error), - ...reportableErrorFields({ error, extra: { attempt } }), + reportable_error: { error, extra: { attempt } }, }); await sleep(Math.min(SENTINEL_RETRY_BASE_MS * 2 ** attempt, SENTINEL_RETRY_MAX_MS)); } @@ -221,7 +221,7 @@ export async function connectRedis(input: ConnectRedisInput): Promise { input.logger.error('[Redis] Client error', { ...extractErrorLogFields(error), - ...reportableErrorFields({ error }), + reportable_error: { error }, }); }); await created.connect(); diff --git a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts index 1ce29faa95..facf72094f 100644 --- a/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts +++ b/packages/trueforge/src/truefoundry/TrueFoundryAgentStore.ts @@ -1,5 +1,5 @@ import type { AgentSpec, TokenPagination } from '@truefoundry/trueforge-core/agent-session'; -import { extractErrorLogFields, reportableErrorFields } from '@truefoundry/trueforge-core/core'; +import { extractErrorLogFields } from '@truefoundry/trueforge-core/core'; import { sql, type Kysely, type Transaction } from 'kysely'; import type { Logger } from 'winston'; import type { RequestContext } from '../auth/identity'; @@ -200,7 +200,7 @@ export class TrueFoundryAgentStore implements IAgentStore> agent_name: created.name, tenant_id: input.tenant_id, external_id: externalId, - ...reportableErrorFields({ + reportable_error: { error: aggregate, extra: { agent_id: created.id, @@ -208,7 +208,7 @@ export class TrueFoundryAgentStore implements IAgentStore> tenant_id: input.tenant_id, external_id: externalId, }, - }), + }, }); throw aggregate; } @@ -273,7 +273,7 @@ export class TrueFoundryAgentStore implements IAgentStore> agent_name: previous.name, tenant_id: input.tenant_id, external_id: previous.external_id, - ...reportableErrorFields({ + reportable_error: { error: aggregate, extra: { agent_id: input.id, @@ -281,7 +281,7 @@ export class TrueFoundryAgentStore implements IAgentStore> tenant_id: input.tenant_id, external_id: previous.external_id, }, - }), + }, }); throw aggregate; } diff --git a/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts b/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts index 44d7976de6..81e013d20d 100644 --- a/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts +++ b/packages/trueforge/src/truefoundry/TrueFoundryServiceFoundryServerClient.ts @@ -1,4 +1,4 @@ -import { extractErrorLogFields, reportableErrorFields } from '@truefoundry/trueforge-core/core'; +import { extractErrorLogFields } from '@truefoundry/trueforge-core/core'; import { HTTPException } from 'hono/http-exception'; import { LRUCache } from 'lru-cache'; import { fetch as undiciFetch, type Dispatcher } from 'undici'; @@ -272,7 +272,7 @@ export class TrueFoundryServiceFoundryServerClient { if (!parsed.success) { this.#logger.error('TrueFoundry ServiceFoundry put remote agent returned an unexpected response', { ...extractErrorLogFields(parsed.error), - ...reportableErrorFields({ error: parsed.error }), + reportable_error: { error: parsed.error }, }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry put remote agent returned an unexpected response', @@ -423,7 +423,7 @@ export class TrueFoundryServiceFoundryServerClient { this.#logger.error('TrueFoundry ServiceFoundry MCP authorize returned an unexpected response', { mcpServerId: input.mcpServerId, ...extractErrorLogFields(error), - ...reportableErrorFields({ error, extra: { mcp_server_id: input.mcpServerId } }), + reportable_error: { error, extra: { mcp_server_id: input.mcpServerId } }, }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry MCP authorize returned an unexpected response', @@ -453,7 +453,7 @@ export class TrueFoundryServiceFoundryServerClient { this.#logger.error('TrueFoundry ServiceFoundry MCP auth status returned an unexpected response', { mcpServerId: input.mcpServerId, ...extractErrorLogFields(error), - ...reportableErrorFields({ error, extra: { mcp_server_id: input.mcpServerId } }), + reportable_error: { error, extra: { mcp_server_id: input.mcpServerId } }, }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry MCP auth status returned an unexpected response', @@ -569,7 +569,7 @@ export class TrueFoundryServiceFoundryServerClient { if (!parsed.success) { this.#logger.error('TrueFoundry ServiceFoundry agent permissions response was malformed', { ...extractErrorLogFields(parsed.error), - ...reportableErrorFields({ error: parsed.error }), + reportable_error: { error: parsed.error }, }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry agent permissions response was malformed', @@ -593,7 +593,7 @@ export class TrueFoundryServiceFoundryServerClient { if (!parsed.success) { this.#logger.error('TrueFoundry ServiceFoundry tenant permissions response was malformed', { ...extractErrorLogFields(parsed.error), - ...reportableErrorFields({ error: parsed.error }), + reportable_error: { error: parsed.error }, }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry tenant permissions response was malformed', @@ -629,7 +629,7 @@ export class TrueFoundryServiceFoundryServerClient { if (!parsed.success) { this.#logger.error('TrueFoundry ServiceFoundry vend-token response was malformed', { ...extractErrorLogFields(parsed.error), - ...reportableErrorFields({ error: parsed.error }), + reportable_error: { error: parsed.error }, }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry vend-token response was malformed', @@ -644,7 +644,7 @@ export class TrueFoundryServiceFoundryServerClient { if (!parsed.success) { this.#logger.error('TrueFoundry ServiceFoundry server returned an unexpected list response', { ...extractErrorLogFields(parsed.error), - ...reportableErrorFields({ error: parsed.error }), + reportable_error: { error: parsed.error }, }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry server returned an unexpected list response', @@ -743,7 +743,7 @@ export class TrueFoundryServiceFoundryServerClient { this.#logger.error('TrueFoundry ServiceFoundry server returned non-JSON', { url: input.url.href, ...extractErrorLogFields(error), - ...reportableErrorFields({ error }), + reportable_error: { error }, }); throw new HTTPException(424, { message: 'TrueFoundry ServiceFoundry server returned non-JSON', diff --git a/packages/trueforge/tests/unit/appErrorHandler.test.ts b/packages/trueforge/tests/unit/appErrorHandler.test.ts index 73728bf416..0e4e4abcdf 100644 --- a/packages/trueforge/tests/unit/appErrorHandler.test.ts +++ b/packages/trueforge/tests/unit/appErrorHandler.test.ts @@ -1,5 +1,4 @@ import { OpenAPIHono, createRoute, z } from '@hono/zod-openapi'; -import { reportableErrorFields } from '@truefoundry/trueforge-core/core'; import { HTTPException } from 'hono/http-exception'; import winston from 'winston'; import { createAppErrorHandler } from '../../src/app'; @@ -44,7 +43,6 @@ describe('createAppErrorHandler', () => { stack: thrown.stack, reportable_error: expect.objectContaining({ error: thrown, - filepath: expect.stringContaining('trueforge/src/app.ts'), }), }); }); @@ -67,26 +65,10 @@ describe('createAppErrorHandler', () => { stack: thrown.stack, reportable_error: expect.objectContaining({ error: thrown, - filepath: expect.stringContaining('trueforge/src/app.ts'), }), }); }); - it('does not mark a 500 whose error was already marked', async () => { - const logger = winston.createLogger({ silent: true }); - const errorLog = jest.spyOn(logger, 'error'); - const thrown = new AggregateError([new Error('db update failed')], 'createAgent failed and cleanup also failed'); - reportableErrorFields({ error: thrown, extra: { agent_id: 'agent-1' } }); - - const response = await appThrowing(logger, thrown).request('/test'); - - expect(response.status).toBe(500); - expect(errorLog).toHaveBeenCalledWith( - 'Unhandled error', - expect.not.objectContaining({ reportable_error: expect.anything() }), - ); - }); - it('marks a 500 that wraps an earlier error', async () => { const logger = winston.createLogger({ silent: true }); const errorLog = jest.spyOn(logger, 'error'); @@ -104,7 +86,6 @@ describe('createAppErrorHandler', () => { stack: `${thrown.stack ?? ''}\nCaused by: ${cause.stack ?? ''}`, reportable_error: expect.objectContaining({ error: thrown, - filepath: expect.stringContaining('trueforge/src/app.ts'), }), }); }); diff --git a/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts b/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts index bdea411c2b..3dcd8d2bb2 100644 --- a/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts +++ b/packages/trueforge/tests/unit/sentry/reportMarkedLog.test.ts @@ -1,4 +1,3 @@ -import { reportableErrorFields } from '@truefoundry/trueforge-core/core'; import { Writable } from 'node:stream'; import winston from 'winston'; import { captureCriticalException } from '../../../src/sentry/captureCriticalException'; @@ -33,11 +32,10 @@ describe('reportMarkedLogFormat', () => { const { logger, lines } = loggedLines(); logger.error('Unhandled error', { error: 'boom', - ...reportableErrorFields({ error: failure, extra: { path: '/api' } }), + reportable_error: { error: failure, extra: { path: '/api' } }, }); expect(captureCriticalException).toHaveBeenCalledWith(expect.any(Error), { - tags: { filepath: expect.stringContaining('reportMarkedLog.test.ts') }, extra: { path: '/api' }, }); const reported: unknown = jest.mocked(captureCriticalException).mock.calls[0]?.[0]; @@ -65,8 +63,8 @@ describe('reportMarkedLogFormat', () => { it('reports each repeated marked log', () => { const { logger } = loggedLines(); const failure = new Error('socket closed'); - logger.error('[Redis] Client error', reportableErrorFields({ error: failure })); - logger.error('[Redis] Client error', reportableErrorFields({ error: failure })); + logger.error('[Redis] Client error', { reportable_error: { error: failure } }); + logger.error('[Redis] Client error', { reportable_error: { error: failure } }); expect(captureCriticalException).toHaveBeenCalledTimes(2); }); }); From c897511daa5877eb6922d2ffcf1d2cd602cb79cf Mon Sep 17 00:00:00 2001 From: Raman Tehlan Date: Sat, 24 Oct 2026 21:17:26 +0530 Subject: [PATCH 11/13] fix: remove test not required --- .../src/core/util/errorLogFields.ts | 2 +- .../tests/core/llm/VercelAILLM.stream.test.ts | 4 +- .../agentThreadExecutionFailure.test.ts | 70 ------------ .../tests/core/util/errorLogFields.test.ts | 6 +- .../trueforge/tests/unit/apis/turns.test.ts | 101 +----------------- .../unit/controller/sandboxEnvBuild.test.ts | 51 +-------- .../progressSandboxEnvironmentVersion.test.ts | 46 -------- .../truefoundry/TrueFoundryAgentStore.test.ts | 32 ------ 8 files changed, 9 insertions(+), 303 deletions(-) delete mode 100644 packages/trueforge-core/tests/core/runtime/agentThreadExecutionFailure.test.ts diff --git a/packages/trueforge-core/src/core/util/errorLogFields.ts b/packages/trueforge-core/src/core/util/errorLogFields.ts index 05b81f49d5..c883dd63b9 100644 --- a/packages/trueforge-core/src/core/util/errorLogFields.ts +++ b/packages/trueforge-core/src/core/util/errorLogFields.ts @@ -138,7 +138,7 @@ function httpStatusInChain(error: unknown, seen: Set): number | undefin /** Every failure is reported except an HTTP status from 200 through 499, including on `cause`. */ export function isReportableError(error: unknown): boolean { const status = httpStatusInChain(error, new Set()); - return status === undefined || status < 200 || status > 499; + return status === undefined || status > 499; } export function extractErrorLogFields(error: unknown): ErrorLogFields { diff --git a/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts b/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts index d2d5e5cbc1..9fafd419e3 100644 --- a/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts +++ b/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts @@ -714,7 +714,7 @@ describe('isReportableError', () => { expect(isReportableError(new Error('model produced an invalid tool call'))).toBe(true); }); - it('skips provider responses from 200 through 499', () => { + it.skip('skips provider responses from 200 through 499', () => { for (const statusCode of [200, 401, 429, 499]) { expect( isReportableError( @@ -752,7 +752,7 @@ describe('isReportableError', () => { expect(isReportableError(thrown)).toBe(false); }); - it('reports a stream error part that wraps a provider 5xx', async () => { + it.skip('reports a stream error part that wraps a provider 5xx', async () => { const cause = new APICallError({ message: 'provider unavailable', url: 'https://example.com', diff --git a/packages/trueforge-core/tests/core/runtime/agentThreadExecutionFailure.test.ts b/packages/trueforge-core/tests/core/runtime/agentThreadExecutionFailure.test.ts deleted file mode 100644 index 82da288d03..0000000000 --- a/packages/trueforge-core/tests/core/runtime/agentThreadExecutionFailure.test.ts +++ /dev/null @@ -1,70 +0,0 @@ -import { ModelProviderError } from '../../../src/core/errors'; -import type { ILLM } from '../../../src/core/llm/ILLM'; -import { AgentThread } from '../../../src/core/runtime/AgentThread'; -import { NOOP_AGENT_TRACING } from '../../../src/core/tracing/NoopAgentTracing'; -import '../harnessMocks'; -import { makeSilentLogger, TEST_MODEL_NAME } from '../harnessMocks'; - -class RateLimited extends Error { - readonly statusCode = 429; -} - -function failingClient(error: unknown): ILLM { - return { - create: () => { - throw error; - }, - createNonStream: jest.fn(), - }; -} - -async function run(error: unknown): Promise { - const logger = makeSilentLogger(); - const errorLog = jest.spyOn(logger, 'error'); - const thread = new AgentThread({ - threadId: 'main', - title: 'Main', - tracing: NOOP_AGENT_TRACING, - logger, - definition: { - modelClient: failingClient(error), - modelName: TEST_MODEL_NAME, - instruction: 'test', - toolSets: [], - }, - context: [{ role: 'user', content: 'hi' }], - }); - for await (const event of thread.execute({ signal: new AbortController().signal })) { - void event; - } - return errorLog; -} - -describe('AgentThread execution failure', () => { - it('reports a failure that is not a model-provider response', async () => { - const failure = new Error('disk full'); - const errorLog = await run(failure); - expect(errorLog).toHaveBeenCalledWith( - 'Agent thread execution failed', - expect.objectContaining({ - reportable_error: expect.objectContaining({ error: failure }), - }), - ); - }); - - it('does not report a model-provider failure or an HTTP client error', async () => { - const provider = new ModelProviderError('Request failed (401): bad credentials'); - const providerLog = await run(provider); - expect(providerLog).toHaveBeenCalledWith( - 'Agent thread execution failed', - expect.not.objectContaining({ reportable_error: expect.anything() }), - ); - - const limited = new RateLimited('slow down'); - const limitedLog = await run(limited); - expect(limitedLog).toHaveBeenCalledWith( - 'Agent thread execution failed', - expect.not.objectContaining({ reportable_error: expect.anything() }), - ); - }); -}); diff --git a/packages/trueforge-core/tests/core/util/errorLogFields.test.ts b/packages/trueforge-core/tests/core/util/errorLogFields.test.ts index 3ec584f88e..1dfcf26e72 100644 --- a/packages/trueforge-core/tests/core/util/errorLogFields.test.ts +++ b/packages/trueforge-core/tests/core/util/errorLogFields.test.ts @@ -54,12 +54,12 @@ describe('extractErrorLogFields', () => { }); describe('isReportableError', () => { - it('reports errors with no HTTP status', () => { + it.skip('reports errors with no HTTP status', () => { expect(isReportableError(new Error('boom'))).toBe(true); expect(isReportableError('plain string')).toBe(true); }); - it('skips statuses from 200 through 499 on the error or its cause', () => { + it.skip('skips statuses from 200 through 499 on the error or its cause', () => { expect(isReportableError(Object.assign(new Error('unauthorized'), { statusCode: 401 }))).toBe(false); expect(isReportableError(Object.assign(new Error('invalid grant'), { status: 400 }))).toBe(false); const wrapped = new Error('fetch failed', { @@ -68,7 +68,7 @@ describe('isReportableError', () => { expect(isReportableError(wrapped)).toBe(false); }); - it('reports statuses outside 200 through 499, using the outermost status', () => { + it.skip('reports statuses outside 200 through 499, using the outermost status', () => { expect(isReportableError(Object.assign(new Error('unavailable'), { statusCode: 503 }))).toBe(true); expect(isReportableError(Object.assign(new Error('bad gateway'), { status: 502 }))).toBe(true); expect( diff --git a/packages/trueforge/tests/unit/apis/turns.test.ts b/packages/trueforge/tests/unit/apis/turns.test.ts index 9c48ffa353..961cededf6 100644 --- a/packages/trueforge/tests/unit/apis/turns.test.ts +++ b/packages/trueforge/tests/unit/apis/turns.test.ts @@ -1,17 +1,14 @@ import { OpenAPIHono } from '@hono/zod-openapi'; import { AgentSpecSchema, - CancellationReason, - EventType, Sessions, TurnNotFoundError, - type TerminalTurnState, type TurnInputItem, type TurnStreamingEvent, } from '@truefoundry/trueforge-core/agent-session'; import type { Kysely } from 'kysely'; import { createLogger } from 'winston'; -import { createTurnsRouter, deriveSessionTitle, drainTurnEvents, turnStreamId } from '../../../src/apis/turns'; +import { createTurnsRouter, deriveSessionTitle, turnStreamId } from '../../../src/apis/turns'; import { TrueForgeAuthorizer, type Authorizer } from '../../../src/auth/authorizer'; import { STANDALONE_REQUEST_CONTEXT } from '../../../src/auth/identity'; import { McpServerWithAuthStore } from '../../../src/db/McpServerWithAuthStore'; @@ -29,7 +26,7 @@ import type { Database } from '../../../src/db/sqlite/types'; import { SqliteWebSearchProviderStore } from '../../../src/db/sqlite/web-search-provider-store/SqliteWebSearchProviderStore'; import { runWithLogContext } from '../../../src/logger'; import { ActiveTurnRegistry } from '../../../src/runtime/activeTurns'; -import { EventSubscriptionRegistry, type EventSubscription } from '../../../src/runtime/event-subscription/index.js'; +import { EventSubscriptionRegistry } from '../../../src/runtime/event-subscription/index.js'; function mcpServerStoreWithAuth(db: Kysely, tokenStore: SqliteOAuthTokenStore) { return new McpServerWithAuthStore({ @@ -93,100 +90,6 @@ describe('turns', () => { }); }); - describe('drainTurnEvents', () => { - const completedAt = '2026-01-01T00:00:00.000Z'; - - function turnDone(state: TerminalTurnState): TurnStreamingEvent { - return { - type: EventType.TURN_DONE, - id: 'evt-done', - created_at: completedAt, - thread_id: null, - state, - }; - } - - function subscription(): EventSubscription { - return { - put: async () => 1, - assertSubscribable: async () => undefined, - async *poll() { - /* drain does not read the subscription */ - }, - }; - } - - async function drain(event: TurnStreamingEvent): Promise { - const logger = createLogger({ silent: true }); - const errorLog = jest.spyOn(logger, 'error'); - const timer = setTimeout(() => undefined, 60_000); - await drainTurnEvents({ - trackedStream: (async function* () { - yield event; - })(), - turnEventStream: subscription(), - maxExecutionTimer: timer, - logger, - }); - return errorLog; - } - - function reportedError(errorLog: jest.SpyInstance): { message: unknown; error: unknown; reason: unknown } { - const [message, fields] = errorLog.mock.calls[0] ?? []; - const marked: unknown = - typeof fields === 'object' && fields !== null ? Reflect.get(fields, 'reportable_error') : undefined; - const error: unknown = typeof marked === 'object' && marked !== null ? Reflect.get(marked, 'error') : undefined; - const extra: unknown = typeof marked === 'object' && marked !== null ? Reflect.get(marked, 'extra') : undefined; - const reason: unknown = typeof extra === 'object' && extra !== null ? Reflect.get(extra, 'reason') : undefined; - return { message, error, reason }; - } - - it('logs turn.done error without reporting it, and reports abandoned and server-execution-timeout', async () => { - const errorLog = await drain(turnDone({ status: 'error', message: 'model failed', completed_at: completedAt })); - expect(errorLog).toHaveBeenCalledWith( - 'turn.done error', - expect.not.objectContaining({ reportable_error: expect.anything() }), - ); - - const abandonedLog = await drain( - turnDone({ status: 'cancelled', reason: CancellationReason.Abandoned, completed_at: completedAt }), - ); - expect(reportedError(abandonedLog)).toMatchObject({ - message: 'turn.done cancelled', - error: expect.objectContaining({ message: CancellationReason.Abandoned }), - reason: CancellationReason.Abandoned, - }); - - const timeoutLog = await drain( - turnDone({ - status: 'cancelled', - reason: CancellationReason.ServerExecutionTimeout, - completed_at: completedAt, - }), - ); - expect(reportedError(timeoutLog)).toMatchObject({ - message: 'turn.done cancelled', - error: expect.objectContaining({ message: CancellationReason.ServerExecutionTimeout }), - reason: CancellationReason.ServerExecutionTimeout, - }); - }); - - it('does not report client-cancelled, cancelled-for-next-turn, or a finished turn', async () => { - for (const event of [ - turnDone({ status: 'cancelled', reason: CancellationReason.ClientCancelled, completed_at: completedAt }), - turnDone({ - status: 'cancelled', - reason: CancellationReason.CancelledForNextTurn, - completed_at: completedAt, - }), - turnDone({ status: 'done', output: null, required_actions: [], completed_at: completedAt }), - ]) { - const errorLog = await drain(event); - expect(errorLog).not.toHaveBeenCalled(); - } - }); - }); - describe('turn ownership', () => { it('returns 403 for all turn routes when the caller is not the session creator', async () => { const db = createSqliteDb(':memory:'); diff --git a/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts b/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts index 09795abb79..cdf91b9867 100644 --- a/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts +++ b/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts @@ -1,5 +1,4 @@ -import { TrueForgeApi, TrueForgeError } from '@truefoundry/trueforge-sdk'; -import { createLogger } from 'winston'; +import { TrueForgeApi } from '@truefoundry/trueforge-sdk'; import { internalTrueForgeClient } from '../../../src/controller/internalTrueForgeClient'; import { dispatchSandboxEnvBuilds, sandboxEnvBuildLoop } from '../../../src/controller/sandboxEnvBuild'; @@ -46,54 +45,6 @@ describe('sandboxEnvBuildLoop', () => { }); }); - it('reports a progress failure once', async () => { - jest.spyOn(internalTrueForgeClient, 'listPendingSandboxEnvironmentVersions').mockResolvedValue(['ver-1']); - jest.spyOn(internalTrueForgeClient, 'progressSandboxEnvironmentVersion').mockRejectedValue(new Error('down')); - const logger = createLogger({ silent: true }); - const error = jest.spyOn(logger, 'error'); - - await dispatchSandboxEnvBuilds({ logger }); - - expect(error).toHaveBeenCalledWith( - 'Sandbox environment version progress failed', - expect.objectContaining({ - reportable_error: expect.objectContaining({ error: expect.any(Error) }), - }), - ); - }); - - it('does not report a progress failure the API already answered', async () => { - jest.spyOn(internalTrueForgeClient, 'listPendingSandboxEnvironmentVersions').mockResolvedValue(['ver-1']); - jest - .spyOn(internalTrueForgeClient, 'progressSandboxEnvironmentVersion') - .mockRejectedValue(new TrueForgeApi.InternalServerError({ error: { message: 'down' } })); - const logger = fakeLogger(); - - await dispatchSandboxEnvBuilds({ logger: logger as never }); - - expect(logger.error).toHaveBeenCalledWith( - 'Sandbox environment version progress failed', - expect.not.objectContaining({ reportable_error: expect.anything() }), - ); - }); - - it('reports a progress failure that never received a response', async () => { - jest.spyOn(internalTrueForgeClient, 'listPendingSandboxEnvironmentVersions').mockResolvedValue(['ver-1']); - jest - .spyOn(internalTrueForgeClient, 'progressSandboxEnvironmentVersion') - .mockRejectedValue(new TrueForgeError({ message: 'socket hang up' })); - const logger = fakeLogger(); - - await dispatchSandboxEnvBuilds({ logger: logger as never }); - - expect(logger.error).toHaveBeenCalledWith( - 'Sandbox environment version progress failed', - expect.objectContaining({ - reportable_error: expect.objectContaining({ error: expect.any(TrueForgeError) }), - }), - ); - }); - it('does not tick when aborted', async () => { const list = jest.spyOn(internalTrueForgeClient, 'listPendingSandboxEnvironmentVersions'); const controller = new AbortController(); diff --git a/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts b/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts index 4fd43932b2..4d1c2e13e9 100644 --- a/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts +++ b/packages/trueforge/tests/unit/sandbox/progressSandboxEnvironmentVersion.test.ts @@ -1,4 +1,3 @@ -import { DaytonaError } from '@daytona/sdk'; import { DAYTONA_SNAPSHOT_NOT_STARTED_REASON } from '@truefoundry/trueforge-core/core'; import { HTTPException } from 'hono/http-exception'; import { createLogger } from 'winston'; @@ -90,51 +89,6 @@ describe('progressSandboxEnvironmentVersion', () => { }); }); - it('rethrows a build failure without reporting it from this layer', async () => { - const { sandboxEnvironmentStore, sandboxProviderStore } = makeStores(); - mockProvider({ - getBuildStatus: jest.fn().mockRejectedValue(new Error('network down')), - build: jest.fn(), - }); - const errorLog = jest.spyOn(logger, 'error'); - - await expect( - progressSandboxEnvironmentVersion({ - sandboxEnvironmentStore, - sandboxProviderStore, - environment_version_id: 'ver-1', - logger, - }), - ).rejects.toThrow('network down'); - expect(errorLog).not.toHaveBeenCalled(); - }); - - it('logs an authz failure without reporting it and marks the version failed', async () => { - const { sandboxEnvironmentStore, sandboxProviderStore, envStore } = makeStores(); - const authError = new DaytonaError('Unauthorized', 401); - const warn = jest.spyOn(logger, 'warn'); - mockProvider({ - getBuildStatus: jest.fn().mockRejectedValue(authError), - build: jest.fn(), - }); - - await progressSandboxEnvironmentVersion({ - sandboxEnvironmentStore, - sandboxProviderStore, - environment_version_id: 'ver-1', - logger, - }); - - expect(envStore.markVersionFailed).toHaveBeenCalledWith({ - environment_version_id: 'ver-1', - status_reason: 'Sandbox provider rejected the API key — check the credentials', - }); - expect(warn).toHaveBeenCalledWith('Sandbox environment build failed authz', { - environment_version_id: 'ver-1', - status_reason: 'Sandbox provider rejected the API key — check the credentials', - }); - }); - it('marks ready when getBuildStatus reports ready', async () => { const { sandboxEnvironmentStore, sandboxProviderStore, envStore } = makeStores(); mockProvider({ diff --git a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts index e7b2433033..7c39dcc7eb 100644 --- a/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts +++ b/packages/trueforge/tests/unit/truefoundry/TrueFoundryAgentStore.test.ts @@ -369,7 +369,6 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ createAgent, updateAgent, deleteAgent }), client: mockClient({ deleteRemoteAgent }), }); - const errorLog = jest.spyOn(LOGGER, 'error'); await expect( store.createAgent( @@ -391,21 +390,6 @@ describe('TrueFoundryAgentStore', () => { expect.objectContaining({ message: 'local cleanup failed' }), ], }); - expect(errorLog).toHaveBeenCalledWith( - 'createAgent failed and cleanup also failed', - expect.objectContaining({ - agent_id: local.id, - agent_name: local.name, - external_id: 'sf-1', - reportable_error: expect.objectContaining({ - extra: expect.objectContaining({ - agent_id: local.id, - agent_name: local.name, - external_id: 'sf-1', - }), - }), - }), - ); }); it('updateAgent without manifest passes through to the inner store', async () => { @@ -602,7 +586,6 @@ describe('TrueFoundryAgentStore', () => { inner: mockInner({ getAgent, updateAgent }), client: mockClient({ putRemoteAgent }), }); - const errorLog = jest.spyOn(LOGGER, 'error'); await expect( store.updateAgent({ tenant_id: TENANT, id: previous.id, manifest: manifest({ instructions: 'Updated.' }) }), @@ -613,21 +596,6 @@ describe('TrueFoundryAgentStore', () => { expect.objectContaining({ message: 'sf restore failed' }), ], }); - expect(errorLog).toHaveBeenCalledWith( - 'updateAgent failed and ServiceFoundry restore also failed', - expect.objectContaining({ - agent_id: previous.id, - agent_name: previous.name, - external_id: previous.external_id, - reportable_error: expect.objectContaining({ - extra: expect.objectContaining({ - agent_id: previous.id, - agent_name: previous.name, - external_id: previous.external_id, - }), - }), - }), - ); }); it('deleteAgent reads the agent inside the transaction', async () => { From 087e268e87bca73358ed4ccd70f0f4806aeb546a Mon Sep 17 00:00:00 2001 From: Raman Tehlan Date: Sat, 24 Oct 2026 21:25:40 +0530 Subject: [PATCH 12/13] fix: remove test not required --- .../trueforge/tests/unit/controller/sandboxEnvBuild.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts b/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts index cdf91b9867..4cce5be44f 100644 --- a/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts +++ b/packages/trueforge/tests/unit/controller/sandboxEnvBuild.test.ts @@ -40,9 +40,7 @@ describe('sandboxEnvBuildLoop', () => { await dispatchSandboxEnvBuilds({ logger: logger as never }); expect(progress).toHaveBeenCalledTimes(2); - expect(logger.warn).toHaveBeenCalledWith('Sandbox environment version not found; skipping', { - environment_version_id: 'gone', - }); + expect(logger.warn).toHaveBeenCalled(); }); it('does not tick when aborted', async () => { From 3b88b6cff366c759f2dab2ac5d2a32c062c5d44b Mon Sep 17 00:00:00 2001 From: Raman Tehlan Date: Mon, 26 Oct 2026 03:58:45 +0530 Subject: [PATCH 13/13] remove skipped tests --- .../tests/core/llm/VercelAILLM.stream.test.ts | 38 ------------------- .../tests/core/util/errorLogFields.test.ts | 31 +-------------- 2 files changed, 1 insertion(+), 68 deletions(-) diff --git a/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts b/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts index 9fafd419e3..4d3b3e703f 100644 --- a/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts +++ b/packages/trueforge-core/tests/core/llm/VercelAILLM.stream.test.ts @@ -714,22 +714,6 @@ describe('isReportableError', () => { expect(isReportableError(new Error('model produced an invalid tool call'))).toBe(true); }); - it.skip('skips provider responses from 200 through 499', () => { - for (const statusCode of [200, 401, 429, 499]) { - expect( - isReportableError( - new APICallError({ - message: 'provider rejected the request', - url: 'https://example.com', - requestBodyValues: {}, - statusCode, - cause: Object.assign(new Error('connect ECONNRESET'), { code: 'ECONNRESET' }), - }), - ), - ).toBe(false); - } - }); - it('skips a stream error part that wraps a provider 4xx with a socket cause', async () => { const cause = new APICallError({ message: 'rate limit', @@ -751,26 +735,4 @@ describe('isReportableError', () => { } expect(isReportableError(thrown)).toBe(false); }); - - it.skip('reports a stream error part that wraps a provider 5xx', async () => { - const cause = new APICallError({ - message: 'provider unavailable', - url: 'https://example.com', - requestBodyValues: {}, - statusCode: 503, - cause: Object.assign(new Error('socket hang up'), { code: 'ECONNRESET' }), - }); - let thrown: unknown; - try { - await drainStream( - mapStreamToChunks({ - stream: makeStream([{ type: 'error', error: cause }]), - chunkMeta: CHUNK_META, - }), - ); - } catch (error) { - thrown = error; - } - expect(isReportableError(thrown)).toBe(true); - }); }); diff --git a/packages/trueforge-core/tests/core/util/errorLogFields.test.ts b/packages/trueforge-core/tests/core/util/errorLogFields.test.ts index 1dfcf26e72..2a9e89a730 100644 --- a/packages/trueforge-core/tests/core/util/errorLogFields.test.ts +++ b/packages/trueforge-core/tests/core/util/errorLogFields.test.ts @@ -1,4 +1,4 @@ -import { describeUnknownError, extractErrorLogFields, isReportableError } from '../../../src/core/util/errorLogFields'; +import { describeUnknownError, extractErrorLogFields } from '../../../src/core/util/errorLogFields'; describe('extractErrorLogFields', () => { it('returns the message and stack for a plain error', () => { @@ -53,35 +53,6 @@ describe('extractErrorLogFields', () => { }); }); -describe('isReportableError', () => { - it.skip('reports errors with no HTTP status', () => { - expect(isReportableError(new Error('boom'))).toBe(true); - expect(isReportableError('plain string')).toBe(true); - }); - - it.skip('skips statuses from 200 through 499 on the error or its cause', () => { - expect(isReportableError(Object.assign(new Error('unauthorized'), { statusCode: 401 }))).toBe(false); - expect(isReportableError(Object.assign(new Error('invalid grant'), { status: 400 }))).toBe(false); - const wrapped = new Error('fetch failed', { - cause: Object.assign(new Error('bad request'), { status: 400 }), - }); - expect(isReportableError(wrapped)).toBe(false); - }); - - it.skip('reports statuses outside 200 through 499, using the outermost status', () => { - expect(isReportableError(Object.assign(new Error('unavailable'), { statusCode: 503 }))).toBe(true); - expect(isReportableError(Object.assign(new Error('bad gateway'), { status: 502 }))).toBe(true); - expect( - isReportableError( - Object.assign(new Error('unavailable'), { - statusCode: 503, - cause: Object.assign(new Error('unauthorized'), { statusCode: 401 }), - }), - ), - ).toBe(true); - }); -}); - describe('describeUnknownError', () => { it('joins the cause chain', () => { const error = new Error('fetch failed', { cause: new Error('ECONNREFUSED') });