From 588317e0a441da2edfae714fa0e1d7fbb3865503 Mon Sep 17 00:00:00 2001 From: Daria Date: Wed, 26 Aug 2026 07:31:23 +0000 Subject: [PATCH] fix(editor): Stop autosave retries on permanent errors (#36967) --- .../app/composables/useWorkflowSaving.test.ts | 219 ++++++++++++++++++ .../src/app/composables/useWorkflowSaving.ts | 168 +++++++++++--- 2 files changed, 355 insertions(+), 32 deletions(-) diff --git a/packages/frontend/editor-ui/src/app/composables/useWorkflowSaving.test.ts b/packages/frontend/editor-ui/src/app/composables/useWorkflowSaving.test.ts index aa01455dc80..272a8bc525f 100644 --- a/packages/frontend/editor-ui/src/app/composables/useWorkflowSaving.test.ts +++ b/packages/frontend/editor-ui/src/app/composables/useWorkflowSaving.test.ts @@ -18,7 +18,9 @@ import { useWorkflowsListStore } from '@/app/stores/workflowsList.store'; import { useWorkflowSaveStore } from '@/app/stores/workflowSave.store'; import { useBackendConnectionStore } from '@/app/stores/backendConnection.store'; import { useSettingsStore } from '@n8n/stores/settings.store'; +import { useFocusPanelStore } from '@/app/stores/focusPanel.store'; import type { WorkflowDataUpdate } from '@n8n/rest-api-client/api/workflows'; +import { ResponseError } from '@n8n/rest-api-client'; import { mockedStore } from '@/__tests__/utils'; import { createTestNode, createTestWorkflow, mockNodeTypeDescription } from '@/__tests__/mocks'; import { CHAT_TRIGGER_NODE_TYPE, NodeConnectionTypes } from 'n8n-workflow'; @@ -1420,6 +1422,223 @@ describe('useWorkflowSaving', () => { } }); + it('retries autosaved create failures that can recover', async () => { + vi.useFakeTimers(); + + try { + const newWorkflowId = 'w-autosave-create-failure'; + const errorMessage = 'Network timeout'; + const workflow = createTestWorkflow({ + id: newWorkflowId, + name: 'Named new workflow', + nodes: [createTestNode({ type: CHAT_TRIGGER_NODE_TYPE, disabled: false })], + }); + + vi.spyOn(workflowsStore, 'createNewWorkflow').mockRejectedValue(new Error(errorMessage)); + + mockRoute.params = { workflowId: newWorkflowId }; + useWorkflowDocumentStore(createWorkflowDocumentId(newWorkflowId)).hydrate(workflow); + + const saveStore = useWorkflowSaveStore(); + const { saveCurrentWorkflow } = useWorkflowSaving({ + router, + }); + + const result = await saveCurrentWorkflow({}, true, false, true); + + expect(result).toBe(false); + expect(saveStore.pendingSave).toBeNull(); + expect(saveStore.retryCount).toBe(1); + expect(saveStore.lastError).toBe(errorMessage); + expect(saveStore.isRetrying).toBe(true); + expect(showMessageSpy).toHaveBeenCalledTimes(1); + expect(showMessageSpy).toHaveBeenCalledWith( + expect.objectContaining({ + message: expect.stringContaining(errorMessage), + type: 'error', + }), + ); + } finally { + vi.useRealTimers(); + } + }); + + it('does not retry autosaved create when a post-create step fails', async () => { + vi.useFakeTimers(); + const newWorkflowId = 'w-autosave-create-post-create-failure'; + const createdWorkflow = createTestWorkflow({ + id: 'w-autosave-created-before-post-create-failure', + name: 'Named new workflow', + nodes: [createTestNode({ type: CHAT_TRIGGER_NODE_TYPE, disabled: false })], + }); + const createSpy = vi + .spyOn(workflowsStore, 'createNewWorkflow') + .mockResolvedValue(createdWorkflow); + const focusPanelSpy = vi + .spyOn(useFocusPanelStore(), 'onNewWorkflowSave') + .mockImplementation(() => { + throw new Error('Focus panel failed'); + }); + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + try { + mockRoute.params = { workflowId: newWorkflowId }; + useWorkflowDocumentStore(createWorkflowDocumentId(newWorkflowId)).hydrate( + createTestWorkflow({ + id: newWorkflowId, + name: 'Named new workflow', + nodes: [createTestNode({ type: CHAT_TRIGGER_NODE_TYPE, disabled: false })], + }), + ); + + const saveStore = useWorkflowSaveStore(); + const { saveCurrentWorkflow } = useWorkflowSaving({ + router, + }); + + const result = await saveCurrentWorkflow({}, true, false, true); + await vi.advanceTimersByTimeAsync(saveStore.getRetryDelay() + 1); + + expect(result).toBe(false); + expect(createSpy).toHaveBeenCalledTimes(1); + expect(workflowsListStore.getWorkflowById(createdWorkflow.id)).toEqual(createdWorkflow); + expect(saveStore.retryCount).toBe(0); + expect(saveStore.isRetrying).toBe(false); + expect(saveStore.lastError).toBeNull(); + expect(consoleErrorSpy).toHaveBeenCalledWith(expect.any(Error)); + } finally { + focusPanelSpy.mockRestore(); + consoleErrorSpy.mockRestore(); + vi.useRealTimers(); + } + }); + + it('does not retry autosave after a permanent client error', async () => { + const workflow = createTestWorkflow({ + id: 'w-autosave-client-error', + nodes: [createTestNode({ type: CHAT_TRIGGER_NODE_TYPE, disabled: false })], + active: true, + }); + const errorMessage = 'Workflow name is required'; + + vi.spyOn(workflowsListStore, 'fetchWorkflow').mockResolvedValue(workflow); + vi.spyOn(workflowsStore, 'updateWorkflow').mockRejectedValue( + new ResponseError(errorMessage, { httpStatusCode: 400 }), + ); + + workflowsStore.setWorkflowId(workflow.id); + useWorkflowDocumentStore(createWorkflowDocumentId(workflow.id)).hydrate(workflow); + workflowsListStore.workflowsById = { [workflow.id]: workflow }; + + const saveStore = useWorkflowSaveStore(); + const { saveCurrentWorkflow } = useWorkflowSaving({ + router, + }); + + const result = await saveCurrentWorkflow({ id: workflow.id }, true, false, true); + + expect(result).toBe(false); + expect(saveStore.pendingSave).toBeNull(); + expect(saveStore.retryCount).toBe(0); + expect(saveStore.isRetrying).toBe(false); + expect(saveStore.lastError).toBe(errorMessage); + }); + + it('uses raw object messages for autosave errors that are not Error instances', async () => { + const { workflow } = prepareHydratedWorkflow('w-autosave-raw-node-api-error'); + const errorMessage = 'Node API request failed'; + + vi.spyOn(workflowsStore, 'updateWorkflow').mockRejectedValue({ + name: 'NodeApiError', + message: errorMessage, + errorCode: 400, + }); + + const saveStore = useWorkflowSaveStore(); + const { saveCurrentWorkflow } = useWorkflowSaving({ + router, + }); + + const result = await saveCurrentWorkflow({ id: workflow.id }, true, false, true); + + expect(result).toBe(false); + expect(saveStore.retryCount).toBe(0); + expect(saveStore.isRetrying).toBe(false); + expect(saveStore.lastError).toBe(errorMessage); + expect(showMessageSpy).toHaveBeenCalledWith( + expect.objectContaining({ + message: errorMessage, + type: 'error', + }), + ); + }); + + it('does not retry autosave after a raw axios permanent client error', async () => { + const { workflow } = prepareHydratedWorkflow('w-autosave-raw-axios-error'); + const errorMessage = 'Request failed with status code 413'; + + vi.spyOn(workflowsStore, 'updateWorkflow').mockRejectedValue( + Object.assign(new Error(errorMessage), { + response: { + status: 413, + }, + }), + ); + + const saveStore = useWorkflowSaveStore(); + const { saveCurrentWorkflow } = useWorkflowSaving({ + router, + }); + + const result = await saveCurrentWorkflow({ id: workflow.id }, true, false, true); + + expect(result).toBe(false); + expect(saveStore.retryCount).toBe(0); + expect(saveStore.isRetrying).toBe(false); + expect(saveStore.lastError).toBe(errorMessage); + }); + + it('does not immediately re-arm debounced autosave after a permanent client error', async () => { + vi.useFakeTimers(); + + try { + mockedStore(useSettingsStore).isAutosaveEnabled = true; + prepareHydratedWorkflow('w-autosave-client-error-debounced'); + const updateSpy = vi + .spyOn(workflowsStore, 'updateWorkflow') + .mockRejectedValue( + new ResponseError('Workflow name is required', { httpStatusCode: 400 }), + ); + const saveStore = useWorkflowSaveStore(); + const uiStore = useUIStore(); + + uiStore.markStateDirty(); + + const { autoSaveWorkflow } = useWorkflowSaving({ router, ownsAutoSave: true }); + + autoSaveWorkflow(); + expect(saveStore.autoSaveState).toBe(AutoSaveState.Scheduled); + + await vi.advanceTimersByTimeAsync( + getDebounceTime(DEBOUNCE_TIME.API.AUTOSAVE_MAX_WAIT) + 1000, + ); + + expect(updateSpy).toHaveBeenCalledTimes(1); + expect(saveStore.autoSaveState).toBe(AutoSaveState.Idle); + expect(saveStore.retryCount).toBe(0); + expect(saveStore.isRetrying).toBe(false); + expect(uiStore.stateIsDirty).toBe(true); + + await vi.advanceTimersByTimeAsync( + getDebounceTime(DEBOUNCE_TIME.API.AUTOSAVE_MAX_WAIT) + 1000, + ); + + expect(updateSpy).toHaveBeenCalledTimes(1); + } finally { + vi.useRealTimers(); + } + }); + it('should not schedule autosave when network is offline', () => { prepareHydratedWorkflow('w-offline'); const saveStore = useWorkflowSaveStore(); diff --git a/packages/frontend/editor-ui/src/app/composables/useWorkflowSaving.ts b/packages/frontend/editor-ui/src/app/composables/useWorkflowSaving.ts index 018f34f958c..bc22caf296f 100644 --- a/packages/frontend/editor-ui/src/app/composables/useWorkflowSaving.ts +++ b/packages/frontend/editor-ui/src/app/composables/useWorkflowSaving.ts @@ -21,6 +21,7 @@ import { useSourceControlStore } from '@/features/integrations/sourceControl.ee/ import { useCanvasStore } from '@/app/stores/canvas.store'; import type { IUpdateInformation, IWorkflowDb } from '@/Interface'; import type { WorkflowDataCreate, WorkflowDataUpdate } from '@n8n/rest-api-client/api/workflows'; +import { ResponseError } from '@n8n/rest-api-client'; import { isExpression, type IDataObject } from 'n8n-workflow'; import { useToast } from '@n8n/composables/useToast'; import { useExternalHooks } from './useExternalHooks'; @@ -43,6 +44,64 @@ import { useBackendConnectionStore } from '@/app/stores/backendConnection.store' import { useSettingsStore } from '@n8n/stores/settings.store'; import { useInvalidNodeGroupCleanup } from '@/app/composables/useInvalidNodeGroupCleanup'; +function getErrorMessage(error: unknown): string { + if (error instanceof Error) { + return error.message; + } + + if (error && typeof error === 'object' && 'message' in error) { + const { message } = error as { message?: unknown }; + if (message !== undefined) { + return String(message); + } + } + + return String(error); +} + +function getHttpStatusCode(error: unknown): number | undefined { + if (error instanceof ResponseError) { + return error.httpStatusCode; + } + + if (!error || typeof error !== 'object') { + return; + } + + const { httpStatusCode, errorCode, response } = error as { + httpStatusCode?: unknown; + errorCode?: unknown; + response?: unknown; + }; + + if (typeof httpStatusCode === 'number') { + return httpStatusCode; + } + + if (response && typeof response === 'object') { + const { status } = response as { status?: unknown }; + if (typeof status === 'number') { + return status; + } + } + + if (typeof errorCode === 'number' && errorCode >= 400 && errorCode < 600) { + return errorCode; + } + + return undefined; +} + +function shouldRetryAutoSaveFailure(error: unknown): boolean { + const statusCode = getHttpStatusCode(error); + + if (!statusCode) { + return true; + } + + return [408, 409, 429].includes(statusCode) || statusCode >= 500; +} + export function useWorkflowSaving({ router, onSaved, @@ -90,7 +149,8 @@ export function useWorkflowSaving({ const currentWorkflowDocumentStore = computed(() => useWorkflowDocumentStore(createWorkflowDocumentId(workflowId.value)), ); - const canScheduleAutoSave = computed(() => { + + const canArmAutoSave = computed(() => { // Don't schedule from a read-only canvas, or from an instance that doesn't // own one. Every autosave entry point funnels through here, so a preview // never writes whatever marked it dirty. @@ -103,6 +163,23 @@ export function useWorkflowSaving({ return false; } + // Don't schedule if we're offline + if (!backendConnectionStore.isOnline) { + return false; + } + + if (!currentWorkflowDocumentStore.value.hydrated) { + return false; + } + + return true; + }); + + const canScheduleAutoSave = computed(() => { + if (!canArmAutoSave.value) { + return false; + } + // Don't schedule if a save is already in progress - the finally block // will reschedule if there are pending changes if (saveStore.pendingSave) { @@ -114,15 +191,6 @@ export function useWorkflowSaving({ return false; } - // Don't schedule if we're offline - if (!backendConnectionStore.isOnline) { - return false; - } - - if (!currentWorkflowDocumentStore.value.hydrated) { - return false; - } - return true; }); @@ -241,20 +309,23 @@ export function useWorkflowSaving({ } const savePromise = (async (): Promise => { - // Check if workflow needs to be saved as new (doesn't exist in store yet) - const existingWorkflow = currentWorkflow - ? workflowsListStore.getWorkflowById(currentWorkflow) - : null; - if (!currentWorkflow || !existingWorkflow?.id) { - const workflowId = await saveAsNewWorkflow( - { parentFolderId, uiContext, autosaved }, - redirect, - ); - return !!workflowId; - } + let isExistingWorkflowSave = false; - // Workflow exists already so update it try { + // Check if workflow needs to be saved as new (doesn't exist in store yet) + const existingWorkflow = currentWorkflow + ? workflowsListStore.getWorkflowById(currentWorkflow) + : null; + if (!currentWorkflow || !existingWorkflow?.id) { + const workflowId = await saveAsNewWorkflow( + { parentFolderId, uiContext, autosaved }, + redirect, + ); + return !!workflowId; + } + + isExistingWorkflowSave = true; + // Workflow exists already so update it if (!forceSave && isLoading) { return true; } @@ -320,9 +391,10 @@ export function useWorkflowSaving({ onSaved?.(false); // Update of existing workflow return true; } catch (error) { + const errorMessage = getErrorMessage(error); console.error(error); - if (error.errorCode === 409) { + if (isExistingWorkflowSave && getHttpStatusCode(error) === 409) { telemetry.track('User attempted to save locked workflow', { workflowId: currentWorkflow, sharing_role: getWorkflowProjectRole(currentWorkflow), @@ -371,8 +443,20 @@ export function useWorkflowSaving({ // Handle autosave failures with exponential backoff if (autosaved) { + if (!shouldRetryAutoSaveFailure(error)) { + saveStore.resetRetry(); + saveStore.setLastError(errorMessage); + toast.showMessage({ + title: i18n.baseText('workflowHelpers.showMessage.title'), + message: errorMessage, + type: 'error', + }); + + return false; + } + saveStore.incrementRetry(); - saveStore.setLastError(error.message); + saveStore.setLastError(errorMessage); // Schedule retry with exponential backoff const retryDelay = saveStore.getRetryDelay(); @@ -390,7 +474,7 @@ export function useWorkflowSaving({ title: i18n.baseText('workflowHelpers.showMessage.title'), message: i18n.baseText('generic.autosave.retrying', { interpolate: { - error: error.message, + error: errorMessage, retryIn: `${Math.ceil(retryDelay / 1000)}s`, }, }), @@ -403,7 +487,7 @@ export function useWorkflowSaving({ toast.showMessage({ title: i18n.baseText('workflowHelpers.showMessage.title'), - message: error.message, + message: errorMessage, type: 'error', }); @@ -449,6 +533,8 @@ export function useWorkflowSaving({ } = {}, redirect = true, ): Promise { + let createRequestFailed = false; + try { const currentDocumentStore = useWorkflowDocumentStore( createWorkflowDocumentId(workflowId.value), @@ -527,7 +613,13 @@ export function useWorkflowSaving({ workflowDataRequest.autosaved = autosaved; } - const workflowData = await workflowsStore.createNewWorkflow(workflowDataRequest); + let workflowData: IWorkflowDb; + try { + workflowData = await workflowsStore.createNewWorkflow(workflowDataRequest); + } catch (e) { + createRequestFailed = true; + throw e; + } workflowsListStore.addWorkflow(workflowData); @@ -622,9 +714,20 @@ export function useWorkflowSaving({ onSaved?.(true); // First save of new workflow return workflowData.id; } catch (e) { + if (autosaved && createRequestFailed) { + throw e; + } + + if (autosaved) { + // The create request already succeeded; retrying this autosave + // would POST the same new workflow again. + console.error(e); + return null; + } + toast.showMessage({ title: i18n.baseText('workflowHelpers.showMessage.title'), - message: (e as Error).message, + message: getErrorMessage(e), type: 'error', }); @@ -647,14 +750,15 @@ export function useWorkflowSaving({ saveStore.setAutoSaveState(AutoSaveState.InProgress); void (async () => { + let saved = false; try { - await saveCurrentWorkflow({}, true, false, true); + saved = await saveCurrentWorkflow({}, true, false, true); } finally { if (saveStore.autoSaveState === AutoSaveState.InProgress) { saveStore.setAutoSaveState(AutoSaveState.Idle); } // If changes were made during save, reschedule autosave - if (uiStore.stateIsDirty && canScheduleAutoSave.value) { + if (saved && uiStore.stateIsDirty && canScheduleAutoSave.value) { saveStore.setAutoSaveState(AutoSaveState.Scheduled); void autoSaveWorkflowDebounced(); } @@ -692,8 +796,8 @@ export function useWorkflowSaving({ // else would save what the agent wrote. Mirrors the AI-builder re-arm in // NodeView. // Watch for network coming back online, and for other autosave eligibility - // returning after retry backoff, save completion, or document hydration. - watch(canScheduleAutoSave, (allowed, wasAllowed) => { + // returning after document hydration. + watch(canArmAutoSave, (allowed, wasAllowed) => { if (allowed && !wasAllowed && uiStore.stateIsDirty) { scheduleAutoSave(); }