Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions apps/sim/executor/execution/block-executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,41 @@ describe('BlockExecutor', () => {
expect(logFailureOnce(createLogger('OuterBoundary'), 'probe', thrown)).toBeUndefined()
})

it('never surfaces a database failure that a wrapper interpolated into its own message', async () => {
const block = createBlock()
const workflow: SerializedWorkflow = {
version: '1',
blocks: [block],
connections: [],
loops: {},
parallels: {},
}
const state = new ExecutionState()
const resolver = new VariableResolver(workflow, {}, state)
const handler: BlockHandler = { canHandle: () => true, execute: vi.fn() }
const executor = new BlockExecutor([handler], resolver, {}, state)
const ctx = createContext(state)
const databaseError = new DrizzleQueryError(
'insert into "workspace_files" ("user_id", "workspace_id") values ($1, $2)',
['owner-secret-id', 'workspace-secret-id'],
Object.assign(new Error('violates foreign key constraint'), { code: '23503' })
)
const wrapped = new Error(`Failed to upload file: ${databaseError.message}`, {
cause: databaseError,
})
vi.mocked(validateBlockType).mockRejectedValueOnce(wrapped)
const message = 'An internal error occurred while executing the block. Please try again.'

const thrown = await executor.execute(ctx, createNode(block), block).catch((error) => error)

expect(thrown.message).toBe(`Function: ${message}`)
expect(state.getBlockOutput(block.id)).toEqual({ error: message })
const surfaced = JSON.stringify([thrown.message, state.getBlockOutput(block.id), ctx.blockLogs])
expect(surfaced).not.toContain('Failed query')
expect(surfaced).not.toContain('owner-secret-id')
expect(surfaced).not.toContain('workspace-secret-id')
})

it('fires block completion callbacks for pausing blocks so clients receive pause output', async () => {
const block = {
...createBlock(),
Expand Down
5 changes: 3 additions & 2 deletions apps/sim/executor/execution/block-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { createLogger, type Logger } from '@sim/logger'
import { describeError, toError } from '@sim/utils/errors'
import { sleep } from '@sim/utils/helpers'
import { isRecordLike, toRecord } from '@sim/utils/object'
import { DrizzleQueryError } from 'drizzle-orm/errors'
import { findDatabaseQueryError } from '@/lib/core/errors/database-query-error'
import { logFailureOnce, markFailureKind, markFailureLogged } from '@/lib/core/errors/failure-log'
import { isTimeoutAbortReason } from '@/lib/core/execution-limits/types'
import { redactApiKeys } from '@/lib/core/security/redaction'
Expand Down Expand Up @@ -660,7 +660,8 @@ export class BlockExecutor {
): Promise<NormalizedBlockOutput> {
const endedAt = new Date().toISOString()
const duration = performance.now() - startTime
const isDatabaseError = error instanceof DrizzleQueryError
/** A wrapper that interpolated a query failure's message carries its SQL and parameters too. */
const isDatabaseError = findDatabaseQueryError(error) !== undefined
const errorMessage = isDatabaseError ? INTERNAL_DATABASE_ERROR_MESSAGE : normalizeError(error)
const hasLogInputs =
inputsForLog && typeof inputsForLog === 'object' && Object.keys(inputsForLog).length > 0
Expand Down
22 changes: 22 additions & 0 deletions apps/sim/executor/utils/file-tool-processor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import {
uploadsMetadataMock,
uploadsMetadataMockFns,
} from '@sim/testing/mocks/uploads-metadata.mock'
import { DrizzleQueryError } from 'drizzle-orm/errors'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { findDatabaseQueryError } from '@/lib/core/errors/database-query-error'
import type { InternalToolOperationContext } from '@/lib/internal/tool-operations/types'
import { MAX_FILE_SIZE } from '@/lib/uploads/utils/validation'
import type { ExecutionContext, UserFile } from '@/executor/types'
Expand Down Expand Up @@ -194,6 +196,26 @@ describe('FileToolProcessor', () => {
expect(mockUploadExecutionFile.mock.calls[0]?.[1]).toEqual(Buffer.from('Hello, world!'))
})

it('redacts a database failure from its message but keeps it as the cause', async () => {
const queryError = new DrizzleQueryError(
'select "key" from "workspace_files" where "user_id" = $1',
['owner-secret-id'],
Object.assign(new Error('connection reset'), { code: 'ECONNRESET' })
)
mockDownloadFileFromUrl.mockRejectedValueOnce(queryError)

const thrown = await FileToolProcessor.processToolOutputs(
{ file: { name: 'report.txt', mimeType: 'text/plain', url: 'https://example.com/report' } },
toolConfig,
executionContext
).catch((error: unknown) => error)

expect(thrown).toBeInstanceOf(Error)
expect((thrown as Error).message).toMatch(/^Failed to process file output 'file': /)
expect((thrown as Error).message).not.toContain('owner-secret-id')
expect(findDatabaseQueryError(thrown)).toBe(queryError)
})

it.each([
undefined,
null,
Expand Down
7 changes: 5 additions & 2 deletions apps/sim/executor/utils/file-tool-processor.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { createLogger } from '@sim/logger'
import { toError } from '@sim/utils/errors'
import { isRecordLike, omit } from '@sim/utils/object'
import { isCanonicalBase64 } from '@/lib/api/contracts/primitives'
import { getUserFacingErrorMessage } from '@/lib/core/errors/database-query-error'
import { isUserFile, type UserFileLike } from '@/lib/core/utils/user-file'
import {
createInternalToolFilesResult,
Expand Down Expand Up @@ -122,7 +122,10 @@ export class FileToolProcessor {
} catch (error) {
signal?.throwIfAborted()
logger.error(`Error processing file output '${outputKey}':`, error)
throw new Error(`Failed to process file output '${outputKey}': ${toError(error).message}`)
throw new Error(
`Failed to process file output '${outputKey}': ${getUserFacingErrorMessage(error)}`,
{ cause: error }
)
}
}

Expand Down
25 changes: 24 additions & 1 deletion apps/sim/lib/core/errors/database-query-error.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
import { findCause, getPostgresCancellationReason, getPostgresErrorCode } from '@sim/utils/errors'
import {
findCause,
getErrorMessage,
getPostgresCancellationReason,
getPostgresErrorCode,
} from '@sim/utils/errors'
import { DrizzleQueryError } from 'drizzle-orm/errors'

/** PostgreSQL `foreign_key_violation`. */
export const FOREIGN_KEY_VIOLATION = '23503'

/**
* The Drizzle query failure anywhere in `error`'s cause chain. Its message carries the SQL text and
* bound parameters, so it must never reach a user; its `cause` holds the driver error and code.
Expand All @@ -9,6 +17,21 @@ export function findDatabaseQueryError(error: unknown): DrizzleQueryError | unde
return findCause(error, (cause): cause is DrizzleQueryError => cause instanceof DrizzleQueryError)
}

/** Stands in for a database query failure's message on any surface a user can read. */
const INTERNAL_DATABASE_ERROR_MESSAGE = 'An internal database error occurred'

/**
* `error`'s message for a surface a user can read: {@link INTERNAL_DATABASE_ERROR_MESSAGE} when a
* Drizzle query failure is anywhere in its cause chain, since a wrapper that interpolated the
* query's message carries its SQL and bound parameters too; otherwise its own message, or
* `fallback` when it has none.
*/
export function getUserFacingErrorMessage(error: unknown, fallback?: string): string {
return findDatabaseQueryError(error)
? INTERNAL_DATABASE_ERROR_MESSAGE
: getErrorMessage(error, fallback)
}

/**
* Replaces a Drizzle query failure with an error naming only its database code, for surfaces such as
* task runners that record a thrown error's message and stack verbatim. The original stays in
Expand Down
18 changes: 18 additions & 0 deletions apps/sim/lib/core/errors/execution-scope-deleted.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/** The part of a run's ownership that can be hard-deleted while the run is in flight. */
export type ExecutionScope = 'workspace' | 'workflow'

/**
* A run's workspace or workflow was hard-deleted while it was in flight, so the run has nowhere
* left to record its log, cost, or counts. An expected outcome of the owner deleting it, not a
* failure to retry or page on. Carries no `cause`: `classifyFailure` would rank the database error
* that revealed the deletion as internal.
*/
export class ExecutionScopeDeletedError extends Error {
readonly scope: ExecutionScope

constructor(executionId: string, scope: ExecutionScope) {
super(`The ${scope} of execution ${executionId} was deleted`)
this.name = 'ExecutionScopeDeletedError'
this.scope = scope
}
}
7 changes: 7 additions & 0 deletions apps/sim/lib/core/errors/failure-log.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { createLogger } from '@sim/logger'
import { DrizzleQueryError } from 'drizzle-orm/errors'
import { describe, expect, it } from 'vitest'
import { ExecutionScopeDeletedError } from '@/lib/core/errors/execution-scope-deleted'
import {
classifyFailure,
inheritFailureMarks,
Expand All @@ -21,6 +22,12 @@ function outerBoundary(error: unknown, executionId?: string) {
}

describe('classifyFailure', () => {
it('attributes a run whose workspace or workflow was deleted to its owner, not to Sim', () => {
const deleted = new ExecutionScopeDeletedError('execution-1', 'workspace')
expect(classifyFailure(deleted)).toBe('user')
expect(classifyFailure(new Error('Execution failed', { cause: deleted }))).toBe('user')
})

it('keeps a database failure internal even beneath a user mark', () => {
const queryError = new DrizzleQueryError('select 1', [], new Error('connection reset'))
const wrapped = markFailureKind(new Error('Block failed', { cause: queryError }), 'user')
Expand Down
9 changes: 6 additions & 3 deletions apps/sim/lib/core/errors/failure-log.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { Logger } from '@sim/logger'
import { findDatabaseQueryError } from '@/lib/core/errors/database-query-error'
import { ExecutionScopeDeletedError } from '@/lib/core/errors/execution-scope-deleted'
import { isRetryableSetupError } from '@/lib/core/errors/retryable-infrastructure'
import { UserFailure } from '@/lib/core/errors/user-failure'
import { HttpError } from '@/lib/core/utils/http-error'
Expand Down Expand Up @@ -63,9 +64,9 @@ export function markFailureKind<T>(error: T, kind: FailureKind): T {

/**
* Attributes `error` from its cause chain. A database or retryable setup failure is always
* internal, then the outermost link with an explicit mark, a {@link UserFailure} or revoked
* credential, a Sim `HttpError` status, or an upstream `status` decides. Anything unattributed is
* internal.
* internal, then the outermost link with an explicit mark, a {@link UserFailure}, revoked
* credential, or deleted run scope, a Sim `HttpError` status, or an upstream `status`
* decides. Anything unattributed is internal.
*/
export function classifyFailure(error: unknown): FailureKind {
if (findDatabaseQueryError(error)) return 'internal'
Expand All @@ -77,6 +78,8 @@ export function classifyFailure(error: unknown): FailureKind {
if (marked) return marked
/** Only the credential's owner reconnecting restores a revoked grant. */
if (link instanceof UserFailure || link instanceof CredentialRevokedError) return 'user'
/** The owner deleted the run's workspace or workflow; nothing is left for Sim to fix. */
if (link instanceof ExecutionScopeDeletedError) return 'user'

if (link instanceof HttpError) {
return link.statusCode >= 400 && link.statusCode < 500 ? 'user' : 'internal'
Expand Down
4 changes: 2 additions & 2 deletions apps/sim/lib/internal/elevenlabs/execute-tool.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { createLogger } from '@sim/logger'
import { getErrorMessage } from '@sim/utils/errors'
import type { z } from 'zod'
import { getValidationErrorMessage } from '@/lib/api/server'
import { DEFAULT_MAX_JSON_BODY_BYTES } from '@/lib/api/server/validation'
import { getUserFacingErrorMessage } from '@/lib/core/errors/database-query-error'
import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
import { ElevenLabsOperationError } from '@/lib/internal/elevenlabs/errors'
import {
Expand Down Expand Up @@ -70,7 +70,7 @@ async function executeOperation<Input>(
if (error instanceof ElevenLabsOperationError) {
return Response.json(error.body, { status: error.status })
}
const message = getErrorMessage(error, 'Unknown error')
const message = getUserFacingErrorMessage(error, 'Unknown error')
logger.error('ElevenLabs operation failed', {
error: message,
requestId: request.requestId,
Expand Down
3 changes: 2 additions & 1 deletion apps/sim/lib/internal/image/operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { createLogger } from '@sim/logger'
import { getErrorMessage } from '@sim/utils/errors'
import { interruptibleSleep } from '@sim/utils/helpers'
import { isRecordLike } from '@sim/utils/object'
import { getUserFacingErrorMessage } from '@/lib/core/errors/database-query-error'
import { getMaxExecutionTimeout } from '@/lib/core/execution-limits'
import {
secureFetchWithPinnedIP,
Expand Down Expand Up @@ -123,7 +124,7 @@ export async function executeImageGeneration(
} catch (error) {
context.signal?.throwIfAborted()
logger.error(`[${requestId}] Image generation operation error:`, error)
const errorMessage = getErrorMessage(error, 'Unknown error')
const errorMessage = getUserFacingErrorMessage(error, 'Unknown error')
return Response.json(
{ error: errorMessage },
{ status: isPayloadSizeLimitError(error) ? 413 : 500 }
Expand Down
7 changes: 5 additions & 2 deletions apps/sim/lib/internal/latex/execute-tool.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getErrorMessage } from '@sim/utils/errors'
import { getUserFacingErrorMessage } from '@/lib/core/errors/database-query-error'
import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
import { LatexOperationError } from '@/lib/internal/latex/errors'
import { compileLatexDocument } from '@/lib/internal/latex/operations'
Expand Down Expand Up @@ -36,6 +36,9 @@ export const executeLatexTool: InternalToolOperationHandler = async (request) =>
: error instanceof LatexOperationError
? error.status
: 500
return Response.json({ error: getErrorMessage(error, 'LaTeX compilation failed') }, { status })
return Response.json(
{ error: getUserFacingErrorMessage(error, 'LaTeX compilation failed') },
{ status }
)
}
}
7 changes: 5 additions & 2 deletions apps/sim/lib/internal/tts/execute-tool.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { getErrorMessage } from '@sim/utils/errors'
import { z } from 'zod'
import { getValidationErrorMessage } from '@/lib/api/server'
import { getUserFacingErrorMessage } from '@/lib/core/errors/database-query-error'
import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
import type {
InternalToolOperationCall,
Expand Down Expand Up @@ -175,7 +175,10 @@ async function executeOperation<I>(
if (error instanceof TtsOperationError) {
return Response.json(error.body, { status: error.status })
}
const message = getErrorMessage(error, legacy ? 'Unknown error' : 'TTS synthesis failed')
const message = getUserFacingErrorMessage(
error,
legacy ? 'Unknown error' : 'TTS synthesis failed'
)
return Response.json(
{ error: legacy ? `Internal Server Error: ${message}` : message },
{ status: isPayloadSizeLimitError(error) ? 413 : 500 }
Expand Down
4 changes: 2 additions & 2 deletions apps/sim/lib/internal/video/operations.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { createLogger } from '@sim/logger'
import { getErrorMessage } from '@sim/utils/errors'
import { getUserFacingErrorMessage } from '@/lib/core/errors/database-query-error'
import { getBaseUrl } from '@/lib/core/utils/urls'
import { validateOpaqueModelInputProvenance } from '@/lib/execution/model-input-provenance'
import {
Expand Down Expand Up @@ -126,7 +126,7 @@ async function storeVideo(
} catch (error) {
context.signal?.throwIfAborted()
throw new VideoOperationError(
`Failed to store video: ${getErrorMessage(error, 'Unknown error')}`,
`Failed to store video: ${getUserFacingErrorMessage(error, 'Unknown error')}`,
500
)
}
Expand Down
63 changes: 62 additions & 1 deletion apps/sim/lib/logs/execution/logger.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { usageLog, workflow, workflowExecutionLogs } from '@sim/db/schema'
import { usageLog, workflow, workflowExecutionLogs, workspace } from '@sim/db/schema'
import {
dbChainMockFns,
flattenMockConditions,
loggerMock,
queueTableRows,
resetDbChainMock,
} from '@sim/testing'
Expand Down Expand Up @@ -749,6 +750,66 @@ describe('ExecutionLogger', () => {
})
})

describe('recordExecutionUsage for a workflow that no longer exists', () => {
const executionLoggerLog =
loggerMock.createLogger.mock.results[
loggerMock.createLogger.mock.calls.findIndex(([name]) => name === 'ExecutionLogger')
]?.value
const costSummary = {
totalCost: 0.005,
totalInputCost: 0,
totalOutputCost: 0,
totalTokens: 0,
totalPromptTokens: 0,
totalCompletionTokens: 0,
baseExecutionCharge: 0.005,
models: {},
}

beforeEach(() => {
resetDbChainMock()
})

const record = () =>
(
new ExecutionLogger() as unknown as {
recordExecutionUsage: (...args: unknown[]) => Promise<unknown>
}
).recordExecutionUsage(
'workflow-1',
costSummary,
'api',
'exec-1',
'user-1',
undefined,
true,
'ws-1'
)

test('records nothing and logs at info when the whole workspace was deleted', async () => {
queueTableRows(workflow, [])
queueTableRows(workspace, [])

await expect(record()).resolves.toEqual({ recordedIncrement: 0, costTotalRefined: false })

expect(recordUsage).not.toHaveBeenCalled()
expect(executionLoggerLog?.error).not.toHaveBeenCalled()
expect(executionLoggerLog?.info).toHaveBeenCalledWith(
"Skipped usage recording: the run's workspace was deleted"
)
})

test('still logs an error when the workflow is missing from a live workspace', async () => {
queueTableRows(workflow, [])
queueTableRows(workspace, [{ id: 'ws-1' }])

await expect(record()).resolves.toEqual({ recordedIncrement: 0, costTotalRefined: false })

expect(recordUsage).not.toHaveBeenCalled()
expect(executionLoggerLog?.error).toHaveBeenCalledWith('Workflow not found for usage recording')
})
})

describe('recordExecutionUsage boundary-delta reconciliation', () => {
let logger: any

Expand Down
Loading
Loading