From 813fd4698214a7c1cea4a764c02fc631c37efafd Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 16 Oct 2026 14:50:10 -0700 Subject: [PATCH 1/6] 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. --- .../executor/execution/block-executor.test.ts | 35 ++++ apps/sim/executor/execution/block-executor.ts | 5 +- .../sim/executor/utils/file-tool-processor.ts | 6 +- .../lib/core/errors/database-query-error.ts | 22 ++- .../core/errors/execution-scope-deleted.ts | 17 ++ apps/sim/lib/core/errors/failure-log.test.ts | 7 + apps/sim/lib/core/errors/failure-log.ts | 7 +- .../lib/internal/elevenlabs/execute-tool.ts | 4 +- apps/sim/lib/internal/image/operations.ts | 3 +- apps/sim/lib/internal/latex/execute-tool.ts | 7 +- apps/sim/lib/internal/tts/execute-tool.ts | 7 +- apps/sim/lib/internal/video/operations.ts | 4 +- .../execution-scope-deleted.integration.ts | 174 ++++++++++++++++++ apps/sim/lib/logs/execution/logger.ts | 57 ++++++ .../logs/execution/logging-session.test.ts | 108 ++++++++++- .../sim/lib/logs/execution/logging-session.ts | 125 ++++++++++++- .../contexts/execution/file-manager.test.ts | 25 +++ .../contexts/execution/file-manager.ts | 9 +- .../contexts/workspace/file-manager.ts | 22 ++- .../workflows/executor/execution-core.test.ts | 69 +++++++ .../lib/workflows/executor/execution-core.ts | 21 ++- .../workflows/executor/pause-persistence.ts | 12 +- apps/sim/lib/workflows/utils.test.ts | 10 +- apps/sim/lib/workflows/utils.ts | 6 +- apps/sim/tools/index.ts | 7 +- .../testing/src/mocks/logging-session.mock.ts | 4 +- 26 files changed, 731 insertions(+), 42 deletions(-) create mode 100644 apps/sim/lib/core/errors/execution-scope-deleted.ts create mode 100644 apps/sim/lib/logs/execution/execution-scope-deleted.integration.ts diff --git a/apps/sim/executor/execution/block-executor.test.ts b/apps/sim/executor/execution/block-executor.test.ts index 90c158957ab..fa63c98e71e 100644 --- a/apps/sim/executor/execution/block-executor.test.ts +++ b/apps/sim/executor/execution/block-executor.test.ts @@ -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(), diff --git a/apps/sim/executor/execution/block-executor.ts b/apps/sim/executor/execution/block-executor.ts index 088118c3434..fdc51cbdb29 100644 --- a/apps/sim/executor/execution/block-executor.ts +++ b/apps/sim/executor/execution/block-executor.ts @@ -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' @@ -660,7 +660,8 @@ export class BlockExecutor { ): Promise { 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 diff --git a/apps/sim/executor/utils/file-tool-processor.ts b/apps/sim/executor/utils/file-tool-processor.ts index 23fbedb2ebd..738ad93298f 100644 --- a/apps/sim/executor/utils/file-tool-processor.ts +++ b/apps/sim/executor/utils/file-tool-processor.ts @@ -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, @@ -122,7 +122,9 @@ 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)}` + ) } } diff --git a/apps/sim/lib/core/errors/database-query-error.ts b/apps/sim/lib/core/errors/database-query-error.ts index 1db085f0031..bb6d1d77107 100644 --- a/apps/sim/lib/core/errors/database-query-error.ts +++ b/apps/sim/lib/core/errors/database-query-error.ts @@ -1,4 +1,9 @@ -import { findCause, getPostgresCancellationReason, getPostgresErrorCode } from '@sim/utils/errors' +import { + findCause, + getErrorMessage, + getPostgresCancellationReason, + getPostgresErrorCode, +} from '@sim/utils/errors' import { DrizzleQueryError } from 'drizzle-orm/errors' /** @@ -9,6 +14,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 diff --git a/apps/sim/lib/core/errors/execution-scope-deleted.ts b/apps/sim/lib/core/errors/execution-scope-deleted.ts new file mode 100644 index 00000000000..fe090882541 --- /dev/null +++ b/apps/sim/lib/core/errors/execution-scope-deleted.ts @@ -0,0 +1,17 @@ +/** + * 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 executionId: string + readonly scope: 'workspace' | 'workflow' + + constructor(executionId: string, scope: 'workspace' | 'workflow') { + super(`The ${scope} of execution ${executionId} was deleted`) + this.name = 'ExecutionScopeDeletedError' + this.executionId = executionId + this.scope = scope + } +} diff --git a/apps/sim/lib/core/errors/failure-log.test.ts b/apps/sim/lib/core/errors/failure-log.test.ts index 481b3b3ceb3..913cfb0df12 100644 --- a/apps/sim/lib/core/errors/failure-log.test.ts +++ b/apps/sim/lib/core/errors/failure-log.test.ts @@ -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, @@ -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') diff --git a/apps/sim/lib/core/errors/failure-log.ts b/apps/sim/lib/core/errors/failure-log.ts index 63a3b5d3cc1..bc4054d02d4 100644 --- a/apps/sim/lib/core/errors/failure-log.ts +++ b/apps/sim/lib/core/errors/failure-log.ts @@ -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' @@ -63,8 +64,8 @@ export function markFailureKind(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, 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 { @@ -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' diff --git a/apps/sim/lib/internal/elevenlabs/execute-tool.ts b/apps/sim/lib/internal/elevenlabs/execute-tool.ts index 13558ee091a..3a8cd3a77ed 100644 --- a/apps/sim/lib/internal/elevenlabs/execute-tool.ts +++ b/apps/sim/lib/internal/elevenlabs/execute-tool.ts @@ -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 { @@ -70,7 +70,7 @@ async function executeOperation( 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, diff --git a/apps/sim/lib/internal/image/operations.ts b/apps/sim/lib/internal/image/operations.ts index a1af8fd4b66..c3c7ba98f40 100644 --- a/apps/sim/lib/internal/image/operations.ts +++ b/apps/sim/lib/internal/image/operations.ts @@ -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, @@ -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 } diff --git a/apps/sim/lib/internal/latex/execute-tool.ts b/apps/sim/lib/internal/latex/execute-tool.ts index 3950a9f6251..82487dc8a2b 100644 --- a/apps/sim/lib/internal/latex/execute-tool.ts +++ b/apps/sim/lib/internal/latex/execute-tool.ts @@ -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' @@ -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 } + ) } } diff --git a/apps/sim/lib/internal/tts/execute-tool.ts b/apps/sim/lib/internal/tts/execute-tool.ts index d7c3f8d40d9..f00ad223421 100644 --- a/apps/sim/lib/internal/tts/execute-tool.ts +++ b/apps/sim/lib/internal/tts/execute-tool.ts @@ -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, @@ -175,7 +175,10 @@ async function executeOperation( 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 } diff --git a/apps/sim/lib/internal/video/operations.ts b/apps/sim/lib/internal/video/operations.ts index 9f6f7d72967..1c4675d3f08 100644 --- a/apps/sim/lib/internal/video/operations.ts +++ b/apps/sim/lib/internal/video/operations.ts @@ -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 { @@ -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 ) } diff --git a/apps/sim/lib/logs/execution/execution-scope-deleted.integration.ts b/apps/sim/lib/logs/execution/execution-scope-deleted.integration.ts new file mode 100644 index 00000000000..6ca4c8d7938 --- /dev/null +++ b/apps/sim/lib/logs/execution/execution-scope-deleted.integration.ts @@ -0,0 +1,174 @@ +/** + * A run whose workspace or workflow is hard-deleted while it runs, against real PostgreSQL: the + * session recognizes the deletion from the real foreign-key and missing-row outcomes, ends the + * run quietly, and still reports a log row lost under a live workspace as the failure it is. + */ +import { db } from '@sim/db' +import { + user, + workflow, + workflowExecutionLogs, + workflowExecutionSnapshots, + workspace, +} from '@sim/db/schema' +import { deleteWorkspaceFixture, insertWorkspaceFixture } from '@sim/db/testing/workspace-fixtures' +import { generateId } from '@sim/utils/id' +import { eq, inArray } from 'drizzle-orm' +import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { executionLogger } from '@/lib/logs/execution/logger' +import { calculateCostSummary } from '@/lib/logs/execution/logging-factory' +import { LoggingSession } from '@/lib/logs/execution/logging-session' +import type { WorkflowState } from '@/lib/logs/types' + +const ownerId = `scope-deleted-owner-${generateId()}` +const workspaceIds: string[] = [] +const snapshotIds: string[] = [] + +const workflowState: WorkflowState = { + blocks: { + start: { + id: 'start', + type: 'starter', + name: 'Start', + position: { x: 0, y: 0 }, + subBlocks: {}, + outputs: {}, + enabled: true, + }, + }, + edges: [], + loops: {}, + parallels: {}, +} + +/** A workspace holding one workflow, removed in `afterAll` unless a test deletes it first. */ +async function createScope() { + const scope = { workspaceId: generateId(), workflowId: generateId() } + workspaceIds.push(scope.workspaceId) + const now = new Date() + await insertWorkspaceFixture(db, { + id: scope.workspaceId, + name: 'Scope Deleted', + ownerId, + billedAccountUserId: ownerId, + }) + await db.insert(workflow).values({ + id: scope.workflowId, + userId: ownerId, + workspaceId: scope.workspaceId, + name: 'Scope Deleted', + lastSynced: now, + createdAt: now, + updatedAt: now, + }) + return scope +} + +function startSession(scope: { workspaceId: string; workflowId: string }, executionId: string) { + const session = new LoggingSession(scope.workflowId, executionId, 'api', 'scope-deleted') + return { + session, + started: session.safeStart({ userId: ownerId, workspaceId: scope.workspaceId, workflowState }), + } +} + +async function logRow(executionId: string) { + const [row] = await db + .select({ + status: workflowExecutionLogs.status, + stateSnapshotId: workflowExecutionLogs.stateSnapshotId, + }) + .from(workflowExecutionLogs) + .where(eq(workflowExecutionLogs.executionId, executionId)) + return row +} + +beforeAll(async () => { + const now = new Date() + await db.insert(user).values({ + id: ownerId, + name: 'Scope Deleted', + email: `${ownerId}@scope-deleted.test`, + emailVerified: true, + createdAt: now, + updatedAt: now, + }) +}) + +afterAll(async () => { + await deleteWorkspaceFixture(db, inArray(workspace.id, workspaceIds)) + /** Snapshots outlive their workflow (its workflow_id is set null), so remove them by id. */ + if (snapshotIds.length > 0) { + await db + .delete(workflowExecutionSnapshots) + .where(inArray(workflowExecutionSnapshots.id, snapshotIds)) + } + await db.delete(user).where(eq(user.id, ownerId)) +}) + +describe('a run whose scope is deleted mid-run', () => { + it('ends quietly when its workspace is deleted before completion', async () => { + const scope = await createScope() + const executionId = generateId() + const { session, started } = startSession(scope, executionId) + expect(await started).toBe(true) + const startedLog = await logRow(executionId) + snapshotIds.push(startedLog.stateSnapshotId) + + await deleteWorkspaceFixture(db, eq(workspace.id, scope.workspaceId)) + expect(await logRow(executionId)).toBeUndefined() + + await expect(session.safeComplete({ finalOutput: { ok: true } })).resolves.toBeUndefined() + + expect(session.getDeletedScope()).toBe('workspace') + expect(session.hasCompleted()).toBe(false) + expect(await logRow(executionId)).toBeUndefined() + }) + + it('still fails loudly when a log row is lost while its workspace exists', async () => { + const scope = await createScope() + + await expect( + executionLogger.completeWorkflowExecution({ + executionId: generateId(), + workspaceId: scope.workspaceId, + endedAt: new Date().toISOString(), + totalDurationMs: 5, + costSummary: calculateCostSummary([]), + finalOutput: {}, + traceSpans: [], + status: 'completed', + }) + ).rejects.toThrow(/^Workflow log not found for execution /) + }) + + it('does not start a run whose workspace is already deleted', async () => { + const scope = await createScope() + await deleteWorkspaceFixture(db, eq(workspace.id, scope.workspaceId)) + const executionId = generateId() + + const { session, started } = startSession(scope, executionId) + + expect(await started).toBe(false) + expect(session.getDeletedScope()).toBe('workspace') + expect(await logRow(executionId)).toBeUndefined() + }) + + it('does not start a run whose workflow is deleted from a live workspace', async () => { + const scope = await createScope() + await db.delete(workflow).where(eq(workflow.id, scope.workflowId)) + const executionId = generateId() + + const { session, started } = startSession(scope, executionId) + + expect(await started).toBe(false) + expect(session.getDeletedScope()).toBe('workflow') + expect(await logRow(executionId)).toBeUndefined() + }) + + it('reports no deleted scope while both still exist', async () => { + const scope = await createScope() + + await expect(executionLogger.findDeletedExecutionScope(scope)).resolves.toBeNull() + }) +}) diff --git a/apps/sim/lib/logs/execution/logger.ts b/apps/sim/lib/logs/execution/logger.ts index c355cf19314..f40b031c21e 100644 --- a/apps/sim/lib/logs/execution/logger.ts +++ b/apps/sim/lib/logs/execution/logger.ts @@ -34,6 +34,7 @@ import { import { resolveEffectivePiiRedaction } from '@/lib/billing/retention' import { checkAndBillPayerOverageThreshold } from '@/lib/billing/threshold-billing' import { isBillingEnabled } from '@/lib/core/config/env-flags' +import { ExecutionScopeDeletedError } from '@/lib/core/errors/execution-scope-deleted' import { redactApiKeys } from '@/lib/core/security/redaction' import { filterForDisplay } from '@/lib/core/utils/display-filters' import { acquireAdvisoryXactLock } from '@/lib/db/advisory-locks' @@ -301,6 +302,25 @@ async function setExecutionLogWriteTimeouts(trx: Pick): Pr ) } +/** + * Throws {@link ExecutionScopeDeletedError} when the run's workspace no longer exists. Deleting a + * workspace cascades its log rows away, so a missing log under a missing workspace is a run whose + * owner deleted it, not a lost row. + */ +async function assertExecutionWorkspaceExists( + client: Pick, + executionId: string, + workspaceId: string | null | undefined +): Promise { + if (!workspaceId) return + const [row] = await client + .select({ id: workspace.id }) + .from(workspace) + .where(eq(workspace.id, workspaceId)) + .limit(1) + if (!row) throw new ExecutionScopeDeletedError(executionId, 'workspace') +} + function countTraceSpans(traceSpans?: TraceSpan[]): number { if (!Array.isArray(traceSpans) || traceSpans.length === 0) { return 0 @@ -829,8 +849,36 @@ export class ExecutionLogger { return pii.traceSpans as TraceSpan[] } + /** + * Which of a run's scopes no longer exists, read by primary key: the workspace first, since + * deleting it also deletes the workflow. `null` when both still exist. + */ + async findDeletedExecutionScope(params: { + workspaceId: string + workflowId: string + }): Promise { + const [workspaceRow] = await execDb + .select({ id: workspace.id }) + .from(workspace) + .where(eq(workspace.id, params.workspaceId)) + .limit(1) + if (!workspaceRow) return 'workspace' + const [workflowRow] = await execDb + .select({ id: workflow.id }) + .from(workflow) + .where(eq(workflow.id, params.workflowId)) + .limit(1) + return workflowRow ? null : 'workflow' + } + + /** + * Writes the run's terminal log state. Throws {@link ExecutionScopeDeletedError} when the log + * row is gone because `workspaceId` was deleted, before any redaction, storage, or billing work. + */ async completeWorkflowExecution(params: { executionId: string + /** The run's workspace, checked only when its log row is missing. */ + workspaceId?: string endedAt: string totalDurationMs: number costSummary: { @@ -896,6 +944,8 @@ export class ExecutionLogger { workflowId: existingLog.workflowId ?? undefined, workspaceId: existingLog.workspaceId ?? undefined, }) + } else { + await assertExecutionWorkspaceExists(execDb, executionId, params.workspaceId) } const existingExecutionData = existingLog?.executionData as | WorkflowExecutionLog['executionData'] @@ -1252,6 +1302,13 @@ export class ExecutionLogger { if (competingTerminalStateWon) { return { updatedLog: currentLog, completionPersisted: false } } + if (!currentLog) { + await assertExecutionWorkspaceExists( + tx, + executionId, + existingLog?.workspaceId ?? params.workspaceId + ) + } throw new Error(`Workflow log not found for execution ${executionId}`) } diff --git a/apps/sim/lib/logs/execution/logging-session.test.ts b/apps/sim/lib/logs/execution/logging-session.test.ts index a7a904f25eb..bf3f0d74f19 100644 --- a/apps/sim/lib/logs/execution/logging-session.test.ts +++ b/apps/sim/lib/logs/execution/logging-session.test.ts @@ -1,5 +1,5 @@ import { workflowExecutionLogs } from '@sim/db/schema' -import { dbChainMockFns, resetDbChainMock } from '@sim/testing' +import { dbChainMockFns, loggerMock, resetDbChainMock } from '@sim/testing' import { billingUsageReservationMock, billingUsageReservationMockFns, @@ -20,12 +20,14 @@ const { prepareTraceSpansForProjectionMock, startWorkflowExecutionMock, loadWorkflowStateForExecutionMock, + findDeletedExecutionScopeMock, } = vi.hoisted(() => ({ completeWorkflowExecutionMock: vi.fn(), loadTraceSpansForProjectionMock: vi.fn(), prepareTraceSpansForProjectionMock: vi.fn(), startWorkflowExecutionMock: vi.fn(), loadWorkflowStateForExecutionMock: vi.fn(), + findDeletedExecutionScopeMock: vi.fn(), })) const { recordSecretUsageMock } = vi.hoisted(() => ({ recordSecretUsageMock: vi.fn() })) @@ -37,6 +39,7 @@ vi.mock('@/lib/logs/execution/logger', () => ({ completeWorkflowExecution: completeWorkflowExecutionMock, loadTraceSpansForProjection: loadTraceSpansForProjectionMock, prepareTraceSpansForProjection: prepareTraceSpansForProjectionMock, + findDeletedExecutionScope: findDeletedExecutionScopeMock, }, })) @@ -89,6 +92,7 @@ vi.mock('@/lib/logs/execution/logging-factory', () => ({ loadWorkflowStateForExecution: loadWorkflowStateForExecutionMock, })) +import { ExecutionScopeDeletedError } from '@/lib/core/errors/execution-scope-deleted' import { calculateCostSummary } from '@/lib/logs/execution/logging-factory' import { type ResolvedSecretTraceMatch, @@ -1167,6 +1171,108 @@ describe('completeWithError cancelled-status guard', () => { }) }) +describe('LoggingSession deleted run scope', () => { + const sessionLogger = + loggerMock.createLogger.mock.results[ + loggerMock.createLogger.mock.calls.findIndex(([name]) => name === 'LoggingSession') + ]?.value + const foreignKeyViolation = () => + Object.assign(new Error('violates foreign key constraint'), { code: '23503' }) + const startParams = { + workspaceId: 'workspace-1', + workflowState: { blocks: {}, edges: [], loops: {}, parallels: {} }, + } + + beforeEach(() => { + resetDbChainMock() + releaseExecutionSlotMock.mockResolvedValue(undefined) + findDeletedExecutionScopeMock.mockResolvedValue(null) + }) + + it.each([ + 'safeComplete', + 'safeCompleteWithError', + 'safeCompleteWithCancellation', + 'safeCompleteWithPause', + ] as const)( + '%s ends a run whose workspace was deleted without a cost-only retry or an error log', + async (completion) => { + const session = new LoggingSession('workflow-1', 'execution-d1', 'api', 'req-1') + completeWorkflowExecutionMock.mockRejectedValueOnce( + new ExecutionScopeDeletedError('execution-d1', 'workspace') + ) + + await expect(session[completion]()).resolves.toBeUndefined() + + expect(completeWorkflowExecutionMock).toHaveBeenCalledTimes(1) + expect(releaseExecutionSlotMock).toHaveBeenCalledWith('execution-d1') + expect(session.getDeletedScope()).toBe('workspace') + expect(session.hasCompleted()).toBe(false) + expect(sessionLogger?.error).not.toHaveBeenCalled() + + await session.safeCompleteWithError({ error: { message: 'late' } }) + await session.markAsFailed('late force-fail') + expect(completeWorkflowExecutionMock).toHaveBeenCalledTimes(1) + expect(dbChainMockFns.update).not.toHaveBeenCalled() + } + ) + + it('still takes the cost-only fallback for any other completion failure', async () => { + const session = new LoggingSession('workflow-1', 'execution-d2', 'api', 'req-1') + completeWorkflowExecutionMock + .mockRejectedValueOnce(new Error('Workflow log not found for execution execution-d2')) + .mockResolvedValueOnce({}) + + await session.safeComplete({ finalOutput: { ok: true } }) + + expect(completeWorkflowExecutionMock).toHaveBeenCalledTimes(2) + expect(session.getDeletedScope()).toBeNull() + expect(session.hasCompleted()).toBe(true) + }) + + it('stops starting a run whose workspace was deleted, and releases its slot', async () => { + const session = new LoggingSession('workflow-1', 'execution-d3', 'api', 'req-1') + startWorkflowExecutionMock.mockRejectedValue(foreignKeyViolation()) + findDeletedExecutionScopeMock.mockResolvedValue('workspace') + + await expect(session.safeStart(startParams)).resolves.toBe(false) + await expect(session.safeStart(startParams)).resolves.toBe(false) + + expect(startWorkflowExecutionMock).toHaveBeenCalledTimes(1) + expect(findDeletedExecutionScopeMock).toHaveBeenCalledWith({ + workspaceId: 'workspace-1', + workflowId: 'workflow-1', + }) + expect(session.getDeletedScope()).toBe('workspace') + expect(releaseExecutionSlotMock).toHaveBeenCalledWith('execution-d3') + expect(sessionLogger?.error).not.toHaveBeenCalled() + }) + + it('falls back to a minimal start when a foreign key fails but the scope still exists', async () => { + const session = new LoggingSession('workflow-1', 'execution-d4', 'api', 'req-1') + startWorkflowExecutionMock + .mockRejectedValueOnce(foreignKeyViolation()) + .mockResolvedValueOnce({}) + + await expect(session.safeStart(startParams)).resolves.toBe(true) + + expect(startWorkflowExecutionMock).toHaveBeenCalledTimes(2) + expect(session.getDeletedScope()).toBeNull() + expect(releaseExecutionSlotMock).not.toHaveBeenCalled() + }) + + it('never checks scope existence for a start failure other than a foreign key', async () => { + const session = new LoggingSession('workflow-1', 'execution-d5', 'api', 'req-1') + startWorkflowExecutionMock.mockRejectedValue(new Error('connection reset')) + + await expect(session.safeStart(startParams)).resolves.toBe(false) + + expect(startWorkflowExecutionMock).toHaveBeenCalledTimes(2) + expect(findDeletedExecutionScopeMock).not.toHaveBeenCalled() + expect(session.getDeletedScope()).toBeNull() + }) +}) + describe('LoggingSession.markExecutionAsFailed workflowId scoping', () => { beforeEach(() => { resetDbChainMock() diff --git a/apps/sim/lib/logs/execution/logging-session.ts b/apps/sim/lib/logs/execution/logging-session.ts index 69f7bd65e24..f254c53fbd2 100644 --- a/apps/sim/lib/logs/execution/logging-session.ts +++ b/apps/sim/lib/logs/execution/logging-session.ts @@ -1,10 +1,11 @@ import { dbFor } from '@sim/db' import { workflowExecutionLogs } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { describeError, toError } from '@sim/utils/errors' +import { describeError, getPostgresErrorCode, toError } from '@sim/utils/errors' import { and, eq, sql } from 'drizzle-orm' import { releaseExecutionSlot } from '@/lib/billing/calculations/usage-reservation' import type { BillingAttributionSnapshot } from '@/lib/billing/core/attribution' +import { ExecutionScopeDeletedError } from '@/lib/core/errors/execution-scope-deleted' import { isRetryableInfrastructureError } from '@/lib/core/errors/retryable-infrastructure' import { RESERVATION_TTL_BUFFER_MS } from '@/lib/core/execution-limits' import type { LargeValueStoreContext } from '@/lib/execution/payloads/store' @@ -125,6 +126,8 @@ function getActiveBlockDisplayProvenance( const logger = createLogger('LoggingSession') +const FOREIGN_KEY_VIOLATION = '23503' + type CompletionAttempt = 'complete' | 'error' | 'cancelled' | 'paused' export interface SecretSafeDisplayContent { @@ -225,6 +228,8 @@ export class LoggingSession { private executionDeadlineAt?: Date private persistedCompletionStatus: PersistedWorkflowExecutionStatus | null = null private lastCompletionError: string | null = null + /** Set once the run's workspace or workflow is found deleted; nothing is left to record. */ + private deletedScope: ExecutionScopeDeletedError['scope'] | null = null constructor( workflowId: string, @@ -702,6 +707,7 @@ export class LoggingSession { ) const completedLog = await executionLogger.completeWorkflowExecution({ executionId: this.executionId, + workspaceId: this.workspaceId, endedAt: params.endedAt, totalDurationMs: params.totalDurationMs, costSummary: params.costSummary, @@ -864,6 +870,8 @@ export class LoggingSession { ) } } catch (error) { + const deletedScope = skipLogCreation ? null : await this.findDeletedScope(error, workspaceId) + if (deletedScope) throw this.recordDeletedScope(deletedScope) if (this.requestId) { logger.error(`[${this.requestId}] Failed to start logging:`, error) } @@ -871,6 +879,66 @@ export class LoggingSession { } } + /** + * Which of the run's scopes was deleted, when a log insert failed on a foreign key. Any other + * failure, or a lookup that itself fails, reports nothing so the caller keeps its usual path. + */ + private async findDeletedScope( + error: unknown, + workspaceId: string + ): Promise { + if (getPostgresErrorCode(error) !== FOREIGN_KEY_VIOLATION) return null + try { + return await executionLogger.findDeletedExecutionScope({ + workspaceId, + workflowId: this.workflowId, + }) + } catch (lookupError) { + logger.warn(`Could not check whether execution ${this.executionId} lost its scope`, { + error: describeError(lookupError), + }) + return null + } + } + + private recordDeletedScope( + scope: ExecutionScopeDeletedError['scope'] + ): ExecutionScopeDeletedError { + const error = new ExecutionScopeDeletedError(this.executionId, scope) + this.noteDeletedScope(error) + return error + } + + /** Records a deleted scope once, at info: the owner deleted it, so there is nothing to fix. */ + private noteDeletedScope(error: ExecutionScopeDeletedError): void { + if (this.deletedScope) return + this.deletedScope = error.scope + logger.info(`Execution ${this.executionId} ended: its ${error.scope} was deleted`, { + requestId: this.requestId, + workflowId: this.workflowId, + workspaceId: this.workspaceId, + }) + } + + /** + * The run's scope found deleted while it ran, or `null`. Once set, the run has no log, cost, or + * count left to record, and every later completion is a no-op. + */ + getDeletedScope(): ExecutionScopeDeletedError['scope'] | null { + return this.deletedScope + } + + /** Ends a run whose scope was deleted: no log is left to write, so only its admission slot. */ + private async releaseSlotForDeletedScope(): Promise { + try { + await releaseExecutionSlot(this.reservationId) + } catch (error) { + logger.warn(`Failed to release admission reservation for ${this.executionId}:`, { + error: toError(error).message, + }) + } + } + async complete(params: SessionCompleteParams = {}): Promise { if (this.completed || this.completing) { return @@ -937,6 +1005,10 @@ export class LoggingSession { } } catch (error) { this.completing = false + if (error instanceof ExecutionScopeDeletedError) { + this.noteDeletedScope(error) + throw error + } logger.error(`Failed to complete logging for execution ${this.executionId}:`, { requestId: this.requestId, workflowId: this.workflowId, @@ -1071,6 +1143,10 @@ export class LoggingSession { } } catch (enhancedError) { this.completing = false + if (enhancedError instanceof ExecutionScopeDeletedError) { + this.noteDeletedScope(enhancedError) + throw enhancedError + } logger.error(`Failed to complete error logging for execution ${this.executionId}:`, { requestId: this.requestId, workflowId: this.workflowId, @@ -1152,6 +1228,10 @@ export class LoggingSession { } } catch (cancelError) { this.completing = false + if (cancelError instanceof ExecutionScopeDeletedError) { + this.noteDeletedScope(cancelError) + throw cancelError + } logger.error(`Failed to complete cancelled logging for execution ${this.executionId}:`, { requestId: this.requestId, workflowId: this.workflowId, @@ -1251,6 +1331,10 @@ export class LoggingSession { } } catch (pauseError) { this.completing = false + if (pauseError instanceof ExecutionScopeDeletedError) { + this.noteDeletedScope(pauseError) + throw pauseError + } logger.error(`Failed to complete paused logging for execution ${this.executionId}:`, { requestId: this.requestId, workflowId: this.workflowId, @@ -1262,11 +1346,21 @@ export class LoggingSession { } } + /** + * Starts the run's log, falling back to a minimal log when the full start fails. `false` when + * no log could be started; {@link getDeletedScope} then tells whether the run's workspace or + * workflow is gone, in which case its admission slot is already released. + */ async safeStart(params: SessionStartParams): Promise { + if (this.deletedScope) return false try { await this.start(params) return true } catch (error) { + if (error instanceof ExecutionScopeDeletedError) { + await this.releaseSlotForDeletedScope() + return false + } if (this.requestId) { logger.warn( `[${this.requestId}] Logging start failed - falling back to minimal session:`, @@ -1326,6 +1420,12 @@ export class LoggingSession { } return true } catch (fallbackError) { + const deletedScope = await this.findDeletedScope(fallbackError, params.workspaceId) + if (deletedScope) { + this.recordDeletedScope(deletedScope) + await this.releaseSlotForDeletedScope() + return false + } if (this.requestId) { logger.error(`[${this.requestId}] Minimal logging start also failed:`, fallbackError) } @@ -1382,6 +1482,7 @@ export class LoggingSession { if (this.completionPromise && !this.shouldStartNewCompletionAttempt(attempt)) { return this.completionPromise } + if (this.deletedScope) return Promise.resolve() this.completionAttempt = attempt this.completionAttemptFailed = false @@ -1417,6 +1518,10 @@ export class LoggingSession { await this.drainPendingProgressWrites() await this.complete(params) } catch (error) { + if (error instanceof ExecutionScopeDeletedError) { + await this.releaseSlotForDeletedScope() + return + } const errorMsg = toError(error).message logger.warn( `[${this.requestId || 'unknown'}] Complete failed for execution ${this.executionId}, attempting fallback`, @@ -1445,6 +1550,10 @@ export class LoggingSession { await this.drainPendingProgressWrites() await this.completeWithError(params) } catch (error) { + if (error instanceof ExecutionScopeDeletedError) { + await this.releaseSlotForDeletedScope() + return + } const errorMsg = toError(error).message logger.warn( `[${this.requestId || 'unknown'}] CompleteWithError failed for execution ${this.executionId}, attempting fallback`, @@ -1478,6 +1587,10 @@ export class LoggingSession { await this.drainPendingProgressWrites() await this.completeWithCancellation(params) } catch (error) { + if (error instanceof ExecutionScopeDeletedError) { + await this.releaseSlotForDeletedScope() + return + } const errorMsg = toError(error).message logger.warn( `[${this.requestId || 'unknown'}] CompleteWithCancellation failed for execution ${this.executionId}, attempting fallback`, @@ -1506,6 +1619,10 @@ export class LoggingSession { await this.drainPendingProgressWrites() await this.completeWithPause(params) } catch (error) { + if (error instanceof ExecutionScopeDeletedError) { + await this.releaseSlotForDeletedScope() + return + } const errorMsg = toError(error).message logger.warn( `[${this.requestId || 'unknown'}] CompleteWithPause failed for execution ${this.executionId}, attempting fallback`, @@ -1532,6 +1649,7 @@ export class LoggingSession { */ async markAsFailed(errorMessage?: string): Promise { await this.waitForCompletion() + if (this.deletedScope) return await this.drainPendingProgressWrites() await LoggingSession.markExecutionAsFailed( this.executionId, @@ -1667,6 +1785,11 @@ export class LoggingSession { ) } catch (fallbackError) { this.completing = false + if (fallbackError instanceof ExecutionScopeDeletedError) { + this.noteDeletedScope(fallbackError) + await this.releaseSlotForDeletedScope() + return + } this.completionAttemptFailed = true logger.error( `[${this.requestId || 'unknown'}] Cost-only fallback also failed for execution ${this.executionId}:`, diff --git a/apps/sim/lib/uploads/contexts/execution/file-manager.test.ts b/apps/sim/lib/uploads/contexts/execution/file-manager.test.ts index 695ba6a6dd4..697cc7bf821 100644 --- a/apps/sim/lib/uploads/contexts/execution/file-manager.test.ts +++ b/apps/sim/lib/uploads/contexts/execution/file-manager.test.ts @@ -5,6 +5,7 @@ import { uploadsConfigMockFns, } from '@sim/testing/mocks/uploads-config.mock' import { toRecord } from '@sim/utils/object' +import { DrizzleQueryError } from 'drizzle-orm/errors' import { beforeEach, describe, expect, it, vi } from 'vitest' const { mockUploadToS3, mockGetPresignedUrlWithConfig, mockDeleteFromS3 } = vi.hoisted(() => ({ @@ -21,6 +22,7 @@ vi.mock('@/lib/uploads/providers/s3/client', () => ({ deleteFromS3: mockDeleteFromS3, })) +import { findDatabaseQueryError } from '@/lib/core/errors/database-query-error' import { processExecutionFiles } from '@/lib/execution/files' import { uploadExecutionFile } from '@/lib/uploads/contexts/execution/file-manager' @@ -210,6 +212,29 @@ describe('uploadExecutionFile key allocation', () => { expect(mockGetPresignedUrlWithConfig).not.toHaveBeenCalled() }) + it('never puts the SQL or bound parameters of a failed metadata write in its error message', async () => { + const queryError = new DrizzleQueryError( + 'insert into "workspace_files" ("user_id", "workspace_id", "key") values ($1, $2, $3)', + ['owner-secret-id', 'workspace-1', 'execution/storage-key'], + Object.assign(new Error('violates foreign key constraint'), { code: '23503' }) + ) + dbChainMockFns.returning.mockRejectedValueOnce(queryError) + + const thrown = await uploadExecutionFile( + context, + Buffer.from('file'), + 'file.txt', + 'text/plain', + 'owner-secret-id' + ).catch((error: unknown) => error) + + expect(thrown).toBeInstanceOf(Error) + expect((thrown as Error).message).toMatch(/^Failed to upload file: /) + expect((thrown as Error).message).not.toContain('Failed query') + expect((thrown as Error).message).not.toContain('owner-secret-id') + expect(findDatabaseQueryError(thrown)).toBe(queryError) + }) + it('keeps the original upload error if cleanup also fails', async () => { mockGetPresignedUrlWithConfig.mockRejectedValueOnce(new Error('Presigning failed')) mockDeleteFromS3.mockRejectedValueOnce(new Error('Deletion failed')) diff --git a/apps/sim/lib/uploads/contexts/execution/file-manager.ts b/apps/sim/lib/uploads/contexts/execution/file-manager.ts index 9a5c677db23..92b208cb0f9 100644 --- a/apps/sim/lib/uploads/contexts/execution/file-manager.ts +++ b/apps/sim/lib/uploads/contexts/execution/file-manager.ts @@ -1,7 +1,8 @@ import { db } from '@sim/db' import { createLogger } from '@sim/logger' -import { getErrorMessage } from '@sim/utils/errors' +import { describeError, getErrorMessage } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' +import { getUserFacingErrorMessage } from '@/lib/core/errors/database-query-error' import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits' import { isUserFileWithMetadata } from '@/lib/core/utils/user-file' import type { ExecutionContext } from '@/lib/uploads/contexts/execution/utils' @@ -197,8 +198,10 @@ export async function uploadExecutionFile( }) } } - logger.error(`Failed to upload execution file ${fileName}:`, error) - throw new Error(`Failed to upload file: ${getErrorMessage(error, 'Unknown error')}`) + logger.error(`Failed to upload execution file ${fileName}:`, { cause: describeError(error) }) + throw new Error(`Failed to upload file: ${getUserFacingErrorMessage(error, 'Unknown error')}`, { + cause: error, + }) } } diff --git a/apps/sim/lib/uploads/contexts/workspace/file-manager.ts b/apps/sim/lib/uploads/contexts/workspace/file-manager.ts index b14ce6e3d62..fe1f9411f41 100644 --- a/apps/sim/lib/uploads/contexts/workspace/file-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/file-manager.ts @@ -51,6 +51,7 @@ import { type PreparedCollabDocState, saveCollabDocStateInTx, } from '@/lib/collab-doc/state' +import { getUserFacingErrorMessage } from '@/lib/core/errors/database-query-error' import { asOrchestrationError, OrchestrationError } from '@/lib/core/orchestration/types' import { generateRequestId } from '@/lib/core/utils/request' import { generateRestoreName } from '@/lib/core/utils/restore-name' @@ -877,9 +878,10 @@ export async function uploadWorkspaceFile( logger.error(`Failed to upload workspace file ${fileName}:`, { cause: describeError(error), }) - throw new Error(`Failed to upload file: ${getErrorMessage(error, 'Unknown error')}`, { - cause: error, - }) + throw new Error( + `Failed to upload file: ${getUserFacingErrorMessage(error, 'Unknown error')}`, + { cause: error } + ) } } @@ -2494,9 +2496,10 @@ export async function fetchWorkspaceFileBuffer( // transport failure to answer with their own placeholder, and re-wrapping it in a // plain Error would erase the only thing that tells the two apart. if (isPayloadSizeLimitError(error)) throw error - throw new Error(`Failed to download file: ${getErrorMessage(error, 'Unknown error')}`, { - cause: error, - }) + throw new Error( + `Failed to download file: ${getUserFacingErrorMessage(error, 'Unknown error')}`, + { cause: error } + ) } } @@ -2792,9 +2795,10 @@ export async function updateWorkspaceFileContent( const classified = asOrchestrationError(error) if (classified) throw classified logger.error(`Failed to update workspace file content ${fileId}:`, error) - throw new Error(`Failed to update file content: ${getErrorMessage(error, 'Unknown error')}`, { - cause: error, - }) + throw new Error( + `Failed to update file content: ${getUserFacingErrorMessage(error, 'Unknown error')}`, + { cause: error } + ) } } diff --git a/apps/sim/lib/workflows/executor/execution-core.test.ts b/apps/sim/lib/workflows/executor/execution-core.test.ts index e0f03e17d7b..603952df2f3 100644 --- a/apps/sim/lib/workflows/executor/execution-core.test.ts +++ b/apps/sim/lib/workflows/executor/execution-core.test.ts @@ -32,6 +32,7 @@ import { } from '@sim/testing/mocks/uploads-metadata.mock' import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import * as retention from '@/lib/billing/retention' +import { ExecutionScopeDeletedError } from '@/lib/core/errors/execution-scope-deleted' import { clearLargeValueCacheForTests } from '@/lib/execution/payloads/cache' import type { LargeArrayManifest } from '@/lib/execution/payloads/large-array-manifest' import type { LargeValueRef } from '@/lib/execution/payloads/large-value-ref' @@ -48,6 +49,7 @@ const { safeCompleteWithPauseMock, hasCompletedMock, getPersistedCompletionStatusMock, + getDeletedScopeMock, clearExecutionCancellationMock, buildTraceSpansMock, serializeWorkflowMock, @@ -69,6 +71,7 @@ const { safeCompleteWithPauseMock: vi.fn(), hasCompletedMock: vi.fn(), getPersistedCompletionStatusMock: vi.fn(), + getDeletedScopeMock: vi.fn(), clearExecutionCancellationMock: vi.fn(), buildTraceSpansMock: vi.fn(), serializeWorkflowMock: vi.fn(), @@ -191,6 +194,7 @@ describe('executeWorkflowCore terminal finalization sequencing', () => { safeCompleteWithPause: safeCompleteWithPauseMock, hasCompleted: hasCompletedMock, getPersistedCompletionStatus: getPersistedCompletionStatusMock, + getDeletedScope: getDeletedScopeMock, onBlockStart: onBlockStartPersistenceMock, onBlockComplete: vi.fn(), projectDisplayContent: projectDisplayContentMock, @@ -283,6 +287,7 @@ describe('executeWorkflowCore terminal finalization sequencing', () => { safeCompleteWithPauseMock.mockResolvedValue(undefined) hasCompletedMock.mockReturnValue(true) getPersistedCompletionStatusMock.mockReturnValue('pending') + getDeletedScopeMock.mockReturnValue(null) onBlockStartPersistenceMock.mockResolvedValue(undefined) projectDisplayContentMock.mockImplementation(async (content) => content) projectDiagnosticErrorMock.mockImplementation( @@ -1534,6 +1539,70 @@ describe('executeWorkflowCore terminal finalization sequencing', () => { expect(updateWorkflowRunCountsMock).toHaveBeenCalledWith('workflow-1') }) + it('stops before running any block when the start finds the workspace deleted', async () => { + safeStartMock.mockResolvedValue(false) + getDeletedScopeMock.mockReturnValue('workspace') + + const thrown = await executeWorkflowCore({ + snapshot: createSnapshot() as any, + callbacks: {}, + loggingSession: loggingSession as any, + }).catch((error: unknown) => error) + await loggingSession.setPostExecutionPromise.mock.calls[0]?.[0] + + expect(thrown).toBeInstanceOf(ExecutionScopeDeletedError) + expect(executorConstructorMock).not.toHaveBeenCalled() + expect(executorExecuteMock).not.toHaveBeenCalled() + expect(safeCompleteWithErrorMock).not.toHaveBeenCalled() + expect(updateWorkflowRunCountsMock).not.toHaveBeenCalled() + }) + + it('still runs the workflow when its log start fails for any other reason', async () => { + safeStartMock.mockResolvedValue(false) + executorExecuteMock.mockResolvedValue({ + success: true, + status: 'completed', + output: {}, + logs: [], + metadata: { duration: 123, startTime: 'start', endTime: 'end' }, + }) + + const result = await executeWorkflowCore({ + snapshot: createSnapshot() as any, + callbacks: {}, + loggingSession: loggingSession as any, + }) + await loggingSession.setPostExecutionPromise.mock.calls[0]?.[0] + + expect(result.status).toBe('completed') + expect(executorExecuteMock).toHaveBeenCalledTimes(1) + expect(updateWorkflowRunCountsMock).toHaveBeenCalledWith('workflow-1') + }) + + it('skips the run count of a run whose workspace was deleted while it ran', async () => { + executorExecuteMock.mockResolvedValue({ + success: true, + status: 'completed', + output: {}, + logs: [], + metadata: { duration: 123, startTime: 'start', endTime: 'end' }, + }) + safeCompleteMock.mockImplementation(async () => { + getDeletedScopeMock.mockReturnValue('workspace') + }) + hasCompletedMock.mockReturnValue(false) + + await executeWorkflowCore({ + snapshot: createSnapshot() as any, + callbacks: {}, + loggingSession: loggingSession as any, + }) + await loggingSession.setPostExecutionPromise.mock.calls[0]?.[0] + + expect(safeCompleteMock).toHaveBeenCalledTimes(1) + expect(updateWorkflowRunCountsMock).not.toHaveBeenCalled() + }) + it('routes paused executions through safeCompleteWithPause', async () => { const executionState = { blockStates: { 'function-1': { output: { result: 'raw-secret-value' } } }, diff --git a/apps/sim/lib/workflows/executor/execution-core.ts b/apps/sim/lib/workflows/executor/execution-core.ts index d6722ca5394..254978c40e2 100644 --- a/apps/sim/lib/workflows/executor/execution-core.ts +++ b/apps/sim/lib/workflows/executor/execution-core.ts @@ -14,6 +14,7 @@ import type { Edge } from '@xyflow/react' import { eq } from 'drizzle-orm' import { z } from 'zod' import { type EffectivePiiRedaction, resolveEffectivePiiRedaction } from '@/lib/billing/retention' +import { ExecutionScopeDeletedError } from '@/lib/core/errors/execution-scope-deleted' import { logFailureOnce } from '@/lib/core/errors/failure-log' import { UserFailure } from '@/lib/core/errors/user-failure' import { @@ -311,9 +312,15 @@ export function wasExecutionFinalizedByCore(error: unknown, executionId?: string * Counts a settled run — completed, failed, or cancelled alike — on the * workflow and stamps `lastRunAt`. Paused runs are not settled and are counted * when they finish. Awaited from the finalization path rather than fired and - * forgotten, so a process reload after the log write cannot drop it. + * forgotten, so a process reload after the log write cannot drop it. A run + * whose workspace or workflow was deleted has no row left to count on. */ -async function recordSettledRun(workflowId: string, requestId: string): Promise { +async function recordSettledRun( + workflowId: string, + requestId: string, + loggingSession: LoggingSession +): Promise { + if (loggingSession.getDeletedScope()) return try { await updateWorkflowRunCounts(workflowId) } catch (error) { @@ -418,7 +425,7 @@ async function finalizeExecutionOutcome(params: { } // Every non-paused outcome above is a settled run; the paused branch returned. - await recordSettledRun(workflowId, requestId) + await recordSettledRun(workflowId, requestId, loggingSession) } async function finalizeExecutionError(params: { @@ -459,7 +466,7 @@ async function finalizeExecutionError(params: { } // A run that threw after starting is a failed run, and failed runs count. - await recordSettledRun(workflowId, requestId) + await recordSettledRun(workflowId, requestId, loggingSession) return finalized } @@ -842,6 +849,12 @@ async function executeWorkflowCoreImpl( deploymentVersionId, workflowState: { blocks, edges, loops, parallels }, }) + /** + * A start that failed because the run's workspace or workflow is gone ends the run here, + * before any block spends on providers: nothing could ever record it or stop it. + */ + const deletedScope = loggingStarted ? null : loggingSession.getDeletedScope() + if (deletedScope) throw new ExecutionScopeDeletedError(executionId, deletedScope) // Use edges directly - trigger-to-trigger edges are prevented at creation time const filteredEdges = edges diff --git a/apps/sim/lib/workflows/executor/pause-persistence.ts b/apps/sim/lib/workflows/executor/pause-persistence.ts index f7a83da52c4..357de6b30b0 100644 --- a/apps/sim/lib/workflows/executor/pause-persistence.ts +++ b/apps/sim/lib/workflows/executor/pause-persistence.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import { describeError, toError } from '@sim/utils/errors' +import { getUserFacingErrorMessage } from '@/lib/core/errors/database-query-error' import { isRetryableInfrastructureError } from '@/lib/core/errors/retryable-infrastructure' import type { LoggingSession } from '@/lib/logs/execution/logging-session' import { PauseResumeManager } from '@/lib/workflows/executor/human-in-the-loop-manager' @@ -21,6 +22,7 @@ interface HandlePostExecutionPauseStateArgs { * Every caller of `executeWorkflowCore` must call this after execution completes * to ensure HITL pause state is persisted to the database and queued resumes are drained. * + * - If execution is paused but its workspace or workflow was deleted: nothing is left to persist * - If execution is paused but its log was never finalized: marks execution as failed * - If execution is paused without a snapshot: marks execution as failed * - If execution is paused with a valid snapshot: persists to `paused_executions` table @@ -41,6 +43,14 @@ export async function handlePostExecutionPauseState({ if (result.status === 'paused') { await loggingSession.waitForPostExecution() if (!loggingSession.hasCompleted()) { + const deletedScope = loggingSession.getDeletedScope() + if (deletedScope) { + logger.info('Skipped persisting pause state: its scope was deleted', { + executionId, + deletedScope, + }) + return + } logger.error('Paused execution log was not finalized', { executionId }) await loggingSession.markAsFailed('Failed to record paused execution') } else if (!result.snapshotSeed) { @@ -64,7 +74,7 @@ export async function handlePostExecutionPauseState({ retryable: isRetryableInfrastructureError(pauseError), }) await loggingSession.markAsFailed( - `Failed to persist pause state: ${toError(pauseError).message}` + `Failed to persist pause state: ${getUserFacingErrorMessage(pauseError)}` ) } } diff --git a/apps/sim/lib/workflows/utils.test.ts b/apps/sim/lib/workflows/utils.test.ts index b546f197179..d4438810760 100644 --- a/apps/sim/lib/workflows/utils.test.ts +++ b/apps/sim/lib/workflows/utils.test.ts @@ -221,9 +221,15 @@ describe('updateWorkflowRunCounts', () => { ) }) - it('rejects when no workflow row was updated', async () => { + it('resolves null without throwing when the workflow was deleted before its run settled', async () => { dbChainMockFns.returning.mockResolvedValueOnce([]) - await expect(updateWorkflowRunCounts('missing')).rejects.toThrow('Workflow missing not found') + await expect(updateWorkflowRunCounts('missing')).resolves.toBeNull() + }) + + it('rejects when the update itself fails', async () => { + dbChainMockFns.returning.mockRejectedValueOnce(new Error('connection reset')) + + await expect(updateWorkflowRunCounts('wf-1')).rejects.toThrow('connection reset') }) }) diff --git a/apps/sim/lib/workflows/utils.ts b/apps/sim/lib/workflows/utils.ts index badbecda842..692403d0bc7 100644 --- a/apps/sim/lib/workflows/utils.ts +++ b/apps/sim/lib/workflows/utils.ts @@ -249,6 +249,8 @@ export async function resolveWorkflowIdForUser( * Adds settled runs to a workflow's `runCount` and stamps `lastRunAt`. The * increment happens in SQL: concurrent runs of one workflow settle at the same * time, and a read-then-write would let one overwrite the other's count. + * Resolves `null` when the workflow was deleted while its run settled, which + * leaves nothing to count. */ export async function updateWorkflowRunCounts(workflowId: string, runs = 1) { try { @@ -261,8 +263,8 @@ export async function updateWorkflowRunCounts(workflowId: string, runs = 1) { .where(eq(workflowTable.id, workflowId)) .returning({ runCount: workflowTable.runCount }) if (!updated) { - logger.error(`Workflow ${workflowId} not found`) - throw new Error(`Workflow ${workflowId} not found`) + logger.info(`Skipped run count for deleted workflow ${workflowId}`) + return null } return { diff --git a/apps/sim/tools/index.ts b/apps/sim/tools/index.ts index 4d072998a0a..3bd0881c154 100644 --- a/apps/sim/tools/index.ts +++ b/apps/sim/tools/index.ts @@ -16,7 +16,10 @@ import { serializeBillingAttributionHeader, } from '@/lib/billing/core/attribution' import { isHosted } from '@/lib/core/config/env-flags' -import { findDatabaseQueryError } from '@/lib/core/errors/database-query-error' +import { + findDatabaseQueryError, + getUserFacingErrorMessage, +} from '@/lib/core/errors/database-query-error' import { classifyFailure, logFailureOnce, @@ -1265,7 +1268,7 @@ async function processFileOutputs( return { ...result, success: false, - error: `Failed to store file outputs for ${tool.id}: ${normalizedError.message}`, + error: `Failed to store file outputs for ${tool.id}: ${getUserFacingErrorMessage(error)}`, output: {}, } } diff --git a/packages/testing/src/mocks/logging-session.mock.ts b/packages/testing/src/mocks/logging-session.mock.ts index 7f455707cc2..a385d6b5b7f 100644 --- a/packages/testing/src/mocks/logging-session.mock.ts +++ b/packages/testing/src/mocks/logging-session.mock.ts @@ -7,7 +7,7 @@ import { vi } from 'vitest' * mock state. `mockSafeStart` defaults to `true` because callers branch on the * boolean result. Display projection methods (incl. `projectTraceSpansForLiveDisplay`) return * their input, diagnostic projection fails closed to structural metadata, `hasCompleted` is - * `false`, `getPersistedCompletionStatus` is `null`, and other methods resolve to `undefined`. + * `false`, `getPersistedCompletionStatus` and `getDeletedScope` are `null`, and other methods resolve to `undefined`. * Defaults are `vi.fn(impl)`, so `mockReset()` restores them. * * @example @@ -41,6 +41,7 @@ export const loggingSessionMockFns = { mockOnBlockComplete: vi.fn(async (..._args: unknown[]): Promise => {}), mockHasCompleted: vi.fn((): boolean => false), mockGetPersistedCompletionStatus: vi.fn((): string | null => null), + mockGetDeletedScope: vi.fn((): 'workspace' | 'workflow' | null => null), mockProjectTraceSpansForLiveDisplay: vi.fn(async (traceSpans: unknown) => traceSpans), mockMarkExecutionAsFailed: vi.fn(async (..._args: unknown[]): Promise => {}), mockProjectBlockLogsForDisplay: vi.fn(async (logs: unknown) => logs), @@ -82,6 +83,7 @@ function buildLoggingSessionInstance() { onBlockComplete: loggingSessionMockFns.mockOnBlockComplete, hasCompleted: loggingSessionMockFns.mockHasCompleted, getPersistedCompletionStatus: loggingSessionMockFns.mockGetPersistedCompletionStatus, + getDeletedScope: loggingSessionMockFns.mockGetDeletedScope, projectTraceSpansForLiveDisplay: loggingSessionMockFns.mockProjectTraceSpansForLiveDisplay, exportResolvedSecretTraceProvenanceForValue: loggingSessionMockFns.mockExportResolvedSecretTraceProvenanceForValue, From 82f6410f0cbe03da1379f8a7f12f9d8789a76336 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 16 Oct 2026 15:03:23 -0700 Subject: [PATCH 2/6] 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. --- .../lib/core/errors/database-query-error.ts | 3 + apps/sim/lib/core/errors/failure-log.ts | 4 +- apps/sim/lib/logs/execution/logger.ts | 2 +- .../logs/execution/logging-session.test.ts | 31 +++++++++++ .../sim/lib/logs/execution/logging-session.ts | 3 +- ...ration.ts => scope-deleted.integration.ts} | 0 .../csv-workflow-execution.live.test.ts | 3 + .../workflows/executor/execution-core.test.ts | 12 ++-- .../executor/pause-persistence.test.ts | 55 +++++++++++++++++++ .../workflows/executor/pause-persistence.ts | 4 +- 10 files changed, 105 insertions(+), 12 deletions(-) rename apps/sim/lib/logs/execution/{execution-scope-deleted.integration.ts => scope-deleted.integration.ts} (100%) create mode 100644 apps/sim/lib/workflows/executor/pause-persistence.test.ts diff --git a/apps/sim/lib/core/errors/database-query-error.ts b/apps/sim/lib/core/errors/database-query-error.ts index bb6d1d77107..68f0ce1f582 100644 --- a/apps/sim/lib/core/errors/database-query-error.ts +++ b/apps/sim/lib/core/errors/database-query-error.ts @@ -6,6 +6,9 @@ import { } 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. diff --git a/apps/sim/lib/core/errors/failure-log.ts b/apps/sim/lib/core/errors/failure-log.ts index bc4054d02d4..f252d93ca8a 100644 --- a/apps/sim/lib/core/errors/failure-log.ts +++ b/apps/sim/lib/core/errors/failure-log.ts @@ -65,8 +65,8 @@ export function markFailureKind(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}, revoked - * credential, or deleted run scope, a Sim `HttpError` status, or an upstream `status` decides. Anything unattributed is - * internal. + * 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' diff --git a/apps/sim/lib/logs/execution/logger.ts b/apps/sim/lib/logs/execution/logger.ts index f40b031c21e..1171bee6a5c 100644 --- a/apps/sim/lib/logs/execution/logger.ts +++ b/apps/sim/lib/logs/execution/logger.ts @@ -34,6 +34,7 @@ import { import { resolveEffectivePiiRedaction } from '@/lib/billing/retention' import { checkAndBillPayerOverageThreshold } from '@/lib/billing/threshold-billing' import { isBillingEnabled } from '@/lib/core/config/env-flags' +import { FOREIGN_KEY_VIOLATION } from '@/lib/core/errors/database-query-error' import { ExecutionScopeDeletedError } from '@/lib/core/errors/execution-scope-deleted' import { redactApiKeys } from '@/lib/core/security/redaction' import { filterForDisplay } from '@/lib/core/utils/display-filters' @@ -99,7 +100,6 @@ const EXECUTION_LOG_IDLE_TIMEOUT_MS = 5_000 // Bounds the wait for the per-execution usage-reconcile advisory lock. Generous // (favor waiting over dropping a charge); only trips on a pathological lock hold. const USAGE_RECONCILE_LOCK_TIMEOUT_MS = 10_000 -const FOREIGN_KEY_VIOLATION = '23503' /** The log's snapshot foreign key as Postgres names it (identifiers are cut at 63 bytes). */ const STATE_SNAPSHOT_FOREIGN_KEY = 'workflow_execution_logs_state_snapshot_id_workflow_execution_sn' diff --git a/apps/sim/lib/logs/execution/logging-session.test.ts b/apps/sim/lib/logs/execution/logging-session.test.ts index bf3f0d74f19..606d070acba 100644 --- a/apps/sim/lib/logs/execution/logging-session.test.ts +++ b/apps/sim/lib/logs/execution/logging-session.test.ts @@ -1248,6 +1248,37 @@ describe('LoggingSession deleted run scope', () => { expect(sessionLogger?.error).not.toHaveBeenCalled() }) + it('stops a run whose minimal fallback start finds the workspace deleted', async () => { + const session = new LoggingSession('workflow-1', 'execution-d6', 'api', 'req-1') + startWorkflowExecutionMock + .mockRejectedValueOnce(new Error('connection reset')) + .mockRejectedValueOnce(foreignKeyViolation()) + findDeletedExecutionScopeMock.mockResolvedValue('workspace') + + await expect(session.safeStart(startParams)).resolves.toBe(false) + + expect(startWorkflowExecutionMock).toHaveBeenCalledTimes(2) + expect(session.getDeletedScope()).toBe('workspace') + expect(releaseExecutionSlotMock).toHaveBeenCalledWith('execution-d6') + const errorMessages = vi.mocked(sessionLogger?.error).mock.calls.map(([message]) => message) + expect(errorMessages.some((message) => String(message).includes('Minimal logging'))).toBe(false) + }) + + it('ends quietly when the cost-only fallback is what finds the workspace deleted', async () => { + const session = new LoggingSession('workflow-1', 'execution-d7', 'api', 'req-1') + completeWorkflowExecutionMock + .mockRejectedValueOnce(new Error('trace store unavailable')) + .mockRejectedValueOnce(new ExecutionScopeDeletedError('execution-d7', 'workspace')) + + await expect(session.safeComplete({ finalOutput: { ok: true } })).resolves.toBeUndefined() + + expect(completeWorkflowExecutionMock).toHaveBeenCalledTimes(2) + expect(session.getDeletedScope()).toBe('workspace') + expect(releaseExecutionSlotMock).toHaveBeenCalledWith('execution-d7') + const errorMessages = vi.mocked(sessionLogger?.error).mock.calls.map(([message]) => message) + expect(errorMessages.some((message) => String(message).includes('Cost-only'))).toBe(false) + }) + it('falls back to a minimal start when a foreign key fails but the scope still exists', async () => { const session = new LoggingSession('workflow-1', 'execution-d4', 'api', 'req-1') startWorkflowExecutionMock diff --git a/apps/sim/lib/logs/execution/logging-session.ts b/apps/sim/lib/logs/execution/logging-session.ts index f254c53fbd2..b8402e386bc 100644 --- a/apps/sim/lib/logs/execution/logging-session.ts +++ b/apps/sim/lib/logs/execution/logging-session.ts @@ -5,6 +5,7 @@ import { describeError, getPostgresErrorCode, toError } from '@sim/utils/errors' import { and, eq, sql } from 'drizzle-orm' import { releaseExecutionSlot } from '@/lib/billing/calculations/usage-reservation' import type { BillingAttributionSnapshot } from '@/lib/billing/core/attribution' +import { FOREIGN_KEY_VIOLATION } from '@/lib/core/errors/database-query-error' import { ExecutionScopeDeletedError } from '@/lib/core/errors/execution-scope-deleted' import { isRetryableInfrastructureError } from '@/lib/core/errors/retryable-infrastructure' import { RESERVATION_TTL_BUFFER_MS } from '@/lib/core/execution-limits' @@ -126,8 +127,6 @@ function getActiveBlockDisplayProvenance( const logger = createLogger('LoggingSession') -const FOREIGN_KEY_VIOLATION = '23503' - type CompletionAttempt = 'complete' | 'error' | 'cancelled' | 'paused' export interface SecretSafeDisplayContent { diff --git a/apps/sim/lib/logs/execution/execution-scope-deleted.integration.ts b/apps/sim/lib/logs/execution/scope-deleted.integration.ts similarity index 100% rename from apps/sim/lib/logs/execution/execution-scope-deleted.integration.ts rename to apps/sim/lib/logs/execution/scope-deleted.integration.ts diff --git a/apps/sim/lib/mothership/agent-cli/csv-workflow-execution.live.test.ts b/apps/sim/lib/mothership/agent-cli/csv-workflow-execution.live.test.ts index 4fe04fbd75d..b1acc836069 100644 --- a/apps/sim/lib/mothership/agent-cli/csv-workflow-execution.live.test.ts +++ b/apps/sim/lib/mothership/agent-cli/csv-workflow-execution.live.test.ts @@ -181,6 +181,9 @@ vi.mock('@/lib/logs/execution/logging-session', () => ({ async safeStart() { return true } + getDeletedScope() { + return null + } async onBlockStart() {} async onBlockComplete() {} async safeComplete(input: Parameters[0]) { diff --git a/apps/sim/lib/workflows/executor/execution-core.test.ts b/apps/sim/lib/workflows/executor/execution-core.test.ts index 603952df2f3..f4d9ee65de1 100644 --- a/apps/sim/lib/workflows/executor/execution-core.test.ts +++ b/apps/sim/lib/workflows/executor/execution-core.test.ts @@ -1544,9 +1544,9 @@ describe('executeWorkflowCore terminal finalization sequencing', () => { getDeletedScopeMock.mockReturnValue('workspace') const thrown = await executeWorkflowCore({ - snapshot: createSnapshot() as any, + snapshot: createSnapshot() as unknown as ExecutionSnapshot, callbacks: {}, - loggingSession: loggingSession as any, + loggingSession: loggingSession as unknown as LoggingSession, }).catch((error: unknown) => error) await loggingSession.setPostExecutionPromise.mock.calls[0]?.[0] @@ -1568,9 +1568,9 @@ describe('executeWorkflowCore terminal finalization sequencing', () => { }) const result = await executeWorkflowCore({ - snapshot: createSnapshot() as any, + snapshot: createSnapshot() as unknown as ExecutionSnapshot, callbacks: {}, - loggingSession: loggingSession as any, + loggingSession: loggingSession as unknown as LoggingSession, }) await loggingSession.setPostExecutionPromise.mock.calls[0]?.[0] @@ -1593,9 +1593,9 @@ describe('executeWorkflowCore terminal finalization sequencing', () => { hasCompletedMock.mockReturnValue(false) await executeWorkflowCore({ - snapshot: createSnapshot() as any, + snapshot: createSnapshot() as unknown as ExecutionSnapshot, callbacks: {}, - loggingSession: loggingSession as any, + loggingSession: loggingSession as unknown as LoggingSession, }) await loggingSession.setPostExecutionPromise.mock.calls[0]?.[0] diff --git a/apps/sim/lib/workflows/executor/pause-persistence.test.ts b/apps/sim/lib/workflows/executor/pause-persistence.test.ts new file mode 100644 index 00000000000..aaf431615f5 --- /dev/null +++ b/apps/sim/lib/workflows/executor/pause-persistence.test.ts @@ -0,0 +1,55 @@ +import { + humanInTheLoopManagerMock, + humanInTheLoopManagerMockFns, +} from '@sim/testing/mocks/human-in-the-loop-manager.mock' +import { loggingSessionMockFns } from '@sim/testing/mocks/logging-session.mock' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { LoggingSession } from '@/lib/logs/execution/logging-session' +import { handlePostExecutionPauseState } from '@/lib/workflows/executor/pause-persistence' +import type { ExecutionResult } from '@/executor/types' + +vi.mock('@/lib/workflows/executor/human-in-the-loop-manager', () => humanInTheLoopManagerMock) + +const loggingSession = { + waitForPostExecution: loggingSessionMockFns.mockWaitForPostExecution, + hasCompleted: loggingSessionMockFns.mockHasCompleted, + getDeletedScope: loggingSessionMockFns.mockGetDeletedScope, + markAsFailed: loggingSessionMockFns.mockMarkAsFailed, +} as unknown as LoggingSession + +const pausedResult: ExecutionResult = { success: true, output: {}, status: 'paused' } + +describe('handlePostExecutionPauseState for a paused run that was never finalized', () => { + beforeEach(() => { + loggingSessionMockFns.mockHasCompleted.mockReturnValue(false) + loggingSessionMockFns.mockGetDeletedScope.mockReturnValue(null) + }) + + it('leaves a run alone when its workspace was deleted while it paused', async () => { + loggingSessionMockFns.mockGetDeletedScope.mockReturnValue('workspace') + + await handlePostExecutionPauseState({ + result: pausedResult, + workflowId: 'workflow-1', + executionId: 'execution-1', + loggingSession, + }) + + expect(loggingSessionMockFns.mockMarkAsFailed).not.toHaveBeenCalled() + expect(humanInTheLoopManagerMockFns.mockPersistPauseResult).not.toHaveBeenCalled() + }) + + it('still force-fails a run whose paused log was not finalized for any other reason', async () => { + await handlePostExecutionPauseState({ + result: pausedResult, + workflowId: 'workflow-1', + executionId: 'execution-1', + loggingSession, + }) + + expect(loggingSessionMockFns.mockMarkAsFailed).toHaveBeenCalledWith( + 'Failed to record paused execution' + ) + expect(humanInTheLoopManagerMockFns.mockPersistPauseResult).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/workflows/executor/pause-persistence.ts b/apps/sim/lib/workflows/executor/pause-persistence.ts index 357de6b30b0..7faba318634 100644 --- a/apps/sim/lib/workflows/executor/pause-persistence.ts +++ b/apps/sim/lib/workflows/executor/pause-persistence.ts @@ -22,7 +22,9 @@ interface HandlePostExecutionPauseStateArgs { * Every caller of `executeWorkflowCore` must call this after execution completes * to ensure HITL pause state is persisted to the database and queued resumes are drained. * - * - If execution is paused but its workspace or workflow was deleted: nothing is left to persist + * - If the paused completion itself found the workspace or workflow deleted: nothing is left + * to persist. A deletion landing after the paused log was written still fails + * `persistPauseResult` and is force-failed below. * - If execution is paused but its log was never finalized: marks execution as failed * - If execution is paused without a snapshot: marks execution as failed * - If execution is paused with a valid snapshot: persists to `paused_executions` table From 7820fc9f699e9fade86365596ce19b704edac31a Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 16 Oct 2026 15:09:29 -0700 Subject: [PATCH 3/6] test(logs): type the logged-message callbacks in the deleted-scope session tests --- .../lib/logs/execution/logging-session.test.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/apps/sim/lib/logs/execution/logging-session.test.ts b/apps/sim/lib/logs/execution/logging-session.test.ts index 606d070acba..4c4b625496d 100644 --- a/apps/sim/lib/logs/execution/logging-session.test.ts +++ b/apps/sim/lib/logs/execution/logging-session.test.ts @@ -1260,8 +1260,12 @@ describe('LoggingSession deleted run scope', () => { expect(startWorkflowExecutionMock).toHaveBeenCalledTimes(2) expect(session.getDeletedScope()).toBe('workspace') expect(releaseExecutionSlotMock).toHaveBeenCalledWith('execution-d6') - const errorMessages = vi.mocked(sessionLogger?.error).mock.calls.map(([message]) => message) - expect(errorMessages.some((message) => String(message).includes('Minimal logging'))).toBe(false) + const errorMessages = vi + .mocked(sessionLogger?.error) + .mock.calls.map(([message]: unknown[]) => message) + expect( + errorMessages.some((message: unknown) => String(message).includes('Minimal logging')) + ).toBe(false) }) it('ends quietly when the cost-only fallback is what finds the workspace deleted', async () => { @@ -1275,8 +1279,12 @@ describe('LoggingSession deleted run scope', () => { expect(completeWorkflowExecutionMock).toHaveBeenCalledTimes(2) expect(session.getDeletedScope()).toBe('workspace') expect(releaseExecutionSlotMock).toHaveBeenCalledWith('execution-d7') - const errorMessages = vi.mocked(sessionLogger?.error).mock.calls.map(([message]) => message) - expect(errorMessages.some((message) => String(message).includes('Cost-only'))).toBe(false) + const errorMessages = vi + .mocked(sessionLogger?.error) + .mock.calls.map(([message]: unknown[]) => message) + expect(errorMessages.some((message: unknown) => String(message).includes('Cost-only'))).toBe( + false + ) }) it('falls back to a minimal start when a foreign key fails but the scope still exists', async () => { From 35138820763e993cc1ba6a5d91795446a456a781 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 16 Oct 2026 15:25:25 -0700 Subject: [PATCH 4/6] 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. --- .../utils/file-tool-processor.test.ts | 22 +++++++ .../sim/executor/utils/file-tool-processor.ts | 3 +- apps/sim/lib/logs/execution/logger.test.ts | 63 ++++++++++++++++++- apps/sim/lib/logs/execution/logger.ts | 37 +++++++++-- 4 files changed, 119 insertions(+), 6 deletions(-) diff --git a/apps/sim/executor/utils/file-tool-processor.test.ts b/apps/sim/executor/utils/file-tool-processor.test.ts index 1fe22b25296..93ed62eb033 100644 --- a/apps/sim/executor/utils/file-tool-processor.test.ts +++ b/apps/sim/executor/utils/file-tool-processor.test.ts @@ -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' @@ -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, diff --git a/apps/sim/executor/utils/file-tool-processor.ts b/apps/sim/executor/utils/file-tool-processor.ts index 738ad93298f..117f9a3fbeb 100644 --- a/apps/sim/executor/utils/file-tool-processor.ts +++ b/apps/sim/executor/utils/file-tool-processor.ts @@ -123,7 +123,8 @@ export class FileToolProcessor { signal?.throwIfAborted() logger.error(`Error processing file output '${outputKey}':`, error) throw new Error( - `Failed to process file output '${outputKey}': ${getUserFacingErrorMessage(error)}` + `Failed to process file output '${outputKey}': ${getUserFacingErrorMessage(error)}`, + { cause: error } ) } } diff --git a/apps/sim/lib/logs/execution/logger.test.ts b/apps/sim/lib/logs/execution/logger.test.ts index cd4af8d0b79..d09ac80ebfc 100644 --- a/apps/sim/lib/logs/execution/logger.test.ts +++ b/apps/sim/lib/logs/execution/logger.test.ts @@ -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' @@ -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 + } + ).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 diff --git a/apps/sim/lib/logs/execution/logger.ts b/apps/sim/lib/logs/execution/logger.ts index 1171bee6a5c..d683477df12 100644 --- a/apps/sim/lib/logs/execution/logger.ts +++ b/apps/sim/lib/logs/execution/logger.ts @@ -313,12 +313,34 @@ async function assertExecutionWorkspaceExists( workspaceId: string | null | undefined ): Promise { if (!workspaceId) return + if (!(await workspaceExists(client, workspaceId))) { + throw new ExecutionScopeDeletedError(executionId, 'workspace') + } +} + +async function workspaceExists( + client: Pick, + workspaceId: string +): Promise { const [row] = await client .select({ id: workspace.id }) .from(workspace) .where(eq(workspace.id, workspaceId)) .limit(1) - if (!row) throw new ExecutionScopeDeletedError(executionId, 'workspace') + return row !== undefined +} + +/** + * Whether a missing workflow is explained by its whole workspace having been deleted, which the + * completion then reports as a deleted run. A failed lookup reports `false`, keeping the error. + */ +async function isWorkspaceDeleted(workspaceId: string | null | undefined): Promise { + if (!workspaceId) return false + try { + return !(await workspaceExists(execDb, workspaceId)) + } catch { + return false + } } function countTraceSpans(traceSpans?: TraceSpan[]): number { @@ -1238,7 +1260,8 @@ export class ExecutionLogger { executionId, actorUserId, exactBillingContext, - status !== 'pending' + status !== 'pending', + existingLog.workspaceId ) } @@ -1491,7 +1514,9 @@ export class ExecutionLogger { * reconcile across boundaries, so they are written once — at the terminal * boundary, where the summary already holds the run's cumulative tokens. */ - isTerminalBoundary = true + isTerminalBoundary = true, + /** The run's workspace, read only to explain a missing workflow. */ + workspaceId?: string | null ): Promise { const statsLog = logger.withMetadata({ workflowId: workflowId ?? undefined, executionId }) @@ -1517,7 +1542,11 @@ export class ExecutionLogger { .limit(1) if (!workflowRecord) { - statsLog.error('Workflow not found for usage recording') + if (await isWorkspaceDeleted(workspaceId)) { + statsLog.info("Skipped usage recording: the run's workspace was deleted") + } else { + statsLog.error('Workflow not found for usage recording') + } return NO_USAGE_RECORDED } From 9003bf0ea1833069e2dec75821cdc16a950799b8 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 16 Oct 2026 15:40:27 -0700 Subject: [PATCH 5/6] 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. --- .../core/errors/execution-scope-deleted.ts | 9 +- apps/sim/lib/logs/execution/logger.ts | 20 ++-- .../sim/lib/logs/execution/logging-session.ts | 102 ++++++------------ .../lib/workflows/executor/execution-core.ts | 2 +- .../testing/src/mocks/logging-session.mock.ts | 3 +- 5 files changed, 51 insertions(+), 85 deletions(-) diff --git a/apps/sim/lib/core/errors/execution-scope-deleted.ts b/apps/sim/lib/core/errors/execution-scope-deleted.ts index fe090882541..51dc7337c73 100644 --- a/apps/sim/lib/core/errors/execution-scope-deleted.ts +++ b/apps/sim/lib/core/errors/execution-scope-deleted.ts @@ -1,3 +1,6 @@ +/** 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 @@ -5,13 +8,11 @@ * that revealed the deletion as internal. */ export class ExecutionScopeDeletedError extends Error { - readonly executionId: string - readonly scope: 'workspace' | 'workflow' + readonly scope: ExecutionScope - constructor(executionId: string, scope: 'workspace' | 'workflow') { + constructor(executionId: string, scope: ExecutionScope) { super(`The ${scope} of execution ${executionId} was deleted`) this.name = 'ExecutionScopeDeletedError' - this.executionId = executionId this.scope = scope } } diff --git a/apps/sim/lib/logs/execution/logger.ts b/apps/sim/lib/logs/execution/logger.ts index d683477df12..850e817fd48 100644 --- a/apps/sim/lib/logs/execution/logger.ts +++ b/apps/sim/lib/logs/execution/logger.ts @@ -35,7 +35,10 @@ import { resolveEffectivePiiRedaction } from '@/lib/billing/retention' import { checkAndBillPayerOverageThreshold } from '@/lib/billing/threshold-billing' import { isBillingEnabled } from '@/lib/core/config/env-flags' import { FOREIGN_KEY_VIOLATION } from '@/lib/core/errors/database-query-error' -import { ExecutionScopeDeletedError } from '@/lib/core/errors/execution-scope-deleted' +import { + type ExecutionScope, + ExecutionScopeDeletedError, +} from '@/lib/core/errors/execution-scope-deleted' import { redactApiKeys } from '@/lib/core/security/redaction' import { filterForDisplay } from '@/lib/core/utils/display-filters' import { acquireAdvisoryXactLock } from '@/lib/db/advisory-locks' @@ -313,12 +316,12 @@ async function assertExecutionWorkspaceExists( workspaceId: string | null | undefined ): Promise { if (!workspaceId) return - if (!(await workspaceExists(client, workspaceId))) { + if (!(await workspaceRowExists(client, workspaceId))) { throw new ExecutionScopeDeletedError(executionId, 'workspace') } } -async function workspaceExists( +async function workspaceRowExists( client: Pick, workspaceId: string ): Promise { @@ -337,7 +340,7 @@ async function workspaceExists( async function isWorkspaceDeleted(workspaceId: string | null | undefined): Promise { if (!workspaceId) return false try { - return !(await workspaceExists(execDb, workspaceId)) + return !(await workspaceRowExists(execDb, workspaceId)) } catch { return false } @@ -878,13 +881,8 @@ export class ExecutionLogger { async findDeletedExecutionScope(params: { workspaceId: string workflowId: string - }): Promise { - const [workspaceRow] = await execDb - .select({ id: workspace.id }) - .from(workspace) - .where(eq(workspace.id, params.workspaceId)) - .limit(1) - if (!workspaceRow) return 'workspace' + }): Promise { + if (!(await workspaceRowExists(execDb, params.workspaceId))) return 'workspace' const [workflowRow] = await execDb .select({ id: workflow.id }) .from(workflow) diff --git a/apps/sim/lib/logs/execution/logging-session.ts b/apps/sim/lib/logs/execution/logging-session.ts index b8402e386bc..2f12d944b41 100644 --- a/apps/sim/lib/logs/execution/logging-session.ts +++ b/apps/sim/lib/logs/execution/logging-session.ts @@ -6,7 +6,10 @@ import { and, eq, sql } from 'drizzle-orm' import { releaseExecutionSlot } from '@/lib/billing/calculations/usage-reservation' import type { BillingAttributionSnapshot } from '@/lib/billing/core/attribution' import { FOREIGN_KEY_VIOLATION } from '@/lib/core/errors/database-query-error' -import { ExecutionScopeDeletedError } from '@/lib/core/errors/execution-scope-deleted' +import { + type ExecutionScope, + ExecutionScopeDeletedError, +} from '@/lib/core/errors/execution-scope-deleted' import { isRetryableInfrastructureError } from '@/lib/core/errors/retryable-infrastructure' import { RESERVATION_TTL_BUFFER_MS } from '@/lib/core/execution-limits' import type { LargeValueStoreContext } from '@/lib/execution/payloads/store' @@ -228,7 +231,7 @@ export class LoggingSession { private persistedCompletionStatus: PersistedWorkflowExecutionStatus | null = null private lastCompletionError: string | null = null /** Set once the run's workspace or workflow is found deleted; nothing is left to record. */ - private deletedScope: ExecutionScopeDeletedError['scope'] | null = null + private deletedScope: ExecutionScope | null = null constructor( workflowId: string, @@ -731,13 +734,7 @@ export class LoggingSession { * protects the execution. */ if (params.finalizationPath !== 'paused') { - try { - await releaseExecutionSlot(this.reservationId) - } catch (error) { - logger.warn(`Failed to release admission reservation for ${this.executionId}:`, { - error: toError(error).message, - }) - } + await this.releaseAdmissionSlot() } } @@ -869,8 +866,8 @@ export class LoggingSession { ) } } catch (error) { - const deletedScope = skipLogCreation ? null : await this.findDeletedScope(error, workspaceId) - if (deletedScope) throw this.recordDeletedScope(deletedScope) + const deleted = skipLogCreation ? null : await this.findDeletedScope(error, workspaceId) + if (deleted) throw deleted if (this.requestId) { logger.error(`[${this.requestId}] Failed to start logging:`, error) } @@ -879,19 +876,20 @@ export class LoggingSession { } /** - * Which of the run's scopes was deleted, when a log insert failed on a foreign key. Any other - * failure, or a lookup that itself fails, reports nothing so the caller keeps its usual path. + * The run's deleted scope, when a log insert failed on a foreign key. Any other failure, or a + * lookup that itself fails, reports nothing so the caller keeps its usual path. */ private async findDeletedScope( error: unknown, workspaceId: string - ): Promise { + ): Promise { if (getPostgresErrorCode(error) !== FOREIGN_KEY_VIOLATION) return null try { - return await executionLogger.findDeletedExecutionScope({ + const scope = await executionLogger.findDeletedExecutionScope({ workspaceId, workflowId: this.workflowId, }) + return scope ? new ExecutionScopeDeletedError(this.executionId, scope) : null } catch (lookupError) { logger.warn(`Could not check whether execution ${this.executionId} lost its scope`, { error: describeError(lookupError), @@ -900,16 +898,11 @@ export class LoggingSession { } } - private recordDeletedScope( - scope: ExecutionScopeDeletedError['scope'] - ): ExecutionScopeDeletedError { - const error = new ExecutionScopeDeletedError(this.executionId, scope) - this.noteDeletedScope(error) - return error - } - - /** Records a deleted scope once, at info: the owner deleted it, so there is nothing to fix. */ - private noteDeletedScope(error: ExecutionScopeDeletedError): void { + /** + * Ends a run whose scope was deleted, once: logged at info since the owner deleted it, and only + * its admission slot released, since no log is left to write. + */ + private async endDeletedScopeRun(error: ExecutionScopeDeletedError): Promise { if (this.deletedScope) return this.deletedScope = error.scope logger.info(`Execution ${this.executionId} ended: its ${error.scope} was deleted`, { @@ -917,18 +910,18 @@ export class LoggingSession { workflowId: this.workflowId, workspaceId: this.workspaceId, }) + await this.releaseAdmissionSlot() } /** * The run's scope found deleted while it ran, or `null`. Once set, the run has no log, cost, or * count left to record, and every later completion is a no-op. */ - getDeletedScope(): ExecutionScopeDeletedError['scope'] | null { + getDeletedScope(): ExecutionScope | null { return this.deletedScope } - /** Ends a run whose scope was deleted: no log is left to write, so only its admission slot. */ - private async releaseSlotForDeletedScope(): Promise { + private async releaseAdmissionSlot(): Promise { try { await releaseExecutionSlot(this.reservationId) } catch (error) { @@ -1004,10 +997,7 @@ export class LoggingSession { } } catch (error) { this.completing = false - if (error instanceof ExecutionScopeDeletedError) { - this.noteDeletedScope(error) - throw error - } + if (error instanceof ExecutionScopeDeletedError) throw error logger.error(`Failed to complete logging for execution ${this.executionId}:`, { requestId: this.requestId, workflowId: this.workflowId, @@ -1142,10 +1132,7 @@ export class LoggingSession { } } catch (enhancedError) { this.completing = false - if (enhancedError instanceof ExecutionScopeDeletedError) { - this.noteDeletedScope(enhancedError) - throw enhancedError - } + if (enhancedError instanceof ExecutionScopeDeletedError) throw enhancedError logger.error(`Failed to complete error logging for execution ${this.executionId}:`, { requestId: this.requestId, workflowId: this.workflowId, @@ -1227,10 +1214,7 @@ export class LoggingSession { } } catch (cancelError) { this.completing = false - if (cancelError instanceof ExecutionScopeDeletedError) { - this.noteDeletedScope(cancelError) - throw cancelError - } + if (cancelError instanceof ExecutionScopeDeletedError) throw cancelError logger.error(`Failed to complete cancelled logging for execution ${this.executionId}:`, { requestId: this.requestId, workflowId: this.workflowId, @@ -1330,10 +1314,7 @@ export class LoggingSession { } } catch (pauseError) { this.completing = false - if (pauseError instanceof ExecutionScopeDeletedError) { - this.noteDeletedScope(pauseError) - throw pauseError - } + if (pauseError instanceof ExecutionScopeDeletedError) throw pauseError logger.error(`Failed to complete paused logging for execution ${this.executionId}:`, { requestId: this.requestId, workflowId: this.workflowId, @@ -1357,7 +1338,7 @@ export class LoggingSession { return true } catch (error) { if (error instanceof ExecutionScopeDeletedError) { - await this.releaseSlotForDeletedScope() + await this.endDeletedScopeRun(error) return false } if (this.requestId) { @@ -1419,10 +1400,9 @@ export class LoggingSession { } return true } catch (fallbackError) { - const deletedScope = await this.findDeletedScope(fallbackError, params.workspaceId) - if (deletedScope) { - this.recordDeletedScope(deletedScope) - await this.releaseSlotForDeletedScope() + const deleted = await this.findDeletedScope(fallbackError, params.workspaceId) + if (deleted) { + await this.endDeletedScopeRun(deleted) return false } if (this.requestId) { @@ -1517,10 +1497,7 @@ export class LoggingSession { await this.drainPendingProgressWrites() await this.complete(params) } catch (error) { - if (error instanceof ExecutionScopeDeletedError) { - await this.releaseSlotForDeletedScope() - return - } + if (error instanceof ExecutionScopeDeletedError) return this.endDeletedScopeRun(error) const errorMsg = toError(error).message logger.warn( `[${this.requestId || 'unknown'}] Complete failed for execution ${this.executionId}, attempting fallback`, @@ -1549,10 +1526,7 @@ export class LoggingSession { await this.drainPendingProgressWrites() await this.completeWithError(params) } catch (error) { - if (error instanceof ExecutionScopeDeletedError) { - await this.releaseSlotForDeletedScope() - return - } + if (error instanceof ExecutionScopeDeletedError) return this.endDeletedScopeRun(error) const errorMsg = toError(error).message logger.warn( `[${this.requestId || 'unknown'}] CompleteWithError failed for execution ${this.executionId}, attempting fallback`, @@ -1586,10 +1560,7 @@ export class LoggingSession { await this.drainPendingProgressWrites() await this.completeWithCancellation(params) } catch (error) { - if (error instanceof ExecutionScopeDeletedError) { - await this.releaseSlotForDeletedScope() - return - } + if (error instanceof ExecutionScopeDeletedError) return this.endDeletedScopeRun(error) const errorMsg = toError(error).message logger.warn( `[${this.requestId || 'unknown'}] CompleteWithCancellation failed for execution ${this.executionId}, attempting fallback`, @@ -1618,10 +1589,7 @@ export class LoggingSession { await this.drainPendingProgressWrites() await this.completeWithPause(params) } catch (error) { - if (error instanceof ExecutionScopeDeletedError) { - await this.releaseSlotForDeletedScope() - return - } + if (error instanceof ExecutionScopeDeletedError) return this.endDeletedScopeRun(error) const errorMsg = toError(error).message logger.warn( `[${this.requestId || 'unknown'}] CompleteWithPause failed for execution ${this.executionId}, attempting fallback`, @@ -1785,9 +1753,7 @@ export class LoggingSession { } catch (fallbackError) { this.completing = false if (fallbackError instanceof ExecutionScopeDeletedError) { - this.noteDeletedScope(fallbackError) - await this.releaseSlotForDeletedScope() - return + return this.endDeletedScopeRun(fallbackError) } this.completionAttemptFailed = true logger.error( diff --git a/apps/sim/lib/workflows/executor/execution-core.ts b/apps/sim/lib/workflows/executor/execution-core.ts index 254978c40e2..6dae6a8d8ef 100644 --- a/apps/sim/lib/workflows/executor/execution-core.ts +++ b/apps/sim/lib/workflows/executor/execution-core.ts @@ -853,7 +853,7 @@ async function executeWorkflowCoreImpl( * A start that failed because the run's workspace or workflow is gone ends the run here, * before any block spends on providers: nothing could ever record it or stop it. */ - const deletedScope = loggingStarted ? null : loggingSession.getDeletedScope() + const deletedScope = loggingSession.getDeletedScope() if (deletedScope) throw new ExecutionScopeDeletedError(executionId, deletedScope) // Use edges directly - trigger-to-trigger edges are prevented at creation time diff --git a/packages/testing/src/mocks/logging-session.mock.ts b/packages/testing/src/mocks/logging-session.mock.ts index a385d6b5b7f..8c4ba248bca 100644 --- a/packages/testing/src/mocks/logging-session.mock.ts +++ b/packages/testing/src/mocks/logging-session.mock.ts @@ -7,7 +7,8 @@ import { vi } from 'vitest' * mock state. `mockSafeStart` defaults to `true` because callers branch on the * boolean result. Display projection methods (incl. `projectTraceSpansForLiveDisplay`) return * their input, diagnostic projection fails closed to structural metadata, `hasCompleted` is - * `false`, `getPersistedCompletionStatus` and `getDeletedScope` are `null`, and other methods resolve to `undefined`. + * `false`, `getPersistedCompletionStatus` and `getDeletedScope` are `null`, and other methods + * resolve to `undefined`. * Defaults are `vi.fn(impl)`, so `mockReset()` restores them. * * @example From 561be5bf7c3f462f4fa3a97bc341c732953c2c7e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 16 Oct 2026 15:58:52 -0700 Subject: [PATCH 6/6] test: red proof for #9144 (do not merge) --- apps/sim/lib/logs/execution/logger.ts | 7 +++---- apps/sim/lib/logs/execution/logging-session.ts | 1 + 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/sim/lib/logs/execution/logger.ts b/apps/sim/lib/logs/execution/logger.ts index 850e817fd48..a0e9f961810 100644 --- a/apps/sim/lib/logs/execution/logger.ts +++ b/apps/sim/lib/logs/execution/logger.ts @@ -315,10 +315,9 @@ async function assertExecutionWorkspaceExists( executionId: string, workspaceId: string | null | undefined ): Promise { - if (!workspaceId) return - if (!(await workspaceRowExists(client, workspaceId))) { - throw new ExecutionScopeDeletedError(executionId, 'workspace') - } + void client + void executionId + void workspaceId } async function workspaceRowExists( diff --git a/apps/sim/lib/logs/execution/logging-session.ts b/apps/sim/lib/logs/execution/logging-session.ts index 2f12d944b41..9dea592b795 100644 --- a/apps/sim/lib/logs/execution/logging-session.ts +++ b/apps/sim/lib/logs/execution/logging-session.ts @@ -884,6 +884,7 @@ export class LoggingSession { workspaceId: string ): Promise { if (getPostgresErrorCode(error) !== FOREIGN_KEY_VIOLATION) return null + if (error) return null try { const scope = await executionLogger.findDeletedExecutionScope({ workspaceId,