Skip to content

Commit df55288

Browse files
author
test
committed
fix(async): retry failed error finalization
Allow error finalization to retry after a non-error completion and fallback both fail, and always persist failed/error semantics for completeWithError.
1 parent a8e6e0a commit df55288

File tree

2 files changed

+67
-2
lines changed

2 files changed

+67
-2
lines changed

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

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,13 @@ describe('LoggingSession completion retries', () => {
5858
vi.clearAllMocks()
5959
})
6060

61-
it('keeps completion best-effort when full completion and fallback both fail', async () => {
61+
it('keeps completion best-effort when a later error completion retries after full completion and fallback both fail', async () => {
6262
const session = new LoggingSession('workflow-1', 'execution-1', 'api', 'req-1')
6363

6464
completeWorkflowExecutionMock
6565
.mockRejectedValueOnce(new Error('success finalize failed'))
6666
.mockRejectedValueOnce(new Error('cost only failed'))
67+
.mockResolvedValueOnce({})
6768

6869
await expect(session.safeComplete({ finalOutput: { ok: true } })).resolves.toBeUndefined()
6970

@@ -73,7 +74,8 @@ describe('LoggingSession completion retries', () => {
7374
})
7475
).resolves.toBeUndefined()
7576

76-
expect(completeWorkflowExecutionMock).toHaveBeenCalledTimes(2)
77+
expect(completeWorkflowExecutionMock).toHaveBeenCalledTimes(3)
78+
expect(session.hasCompleted()).toBe(true)
7779
})
7880

7981
it('reuses the settled completion promise for repeated completion attempts', async () => {
@@ -90,6 +92,66 @@ describe('LoggingSession completion retries', () => {
9092
expect(completeWorkflowExecutionMock).toHaveBeenCalledTimes(2)
9193
})
9294

95+
it('starts a new error completion attempt after a non-error completion and fallback both fail', async () => {
96+
const session = new LoggingSession('workflow-1', 'execution-3', 'api', 'req-1')
97+
98+
completeWorkflowExecutionMock
99+
.mockRejectedValueOnce(new Error('success finalize failed'))
100+
.mockRejectedValueOnce(new Error('cost only failed'))
101+
.mockResolvedValueOnce({})
102+
103+
await expect(session.safeComplete({ finalOutput: { ok: true } })).resolves.toBeUndefined()
104+
105+
await expect(
106+
session.safeCompleteWithError({
107+
error: { message: 'late error finalize' },
108+
})
109+
).resolves.toBeUndefined()
110+
111+
expect(completeWorkflowExecutionMock).toHaveBeenCalledTimes(3)
112+
expect(completeWorkflowExecutionMock).toHaveBeenLastCalledWith(
113+
expect.objectContaining({
114+
executionId: 'execution-3',
115+
finalOutput: { error: 'late error finalize' },
116+
})
117+
)
118+
expect(session.hasCompleted()).toBe(true)
119+
})
120+
121+
it('persists failed error semantics when completeWithError receives non-error trace spans', async () => {
122+
const session = new LoggingSession('workflow-1', 'execution-4', 'api', 'req-1')
123+
const traceSpans = [
124+
{
125+
id: 'span-1',
126+
name: 'Block A',
127+
type: 'tool',
128+
duration: 25,
129+
startTime: '2026-03-13T10:00:00.000Z',
130+
endTime: '2026-03-13T10:00:00.025Z',
131+
status: 'success',
132+
},
133+
]
134+
135+
completeWorkflowExecutionMock.mockResolvedValue({})
136+
137+
await expect(
138+
session.safeCompleteWithError({
139+
error: { message: 'persist me as failed' },
140+
traceSpans,
141+
})
142+
).resolves.toBeUndefined()
143+
144+
expect(completeWorkflowExecutionMock).toHaveBeenCalledWith(
145+
expect.objectContaining({
146+
executionId: 'execution-4',
147+
finalOutput: { error: 'persist me as failed' },
148+
traceSpans,
149+
level: 'error',
150+
status: 'failed',
151+
})
152+
)
153+
})
154+
93155
it('marks paused completions as completed and deduplicates later attempts', async () => {
94156
const session = new LoggingSession('workflow-1', 'execution-1', 'api', 'req-1')
95157

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,8 @@ export class LoggingSession {
409409
costSummary,
410410
finalOutput: { error: message },
411411
traceSpans: spans,
412+
level: 'error',
413+
status: 'failed',
412414
})
413415

414416
this.completed = true
@@ -919,6 +921,7 @@ export class LoggingSession {
919921
)
920922
} catch (fallbackError) {
921923
this.completing = false
924+
this.completionAttemptFailed = true
922925
logger.error(
923926
`[${this.requestId || 'unknown'}] Cost-only fallback also failed for execution ${this.executionId}:`,
924927
{ error: fallbackError instanceof Error ? fallbackError.message : String(fallbackError) }

0 commit comments

Comments
 (0)