Skip to content

Commit e5feb08

Browse files
authored
fix(execution): end runs quietly when their workspace is deleted mid-run (#9144)
* fix(execution): end runs quietly when their workspace is deleted mid-run Deleting an account hard-deletes its solely-owned workspaces, and the cascade takes each in-flight run's workflow and execution log with it. A run caught in that window kept executing, then failed every terminal write: completion threw "Workflow log not found", the cost-only fallback failed the same way, run counts threw and logged twice, pause persistence logged errors, and the admission reservation leaked until its TTL. A run whose log insert failed on the deleted workspace kept executing blocks with no log row, so nothing could record or stop it. Along the way a file upload failure surfaced raw Drizzle SQL and bound parameters (user id, workspace id, storage key) in a block error. - ExecutionScopeDeletedError (no cause) marks the outcome; classifyFailure attributes it to the owner, so it logs at info. - completeWorkflowExecution, when the log row is missing, checks the run's workspace by primary key (inside the completion transaction when the row vanished mid-completion) and throws the typed error before any redaction, storage, or billing work. A missing row under a live workspace still throws "Workflow log not found" as before. - LoggingSession recognizes it in every completion path: info instead of error, no cost-only fallback, admission slot released, later completions and force-fails become no-ops, and getDeletedScope() reports it. - A log start that fails on a foreign key checks whether the workspace or workflow still exists; if not, safeStart returns false without the fallback, releases the slot, and executeWorkflowCore stops before building the executor. Any other start failure behaves exactly as before. - Run counts skip a deleted scope, and a zero-row update is an info no-op. - Pause persistence returns quietly for a deleted scope. - User-visible error text never carries a query failure's SQL: the execution upload wrapper keeps its cause and redacts its message, the block executor redacts any query failure in the cause chain, and the workspace file, file-output, video, image, TTS, ElevenLabs, LaTeX, and pause sites use a shared getUserFacingErrorMessage. * fix(execution): close review gaps in deleted-scope run ending - Rename the integration suite to scope-deleted.integration.ts so its name does not repeat its folder (check:file-names). - Type the new execution-core tests through ExecutionSnapshot and LoggingSession instead of `as any` (check:explicit-any). - Define FOREIGN_KEY_VIOLATION once in database-query-error and import it in the logger and the logging session. - Cover the guards the first commit left untested: a minimal fallback start that hits the deleted scope's foreign key, a cost-only fallback that is the first to find the workspace deleted, and pause persistence skipping a run whose deletion its paused completion found. - Give the csv-workflow-execution live test's LoggingSession stub the new getDeletedScope method. Correction to the previous commit message: pause persistence is quiet only when the paused completion itself found the workspace or workflow deleted. A deletion that lands after the paused log was written still fails persistPauseResult, which logs at error and force-fails the run, exactly as before. The TSDoc now says so. * test(logs): type the logged-message callbacks in the deleted-scope session tests * fix(execution): keep wrapped causes and quiet usage logging for deleted workspaces - The file-output wrapper in FileToolProcessor keeps the original failure as its cause, so diagnostics and the tool boundary still see a database failure underneath the redacted message. - recordExecutionUsage logs a missing workflow at info, not error, when the run's whole workspace is gone. The completion then reports the deleted run. The lookup runs only on that missing-workflow branch, both branches record nothing as before, and a failed lookup or a live workspace keeps the error. * refactor(execution): simplify deleted-scope handling after audit Fold the deleted-scope note and slot release into one session helper, reuse the admission-slot release on the normal completion path, share an ExecutionScope type, drop the unread executionId field and a redundant guard.
1 parent 7561e0f commit e5feb08

30 files changed

Lines changed: 924 additions & 55 deletions

‎apps/sim/executor/execution/block-executor.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,41 @@ describe('BlockExecutor', () => {
504504
expect(logFailureOnce(createLogger('OuterBoundary'), 'probe', thrown)).toBeUndefined()
505505
})
506506

507+
it('never surfaces a database failure that a wrapper interpolated into its own message', async () => {
508+
const block = createBlock()
509+
const workflow: SerializedWorkflow = {
510+
version: '1',
511+
blocks: [block],
512+
connections: [],
513+
loops: {},
514+
parallels: {},
515+
}
516+
const state = new ExecutionState()
517+
const resolver = new VariableResolver(workflow, {}, state)
518+
const handler: BlockHandler = { canHandle: () => true, execute: vi.fn() }
519+
const executor = new BlockExecutor([handler], resolver, {}, state)
520+
const ctx = createContext(state)
521+
const databaseError = new DrizzleQueryError(
522+
'insert into "workspace_files" ("user_id", "workspace_id") values ($1, $2)',
523+
['owner-secret-id', 'workspace-secret-id'],
524+
Object.assign(new Error('violates foreign key constraint'), { code: '23503' })
525+
)
526+
const wrapped = new Error(`Failed to upload file: ${databaseError.message}`, {
527+
cause: databaseError,
528+
})
529+
vi.mocked(validateBlockType).mockRejectedValueOnce(wrapped)
530+
const message = 'An internal error occurred while executing the block. Please try again.'
531+
532+
const thrown = await executor.execute(ctx, createNode(block), block).catch((error) => error)
533+
534+
expect(thrown.message).toBe(`Function: ${message}`)
535+
expect(state.getBlockOutput(block.id)).toEqual({ error: message })
536+
const surfaced = JSON.stringify([thrown.message, state.getBlockOutput(block.id), ctx.blockLogs])
537+
expect(surfaced).not.toContain('Failed query')
538+
expect(surfaced).not.toContain('owner-secret-id')
539+
expect(surfaced).not.toContain('workspace-secret-id')
540+
})
541+
507542
it('fires block completion callbacks for pausing blocks so clients receive pause output', async () => {
508543
const block = {
509544
...createBlock(),

‎apps/sim/executor/execution/block-executor.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { createLogger, type Logger } from '@sim/logger'
22
import { describeError, toError } from '@sim/utils/errors'
33
import { sleep } from '@sim/utils/helpers'
44
import { isRecordLike, toRecord } from '@sim/utils/object'
5-
import { DrizzleQueryError } from 'drizzle-orm/errors'
5+
import { findDatabaseQueryError } from '@/lib/core/errors/database-query-error'
66
import { logFailureOnce, markFailureKind, markFailureLogged } from '@/lib/core/errors/failure-log'
77
import { isTimeoutAbortReason } from '@/lib/core/execution-limits/types'
88
import { redactApiKeys } from '@/lib/core/security/redaction'
@@ -660,7 +660,8 @@ export class BlockExecutor {
660660
): Promise<NormalizedBlockOutput> {
661661
const endedAt = new Date().toISOString()
662662
const duration = performance.now() - startTime
663-
const isDatabaseError = error instanceof DrizzleQueryError
663+
/** A wrapper that interpolated a query failure's message carries its SQL and parameters too. */
664+
const isDatabaseError = findDatabaseQueryError(error) !== undefined
664665
const errorMessage = isDatabaseError ? INTERNAL_DATABASE_ERROR_MESSAGE : normalizeError(error)
665666
const hasLogInputs =
666667
inputsForLog && typeof inputsForLog === 'object' && Object.keys(inputsForLog).length > 0

‎apps/sim/executor/utils/file-tool-processor.test.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ import {
1212
uploadsMetadataMock,
1313
uploadsMetadataMockFns,
1414
} from '@sim/testing/mocks/uploads-metadata.mock'
15+
import { DrizzleQueryError } from 'drizzle-orm/errors'
1516
import { beforeEach, describe, expect, it, vi } from 'vitest'
17+
import { findDatabaseQueryError } from '@/lib/core/errors/database-query-error'
1618
import type { InternalToolOperationContext } from '@/lib/internal/tool-operations/types'
1719
import { MAX_FILE_SIZE } from '@/lib/uploads/utils/validation'
1820
import type { ExecutionContext, UserFile } from '@/executor/types'
@@ -194,6 +196,26 @@ describe('FileToolProcessor', () => {
194196
expect(mockUploadExecutionFile.mock.calls[0]?.[1]).toEqual(Buffer.from('Hello, world!'))
195197
})
196198

199+
it('redacts a database failure from its message but keeps it as the cause', async () => {
200+
const queryError = new DrizzleQueryError(
201+
'select "key" from "workspace_files" where "user_id" = $1',
202+
['owner-secret-id'],
203+
Object.assign(new Error('connection reset'), { code: 'ECONNRESET' })
204+
)
205+
mockDownloadFileFromUrl.mockRejectedValueOnce(queryError)
206+
207+
const thrown = await FileToolProcessor.processToolOutputs(
208+
{ file: { name: 'report.txt', mimeType: 'text/plain', url: 'https://example.com/report' } },
209+
toolConfig,
210+
executionContext
211+
).catch((error: unknown) => error)
212+
213+
expect(thrown).toBeInstanceOf(Error)
214+
expect((thrown as Error).message).toMatch(/^Failed to process file output 'file': /)
215+
expect((thrown as Error).message).not.toContain('owner-secret-id')
216+
expect(findDatabaseQueryError(thrown)).toBe(queryError)
217+
})
218+
197219
it.each([
198220
undefined,
199221
null,

‎apps/sim/executor/utils/file-tool-processor.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { createLogger } from '@sim/logger'
2-
import { toError } from '@sim/utils/errors'
32
import { isRecordLike, omit } from '@sim/utils/object'
43
import { isCanonicalBase64 } from '@/lib/api/contracts/primitives'
4+
import { getUserFacingErrorMessage } from '@/lib/core/errors/database-query-error'
55
import { isUserFile, type UserFileLike } from '@/lib/core/utils/user-file'
66
import {
77
createInternalToolFilesResult,
@@ -122,7 +122,10 @@ export class FileToolProcessor {
122122
} catch (error) {
123123
signal?.throwIfAborted()
124124
logger.error(`Error processing file output '${outputKey}':`, error)
125-
throw new Error(`Failed to process file output '${outputKey}': ${toError(error).message}`)
125+
throw new Error(
126+
`Failed to process file output '${outputKey}': ${getUserFacingErrorMessage(error)}`,
127+
{ cause: error }
128+
)
126129
}
127130
}
128131

‎apps/sim/lib/core/errors/database-query-error.ts‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
1-
import { findCause, getPostgresCancellationReason, getPostgresErrorCode } from '@sim/utils/errors'
1+
import {
2+
findCause,
3+
getErrorMessage,
4+
getPostgresCancellationReason,
5+
getPostgresErrorCode,
6+
} from '@sim/utils/errors'
27
import { DrizzleQueryError } from 'drizzle-orm/errors'
38

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

20+
/** Stands in for a database query failure's message on any surface a user can read. */
21+
const INTERNAL_DATABASE_ERROR_MESSAGE = 'An internal database error occurred'
22+
23+
/**
24+
* `error`'s message for a surface a user can read: {@link INTERNAL_DATABASE_ERROR_MESSAGE} when a
25+
* Drizzle query failure is anywhere in its cause chain, since a wrapper that interpolated the
26+
* query's message carries its SQL and bound parameters too; otherwise its own message, or
27+
* `fallback` when it has none.
28+
*/
29+
export function getUserFacingErrorMessage(error: unknown, fallback?: string): string {
30+
return findDatabaseQueryError(error)
31+
? INTERNAL_DATABASE_ERROR_MESSAGE
32+
: getErrorMessage(error, fallback)
33+
}
34+
1235
/**
1336
* Replaces a Drizzle query failure with an error naming only its database code, for surfaces such as
1437
* task runners that record a thrown error's message and stack verbatim. The original stays in
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/** The part of a run's ownership that can be hard-deleted while the run is in flight. */
2+
export type ExecutionScope = 'workspace' | 'workflow'
3+
4+
/**
5+
* A run's workspace or workflow was hard-deleted while it was in flight, so the run has nowhere
6+
* left to record its log, cost, or counts. An expected outcome of the owner deleting it, not a
7+
* failure to retry or page on. Carries no `cause`: `classifyFailure` would rank the database error
8+
* that revealed the deletion as internal.
9+
*/
10+
export class ExecutionScopeDeletedError extends Error {
11+
readonly scope: ExecutionScope
12+
13+
constructor(executionId: string, scope: ExecutionScope) {
14+
super(`The ${scope} of execution ${executionId} was deleted`)
15+
this.name = 'ExecutionScopeDeletedError'
16+
this.scope = scope
17+
}
18+
}

‎apps/sim/lib/core/errors/failure-log.test.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { createLogger } from '@sim/logger'
22
import { DrizzleQueryError } from 'drizzle-orm/errors'
33
import { describe, expect, it } from 'vitest'
4+
import { ExecutionScopeDeletedError } from '@/lib/core/errors/execution-scope-deleted'
45
import {
56
classifyFailure,
67
inheritFailureMarks,
@@ -21,6 +22,12 @@ function outerBoundary(error: unknown, executionId?: string) {
2122
}
2223

2324
describe('classifyFailure', () => {
25+
it('attributes a run whose workspace or workflow was deleted to its owner, not to Sim', () => {
26+
const deleted = new ExecutionScopeDeletedError('execution-1', 'workspace')
27+
expect(classifyFailure(deleted)).toBe('user')
28+
expect(classifyFailure(new Error('Execution failed', { cause: deleted }))).toBe('user')
29+
})
30+
2431
it('keeps a database failure internal even beneath a user mark', () => {
2532
const queryError = new DrizzleQueryError('select 1', [], new Error('connection reset'))
2633
const wrapped = markFailureKind(new Error('Block failed', { cause: queryError }), 'user')

‎apps/sim/lib/core/errors/failure-log.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { Logger } from '@sim/logger'
22
import { findDatabaseQueryError } from '@/lib/core/errors/database-query-error'
3+
import { ExecutionScopeDeletedError } from '@/lib/core/errors/execution-scope-deleted'
34
import { isRetryableSetupError } from '@/lib/core/errors/retryable-infrastructure'
45
import { UserFailure } from '@/lib/core/errors/user-failure'
56
import { HttpError } from '@/lib/core/utils/http-error'
@@ -63,9 +64,9 @@ export function markFailureKind<T>(error: T, kind: FailureKind): T {
6364

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

8184
if (link instanceof HttpError) {
8285
return link.statusCode >= 400 && link.statusCode < 500 ? 'user' : 'internal'

‎apps/sim/lib/internal/elevenlabs/execute-tool.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { createLogger } from '@sim/logger'
2-
import { getErrorMessage } from '@sim/utils/errors'
32
import type { z } from 'zod'
43
import { getValidationErrorMessage } from '@/lib/api/server'
54
import { DEFAULT_MAX_JSON_BODY_BYTES } from '@/lib/api/server/validation'
5+
import { getUserFacingErrorMessage } from '@/lib/core/errors/database-query-error'
66
import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
77
import { ElevenLabsOperationError } from '@/lib/internal/elevenlabs/errors'
88
import {
@@ -70,7 +70,7 @@ async function executeOperation<Input>(
7070
if (error instanceof ElevenLabsOperationError) {
7171
return Response.json(error.body, { status: error.status })
7272
}
73-
const message = getErrorMessage(error, 'Unknown error')
73+
const message = getUserFacingErrorMessage(error, 'Unknown error')
7474
logger.error('ElevenLabs operation failed', {
7575
error: message,
7676
requestId: request.requestId,

‎apps/sim/lib/internal/image/operations.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { createLogger } from '@sim/logger'
22
import { getErrorMessage } from '@sim/utils/errors'
33
import { interruptibleSleep } from '@sim/utils/helpers'
44
import { isRecordLike } from '@sim/utils/object'
5+
import { getUserFacingErrorMessage } from '@/lib/core/errors/database-query-error'
56
import { getMaxExecutionTimeout } from '@/lib/core/execution-limits'
67
import {
78
secureFetchWithPinnedIP,
@@ -123,7 +124,7 @@ export async function executeImageGeneration(
123124
} catch (error) {
124125
context.signal?.throwIfAborted()
125126
logger.error(`[${requestId}] Image generation operation error:`, error)
126-
const errorMessage = getErrorMessage(error, 'Unknown error')
127+
const errorMessage = getUserFacingErrorMessage(error, 'Unknown error')
127128
return Response.json(
128129
{ error: errorMessage },
129130
{ status: isPayloadSizeLimitError(error) ? 413 : 500 }

0 commit comments

Comments
 (0)