Skip to content

Commit 08f54ef

Browse files
committed
fix(logs): preserve markers when the completion read fails
getProgressMarkers now returns null on a Redis read error (vs {} for genuinely empty). completeWorkflowExecution and markExecutionAsFailed skip clearProgressMarkers when the read returns null, so a transient read error at completion no longer wipes markers that are still durably in Redis — the TTL reclaims them instead.
1 parent 27afbce commit 08f54ef

6 files changed

Lines changed: 25 additions & 9 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -749,7 +749,7 @@ export class ExecutionLogger implements IExecutionLoggerService {
749749

750750
const builtExecutionData = this.buildCompletedExecutionData({
751751
existingExecutionData,
752-
progressMarkers,
752+
progressMarkers: progressMarkers ?? undefined,
753753
traceSpans: mergedTraceSpans,
754754
finalOutput,
755755
finalizationPath,
@@ -910,7 +910,7 @@ export class ExecutionLogger implements IExecutionLoggerService {
910910
return log
911911
})
912912

913-
void clearProgressMarkers(executionId)
913+
if (progressMarkers !== null) void clearProgressMarkers(executionId)
914914

915915
try {
916916
// Skip workflow lookup if workflow was deleted.

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -699,6 +699,12 @@ describe('LoggingSession.markExecutionAsFailed workflowId scoping', () => {
699699
expect(folded).toContain('lastCompletedBlock')
700700
expect(clearProgressMarkersMock).toHaveBeenCalledWith('exec-9')
701701
})
702+
703+
it('does not clear markers when the Redis read fails (avoids wiping the only copy)', async () => {
704+
getProgressMarkersMock.mockResolvedValueOnce(null)
705+
await LoggingSession.markExecutionAsFailed('exec-readfail', 'boom', undefined, 'wf-x')
706+
expect(clearProgressMarkersMock).not.toHaveBeenCalled()
707+
})
702708
})
703709

704710
describe('LoggingSession progress-marker write path', () => {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,10 +1059,10 @@ export class LoggingSession {
10591059
ARRAY['finalizationPath'],
10601060
to_jsonb('force_failed'::text)
10611061
)`
1062-
if (markers.lastStartedBlock) {
1062+
if (markers?.lastStartedBlock) {
10631063
executionData = sql`jsonb_set(${executionData}, ARRAY['lastStartedBlock'], ${JSON.stringify(markers.lastStartedBlock)}::jsonb)`
10641064
}
1065-
if (markers.lastCompletedBlock) {
1065+
if (markers?.lastCompletedBlock) {
10661066
executionData = sql`jsonb_set(${executionData}, ARRAY['lastCompletedBlock'], ${JSON.stringify(markers.lastCompletedBlock)}::jsonb)`
10671067
}
10681068

@@ -1076,7 +1076,7 @@ export class LoggingSession {
10761076
)
10771077
)
10781078

1079-
void clearProgressMarkers(executionId)
1079+
if (markers !== null) void clearProgressMarkers(executionId)
10801080

10811081
logger.info(`[${requestId || 'unknown'}] Marked execution ${executionId} as failed`)
10821082
} catch (error) {

apps/sim/lib/logs/execution/progress-markers.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,11 @@ describe('progress-markers', () => {
151151
expect(await getProgressMarkers(EXECUTION_ID)).toEqual({})
152152
expect(mockRedis.hgetall).not.toHaveBeenCalled()
153153
})
154+
155+
it('returns null when the Redis read fails so callers do not clear the only copy', async () => {
156+
mockRedis.hgetall.mockRejectedValueOnce(new Error('redis down'))
157+
expect(await getProgressMarkers(EXECUTION_ID)).toBeNull()
158+
})
154159
})
155160

156161
describe('clearProgressMarkers', () => {

apps/sim/lib/logs/execution/progress-markers.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,14 @@ function parseCompletedMarker(raw: string | undefined): ExecutionLastCompletedBl
176176

177177
/**
178178
* Read both markers for an execution. Returns an empty object when Redis is
179-
* unavailable, the key has expired, or nothing has been written yet.
179+
* unavailable (markers were never stored here) or the key holds nothing, and
180+
* `null` when the Redis read itself failed — callers must treat `null` as
181+
* "unknown" and skip {@link clearProgressMarkers}, so a transient read error
182+
* never wipes the only copy of markers that are still in Redis.
180183
*/
181-
export async function getProgressMarkers(executionId: string): Promise<ExecutionProgressMarkers> {
184+
export async function getProgressMarkers(
185+
executionId: string
186+
): Promise<ExecutionProgressMarkers | null> {
182187
const redis = getMarkerClient()
183188
if (!redis) return {}
184189

@@ -196,7 +201,7 @@ export async function getProgressMarkers(executionId: string): Promise<Execution
196201
logger.error(`Failed to read progress markers for execution ${executionId}`, {
197202
error: toError(error).message,
198203
})
199-
return {}
204+
return null
200205
}
201206
}
202207

apps/sim/lib/logs/fetch-log-detail.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ export async function fetchLogDetail({
172172

173173
const liveMarkers =
174174
log.status === 'running' || log.status === 'pending'
175-
? await getProgressMarkers(log.executionId)
175+
? ((await getProgressMarkers(log.executionId)) ?? {})
176176
: {}
177177

178178
return {

0 commit comments

Comments
 (0)