opensource/n8n
3
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2026-09-24 23:22:38 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(editor): Harden credential modal against async hangs (#31292)

Browse Source 
Co-authored-by: yehorkardash <yehor.kardash@n8n.io>
This commit is contained in:
Srinjoy
2026-06-11 07:08:56 +00:00
committed by GitHub
co-authored by yehorkardash
parent f9496c2397
commit 037bdb216f
2 changed files with 185 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/frontend/editor-ui/src/features/credentials/components/CredentialEdit/CredentialEdit.test.ts
+94
View File
@@ -247,6 +247,17 @@ const renderComponent = createComponentRenderer(CredentialEdit, {
}),
});
const modalLoadingStub = {
props: ['loading'],
template: `
<div data-test-id="credential-edit-modal-stub" :data-loading="String(loading)">
<slot v-if="!loading" name="header" />
<slot v-if="!loading" name="content" />
<slot v-if="!loading" name="footer" />
</div>
`,
};
let broadcastMessageListener: ((event: MessageEvent) => void) | undefined;
class BroadcastChannelMock {
@@ -498,12 +509,50 @@ describe('CredentialEdit', () => {
const { getByTestId } = renderComponent({
props: { modalName: CREDENTIAL_EDIT_MODAL_KEY, mode: 'new' },
global: {
stubs: {
Modal: modalLoadingStub,
},
},
});
await waitFor(() => {
expect(getByTestId('credential-edit-modal-stub')).toHaveAttribute('data-loading', 'false');
expect(getByTestId('credential-edit-dialog')).toBeInTheDocument();
});
});
it('should stop loading when credential loading fails', async () => {
const credentialsStore = mockedStore(useCredentialsStore);
credentialsStore.getCredentialData.mockRejectedValueOnce(new Error('Failed to load'));
const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
const { getByTestId } = renderComponent({
props: {
activeId: 'missing-credential',
modalName: CREDENTIAL_EDIT_MODAL_KEY,
mode: 'edit',
},
global: {
stubs: {
Modal: modalLoadingStub,
},
},
});
try {
await waitFor(() => {
expect(credentialsStore.getCredentialData).toHaveBeenCalled();
expect(getByTestId('credential-edit-modal-stub')).toHaveAttribute(
'data-loading',
'false',
);
});
} finally {
consoleErrorSpy.mockRestore();
}
});
});
describe('managed credential scope hiding', () => {
@@ -711,6 +760,51 @@ describe('CredentialEdit', () => {
await retry(() => expect(queryByText('Custom Scopes')).toBeInTheDocument());
expect(queryByText('Enabled Scopes')).toBeInTheDocument();
});
it('should not block modal when external hooks throw', async () => {
window.n8nExternalHooks = {
credentialsEdit: {
credentialModalOpened: [
() => {
throw new Error('plugin error');
},
],
},
};
const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
const { getByTestId } = renderComponent({
props: { modalName: CREDENTIAL_EDIT_MODAL_KEY, mode: 'new' },
global: {
stubs: {
Modal: modalLoadingStub,
},
},
});
try {
// Wait for the modal to appear and loading to finish
await waitFor(() => {
expect(getByTestId('credential-edit-modal-stub')).toHaveAttribute(
'data-loading',
'false',
);
expect(getByTestId('credential-edit-dialog')).toBeInTheDocument();
});
// The hook error is logged asynchronously; wait for it
await waitFor(() => {
expect(consoleErrorSpy).toHaveBeenCalledWith(
'[CredentialEdit] External hooks execution failed',
expect.any(Error),
);
});
} finally {
consoleErrorSpy.mockRestore();
delete window.n8nExternalHooks;
}
});
});
test('should use the requested credential type when node has multiple credential types', async () => {
packages/frontend/editor-ui/src/features/credentials/components/CredentialEdit/CredentialEdit.vue
+91 -80
View File
@@ -491,89 +491,101 @@ watch(
);
onMounted(async () => {
const modalState = uiStore.modalsById[CREDENTIAL_EDIT_MODAL_KEY];
requiredCredentials.value =
isCredentialModalState(modalState) && modalState.showAuthSelector === true;
// Inner try isolates optional secrets loading; outer try catches all other initialization failures.
try {
const modalState = uiStore.modalsById[CREDENTIAL_EDIT_MODAL_KEY];
requiredCredentials.value =
isCredentialModalState(modalState) && modalState.showAuthSelector === true;
const forceManual = isCredentialModalState(modalState) && modalState.forceManualMode === true;
const forceManual = isCredentialModalState(modalState) && modalState.forceManualMode === true;
const overrideProjectId = isCredentialModalState(modalState) ? modalState.projectId : undefined;
const projectId =
overrideProjectId ?? projectsStore.currentProjectId ?? projectsStore.personalProject?.id;
if (projectId) {
try {
await externalSecretsStore.fetchSecretsForProject(projectId);
} catch {
// Secrets fetch failure should not block the credential modal
}
}
if (props.mode === 'new' && credentialTypeName.value) {
const modalSuggestedName = isCredentialModalState(modalState)
? modalState.suggestedName
: undefined;
credentialName.value = modalSuggestedName
? modalSuggestedName
: await credentialsStore.getNewCredentialName({
credentialTypeName: defaultCredentialTypeName.value,
});
credentialData.value = {
...credentialData.value,
...(homeProject.value ? { homeProject: homeProject.value } : {}),
};
} else {
await loadCurrentCredential();
}
setCredentialPropertyDefaults();
// Detect if existing credential uses custom OAuth (user-provided clientId/clientSecret).
// Use __overwrittenProperties directly instead of managedOAuthAvailable so that skip-list
// types (where managedOAuthAvailable is false) still auto-detect custom credentials.
if (
credentialType.value?.__overwrittenProperties?.includes('clientId') &&
credentialData.value.clientId &&
credentialData.value.clientSecret
) {
useCustomOAuth.value = true;
}
// Default to quick connect mode for new credentials when available and not forced to manual
if (
props.mode === 'new' &&
!forceManual &&
credentialTypeName.value &&
ndvStore.value.activeNode
) {
const qcOption = getQuickConnectOption(
credentialTypeName.value,
ndvStore.value.activeNode.type,
);
if (qcOption) {
isQuickConnectMode.value = true;
}
}
await externalHooks.run('credentialsEdit.credentialModalOpened', {
credentialType: credentialTypeName.value,
isEditingCredential: props.mode === 'edit',
activeNode: ndvStore.value.activeNode,
});
setTimeout(async () => {
if (credentialId.value) {
if (!requiredPropertiesFilled.value && credentialPermissions.value.update) {
// sharees can't see properties, so this check would always fail for them
// if the credential contains required fields.
showValidationWarning.value = true;
} else {
await retestCredential();
const overrideProjectId = isCredentialModalState(modalState) ? modalState.projectId : undefined;
const projectId =
overrideProjectId ?? projectsStore.currentProjectId ?? projectsStore.personalProject?.id;
if (projectId) {
try {
await externalSecretsStore.fetchSecretsForProject(projectId);
} catch {
// Secrets fetch failure should not block the credential modal
}
}
}, 0);
loading.value = false;
if (props.mode === 'new' && credentialTypeName.value) {
const modalSuggestedName = isCredentialModalState(modalState)
? modalState.suggestedName
: undefined;
credentialName.value = modalSuggestedName
? modalSuggestedName
: await credentialsStore.getNewCredentialName({
credentialTypeName: defaultCredentialTypeName.value,
});
credentialData.value = {
...credentialData.value,
...(homeProject.value ? { homeProject: homeProject.value } : {}),
};
} else {
// loadCurrentCredential handles its own recovery by showing an error toast and closing the modal.
// It should be allowed to propagate to avoid subsequent initialization steps running on empty data.
await loadCurrentCredential();
}
setCredentialPropertyDefaults();
// Detect if existing credential uses custom OAuth (user-provided clientId/clientSecret).
// Use __overwrittenProperties directly instead of managedOAuthAvailable so that skip-list
// types (where managedOAuthAvailable is false) still auto-detect custom credentials.
if (
credentialType.value?.__overwrittenProperties?.includes('clientId') &&
credentialData.value.clientId &&
credentialData.value.clientSecret
) {
useCustomOAuth.value = true;
}
// Default to quick connect mode for new credentials when available and not forced to manual
if (
props.mode === 'new' &&
!forceManual &&
credentialTypeName.value &&
ndvStore.value.activeNode
) {
const qcOption = getQuickConnectOption(
credentialTypeName.value,
ndvStore.value.activeNode.type,
);
if (qcOption) {
isQuickConnectMode.value = true;
}
}
// External hooks are fire-and-forget so slow or failing hooks cannot keep the modal loading.
void externalHooks
.run('credentialsEdit.credentialModalOpened', {
credentialType: credentialTypeName.value,
isEditingCredential: props.mode === 'edit',
activeNode: ndvStore.value.activeNode,
})
.catch((error) => {
console.error('[CredentialEdit] External hooks execution failed', error);
});
setTimeout(async () => {
if (credentialId.value) {
if (!requiredPropertiesFilled.value && credentialPermissions.value.update) {
// sharees can't see properties, so this check would always fail for them
// if the credential contains required fields.
showValidationWarning.value = true;
} else {
await retestCredential();
}
}
}, 0);
} catch (error) {
console.error('[CredentialEdit] Initialization error', error);
} finally {
loading.value = false;
}
});
async function beforeClose() {
@@ -729,8 +741,7 @@ async function loadCurrentCredential(id = props.activeId ?? '') {
i18n.baseText('credentialEdit.credentialEdit.showError.loadCredential.title'),
);
closeDialog();
return;
throw error;
}
}