Skip to content

Commit 9003bf0

Browse files
committed
refactor(execution): simplify deleted-scope handling after audit
Fold the deleted-scope note and slot release into one session helper, reuse the admission-slot release on the normal completion path, share an ExecutionScope type, drop the unread executionId field and a redundant guard.
1 parent 3513882 commit 9003bf0

5 files changed

Lines changed: 51 additions & 85 deletions

File tree

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
1+
/** The part of a run's ownership that can be hard-deleted while the run is in flight. */
2+
export type ExecutionScope = 'workspace' | 'workflow'
3+
14
/**
25
* A run's workspace or workflow was hard-deleted while it was in flight, so the run has nowhere
36
* left to record its log, cost, or counts. An expected outcome of the owner deleting it, not a
47
* failure to retry or page on. Carries no `cause`: `classifyFailure` would rank the database error
58
* that revealed the deletion as internal.
69
*/
710
export class ExecutionScopeDeletedError extends Error {
8-
readonly executionId: string
9-
readonly scope: 'workspace' | 'workflow'
11+
readonly scope: ExecutionScope
1012

11-
constructor(executionId: string, scope: 'workspace' | 'workflow') {
13+
constructor(executionId: string, scope: ExecutionScope) {
1214
super(`The ${scope} of execution ${executionId} was deleted`)
1315
this.name = 'ExecutionScopeDeletedError'
14-
this.executionId = executionId
1516
this.scope = scope
1617
}
1718
}

‎apps/sim/lib/logs/execution/logger.ts‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,10 @@ import { resolveEffectivePiiRedaction } from '@/lib/billing/retention'
3535
import { checkAndBillPayerOverageThreshold } from '@/lib/billing/threshold-billing'
3636
import { isBillingEnabled } from '@/lib/core/config/env-flags'
3737
import { FOREIGN_KEY_VIOLATION } from '@/lib/core/errors/database-query-error'
38-
import { ExecutionScopeDeletedError } from '@/lib/core/errors/execution-scope-deleted'
38+
import {
39+
type ExecutionScope,
40+
ExecutionScopeDeletedError,
41+
} from '@/lib/core/errors/execution-scope-deleted'
3942
import { redactApiKeys } from '@/lib/core/security/redaction'
4043
import { filterForDisplay } from '@/lib/core/utils/display-filters'
4144
import { acquireAdvisoryXactLock } from '@/lib/db/advisory-locks'
@@ -313,12 +316,12 @@ async function assertExecutionWorkspaceExists(
313316
workspaceId: string | null | undefined
314317
): Promise<void> {
315318
if (!workspaceId) return
316-
if (!(await workspaceExists(client, workspaceId))) {
319+
if (!(await workspaceRowExists(client, workspaceId))) {
317320
throw new ExecutionScopeDeletedError(executionId, 'workspace')
318321
}
319322
}
320323

321-
async function workspaceExists(
324+
async function workspaceRowExists(
322325
client: Pick<typeof db, 'select'>,
323326
workspaceId: string
324327
): Promise<boolean> {
@@ -337,7 +340,7 @@ async function workspaceExists(
337340
async function isWorkspaceDeleted(workspaceId: string | null | undefined): Promise<boolean> {
338341
if (!workspaceId) return false
339342
try {
340-
return !(await workspaceExists(execDb, workspaceId))
343+
return !(await workspaceRowExists(execDb, workspaceId))
341344
} catch {
342345
return false
343346
}
@@ -878,13 +881,8 @@ export class ExecutionLogger {
878881
async findDeletedExecutionScope(params: {
879882
workspaceId: string
880883
workflowId: string
881-
}): Promise<ExecutionScopeDeletedError['scope'] | null> {
882-
const [workspaceRow] = await execDb
883-
.select({ id: workspace.id })
884-
.from(workspace)
885-
.where(eq(workspace.id, params.workspaceId))
886-
.limit(1)
887-
if (!workspaceRow) return 'workspace'
884+
}): Promise<ExecutionScope | null> {
885+
if (!(await workspaceRowExists(execDb, params.workspaceId))) return 'workspace'
888886
const [workflowRow] = await execDb
889887
.select({ id: workflow.id })
890888
.from(workflow)

‎apps/sim/lib/logs/execution/logging-session.ts‎

Lines changed: 34 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ import { and, eq, sql } from 'drizzle-orm'
66
import { releaseExecutionSlot } from '@/lib/billing/calculations/usage-reservation'
77
import type { BillingAttributionSnapshot } from '@/lib/billing/core/attribution'
88
import { FOREIGN_KEY_VIOLATION } from '@/lib/core/errors/database-query-error'
9-
import { ExecutionScopeDeletedError } from '@/lib/core/errors/execution-scope-deleted'
9+
import {
10+
type ExecutionScope,
11+
ExecutionScopeDeletedError,
12+
} from '@/lib/core/errors/execution-scope-deleted'
1013
import { isRetryableInfrastructureError } from '@/lib/core/errors/retryable-infrastructure'
1114
import { RESERVATION_TTL_BUFFER_MS } from '@/lib/core/execution-limits'
1215
import type { LargeValueStoreContext } from '@/lib/execution/payloads/store'
@@ -228,7 +231,7 @@ export class LoggingSession {
228231
private persistedCompletionStatus: PersistedWorkflowExecutionStatus | null = null
229232
private lastCompletionError: string | null = null
230233
/** Set once the run's workspace or workflow is found deleted; nothing is left to record. */
231-
private deletedScope: ExecutionScopeDeletedError['scope'] | null = null
234+
private deletedScope: ExecutionScope | null = null
232235

233236
constructor(
234237
workflowId: string,
@@ -731,13 +734,7 @@ export class LoggingSession {
731734
* protects the execution.
732735
*/
733736
if (params.finalizationPath !== 'paused') {
734-
try {
735-
await releaseExecutionSlot(this.reservationId)
736-
} catch (error) {
737-
logger.warn(`Failed to release admission reservation for ${this.executionId}:`, {
738-
error: toError(error).message,
739-
})
740-
}
737+
await this.releaseAdmissionSlot()
741738
}
742739
}
743740

@@ -869,8 +866,8 @@ export class LoggingSession {
869866
)
870867
}
871868
} catch (error) {
872-
const deletedScope = skipLogCreation ? null : await this.findDeletedScope(error, workspaceId)
873-
if (deletedScope) throw this.recordDeletedScope(deletedScope)
869+
const deleted = skipLogCreation ? null : await this.findDeletedScope(error, workspaceId)
870+
if (deleted) throw deleted
874871
if (this.requestId) {
875872
logger.error(`[${this.requestId}] Failed to start logging:`, error)
876873
}
@@ -879,19 +876,20 @@ export class LoggingSession {
879876
}
880877

881878
/**
882-
* Which of the run's scopes was deleted, when a log insert failed on a foreign key. Any other
883-
* failure, or a lookup that itself fails, reports nothing so the caller keeps its usual path.
879+
* The run's deleted scope, when a log insert failed on a foreign key. Any other failure, or a
880+
* lookup that itself fails, reports nothing so the caller keeps its usual path.
884881
*/
885882
private async findDeletedScope(
886883
error: unknown,
887884
workspaceId: string
888-
): Promise<ExecutionScopeDeletedError['scope'] | null> {
885+
): Promise<ExecutionScopeDeletedError | null> {
889886
if (getPostgresErrorCode(error) !== FOREIGN_KEY_VIOLATION) return null
890887
try {
891-
return await executionLogger.findDeletedExecutionScope({
888+
const scope = await executionLogger.findDeletedExecutionScope({
892889
workspaceId,
893890
workflowId: this.workflowId,
894891
})
892+
return scope ? new ExecutionScopeDeletedError(this.executionId, scope) : null
895893
} catch (lookupError) {
896894
logger.warn(`Could not check whether execution ${this.executionId} lost its scope`, {
897895
error: describeError(lookupError),
@@ -900,35 +898,30 @@ export class LoggingSession {
900898
}
901899
}
902900

903-
private recordDeletedScope(
904-
scope: ExecutionScopeDeletedError['scope']
905-
): ExecutionScopeDeletedError {
906-
const error = new ExecutionScopeDeletedError(this.executionId, scope)
907-
this.noteDeletedScope(error)
908-
return error
909-
}
910-
911-
/** Records a deleted scope once, at info: the owner deleted it, so there is nothing to fix. */
912-
private noteDeletedScope(error: ExecutionScopeDeletedError): void {
901+
/**
902+
* Ends a run whose scope was deleted, once: logged at info since the owner deleted it, and only
903+
* its admission slot released, since no log is left to write.
904+
*/
905+
private async endDeletedScopeRun(error: ExecutionScopeDeletedError): Promise<void> {
913906
if (this.deletedScope) return
914907
this.deletedScope = error.scope
915908
logger.info(`Execution ${this.executionId} ended: its ${error.scope} was deleted`, {
916909
requestId: this.requestId,
917910
workflowId: this.workflowId,
918911
workspaceId: this.workspaceId,
919912
})
913+
await this.releaseAdmissionSlot()
920914
}
921915

922916
/**
923917
* The run's scope found deleted while it ran, or `null`. Once set, the run has no log, cost, or
924918
* count left to record, and every later completion is a no-op.
925919
*/
926-
getDeletedScope(): ExecutionScopeDeletedError['scope'] | null {
920+
getDeletedScope(): ExecutionScope | null {
927921
return this.deletedScope
928922
}
929923

930-
/** Ends a run whose scope was deleted: no log is left to write, so only its admission slot. */
931-
private async releaseSlotForDeletedScope(): Promise<void> {
924+
private async releaseAdmissionSlot(): Promise<void> {
932925
try {
933926
await releaseExecutionSlot(this.reservationId)
934927
} catch (error) {
@@ -1004,10 +997,7 @@ export class LoggingSession {
1004997
}
1005998
} catch (error) {
1006999
this.completing = false
1007-
if (error instanceof ExecutionScopeDeletedError) {
1008-
this.noteDeletedScope(error)
1009-
throw error
1010-
}
1000+
if (error instanceof ExecutionScopeDeletedError) throw error
10111001
logger.error(`Failed to complete logging for execution ${this.executionId}:`, {
10121002
requestId: this.requestId,
10131003
workflowId: this.workflowId,
@@ -1142,10 +1132,7 @@ export class LoggingSession {
11421132
}
11431133
} catch (enhancedError) {
11441134
this.completing = false
1145-
if (enhancedError instanceof ExecutionScopeDeletedError) {
1146-
this.noteDeletedScope(enhancedError)
1147-
throw enhancedError
1148-
}
1135+
if (enhancedError instanceof ExecutionScopeDeletedError) throw enhancedError
11491136
logger.error(`Failed to complete error logging for execution ${this.executionId}:`, {
11501137
requestId: this.requestId,
11511138
workflowId: this.workflowId,
@@ -1227,10 +1214,7 @@ export class LoggingSession {
12271214
}
12281215
} catch (cancelError) {
12291216
this.completing = false
1230-
if (cancelError instanceof ExecutionScopeDeletedError) {
1231-
this.noteDeletedScope(cancelError)
1232-
throw cancelError
1233-
}
1217+
if (cancelError instanceof ExecutionScopeDeletedError) throw cancelError
12341218
logger.error(`Failed to complete cancelled logging for execution ${this.executionId}:`, {
12351219
requestId: this.requestId,
12361220
workflowId: this.workflowId,
@@ -1330,10 +1314,7 @@ export class LoggingSession {
13301314
}
13311315
} catch (pauseError) {
13321316
this.completing = false
1333-
if (pauseError instanceof ExecutionScopeDeletedError) {
1334-
this.noteDeletedScope(pauseError)
1335-
throw pauseError
1336-
}
1317+
if (pauseError instanceof ExecutionScopeDeletedError) throw pauseError
13371318
logger.error(`Failed to complete paused logging for execution ${this.executionId}:`, {
13381319
requestId: this.requestId,
13391320
workflowId: this.workflowId,
@@ -1357,7 +1338,7 @@ export class LoggingSession {
13571338
return true
13581339
} catch (error) {
13591340
if (error instanceof ExecutionScopeDeletedError) {
1360-
await this.releaseSlotForDeletedScope()
1341+
await this.endDeletedScopeRun(error)
13611342
return false
13621343
}
13631344
if (this.requestId) {
@@ -1419,10 +1400,9 @@ export class LoggingSession {
14191400
}
14201401
return true
14211402
} catch (fallbackError) {
1422-
const deletedScope = await this.findDeletedScope(fallbackError, params.workspaceId)
1423-
if (deletedScope) {
1424-
this.recordDeletedScope(deletedScope)
1425-
await this.releaseSlotForDeletedScope()
1403+
const deleted = await this.findDeletedScope(fallbackError, params.workspaceId)
1404+
if (deleted) {
1405+
await this.endDeletedScopeRun(deleted)
14261406
return false
14271407
}
14281408
if (this.requestId) {
@@ -1517,10 +1497,7 @@ export class LoggingSession {
15171497
await this.drainPendingProgressWrites()
15181498
await this.complete(params)
15191499
} catch (error) {
1520-
if (error instanceof ExecutionScopeDeletedError) {
1521-
await this.releaseSlotForDeletedScope()
1522-
return
1523-
}
1500+
if (error instanceof ExecutionScopeDeletedError) return this.endDeletedScopeRun(error)
15241501
const errorMsg = toError(error).message
15251502
logger.warn(
15261503
`[${this.requestId || 'unknown'}] Complete failed for execution ${this.executionId}, attempting fallback`,
@@ -1549,10 +1526,7 @@ export class LoggingSession {
15491526
await this.drainPendingProgressWrites()
15501527
await this.completeWithError(params)
15511528
} catch (error) {
1552-
if (error instanceof ExecutionScopeDeletedError) {
1553-
await this.releaseSlotForDeletedScope()
1554-
return
1555-
}
1529+
if (error instanceof ExecutionScopeDeletedError) return this.endDeletedScopeRun(error)
15561530
const errorMsg = toError(error).message
15571531
logger.warn(
15581532
`[${this.requestId || 'unknown'}] CompleteWithError failed for execution ${this.executionId}, attempting fallback`,
@@ -1586,10 +1560,7 @@ export class LoggingSession {
15861560
await this.drainPendingProgressWrites()
15871561
await this.completeWithCancellation(params)
15881562
} catch (error) {
1589-
if (error instanceof ExecutionScopeDeletedError) {
1590-
await this.releaseSlotForDeletedScope()
1591-
return
1592-
}
1563+
if (error instanceof ExecutionScopeDeletedError) return this.endDeletedScopeRun(error)
15931564
const errorMsg = toError(error).message
15941565
logger.warn(
15951566
`[${this.requestId || 'unknown'}] CompleteWithCancellation failed for execution ${this.executionId}, attempting fallback`,
@@ -1618,10 +1589,7 @@ export class LoggingSession {
16181589
await this.drainPendingProgressWrites()
16191590
await this.completeWithPause(params)
16201591
} catch (error) {
1621-
if (error instanceof ExecutionScopeDeletedError) {
1622-
await this.releaseSlotForDeletedScope()
1623-
return
1624-
}
1592+
if (error instanceof ExecutionScopeDeletedError) return this.endDeletedScopeRun(error)
16251593
const errorMsg = toError(error).message
16261594
logger.warn(
16271595
`[${this.requestId || 'unknown'}] CompleteWithPause failed for execution ${this.executionId}, attempting fallback`,
@@ -1785,9 +1753,7 @@ export class LoggingSession {
17851753
} catch (fallbackError) {
17861754
this.completing = false
17871755
if (fallbackError instanceof ExecutionScopeDeletedError) {
1788-
this.noteDeletedScope(fallbackError)
1789-
await this.releaseSlotForDeletedScope()
1790-
return
1756+
return this.endDeletedScopeRun(fallbackError)
17911757
}
17921758
this.completionAttemptFailed = true
17931759
logger.error(

‎apps/sim/lib/workflows/executor/execution-core.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -853,7 +853,7 @@ async function executeWorkflowCoreImpl(
853853
* A start that failed because the run's workspace or workflow is gone ends the run here,
854854
* before any block spends on providers: nothing could ever record it or stop it.
855855
*/
856-
const deletedScope = loggingStarted ? null : loggingSession.getDeletedScope()
856+
const deletedScope = loggingSession.getDeletedScope()
857857
if (deletedScope) throw new ExecutionScopeDeletedError(executionId, deletedScope)
858858

859859
// Use edges directly - trigger-to-trigger edges are prevented at creation time

‎packages/testing/src/mocks/logging-session.mock.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ import { vi } from 'vitest'
77
* mock state. `mockSafeStart` defaults to `true` because callers branch on the
88
* boolean result. Display projection methods (incl. `projectTraceSpansForLiveDisplay`) return
99
* their input, diagnostic projection fails closed to structural metadata, `hasCompleted` is
10-
* `false`, `getPersistedCompletionStatus` and `getDeletedScope` are `null`, and other methods resolve to `undefined`.
10+
* `false`, `getPersistedCompletionStatus` and `getDeletedScope` are `null`, and other methods
11+
* resolve to `undefined`.
1112
* Defaults are `vi.fn(impl)`, so `mockReset()` restores them.
1213
*
1314
* @example

0 commit comments

Comments
 (0)