mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-25 10:35:58 +08:00
fix(core): Do not report activation errors for credentials that need reconnecting (#41840)
This commit is contained in:
@@ -1,11 +1,15 @@
|
||||
import type { Logger } from '@n8n/backend-common';
|
||||
import type { ErrorEvent } from '@sentry/core';
|
||||
import { Container } from '@n8n/di';
|
||||
import { createDeferredPromise } from '@n8n/utils/promise/deferred-promise';
|
||||
import type {
|
||||
IAllExecuteFunctions,
|
||||
ICredentialDataDecryptedObject,
|
||||
INode,
|
||||
INodeExecutionData,
|
||||
ITriggerResponse,
|
||||
IWorkflowExecuteAdditionalData,
|
||||
JsonObject,
|
||||
Workflow,
|
||||
WorkflowActivateMode,
|
||||
WorkflowExecuteMode,
|
||||
@@ -14,20 +18,24 @@ import type {
|
||||
} from 'n8n-workflow';
|
||||
import {
|
||||
LoggerProxy,
|
||||
NodeApiError,
|
||||
NodeOperationError,
|
||||
TriggerCloseError,
|
||||
WorkflowActivationError,
|
||||
WorkflowDeactivationError,
|
||||
} from 'n8n-workflow';
|
||||
import nock from 'nock';
|
||||
import type { Mock } from 'vitest';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
import { mock, mockDeep } from 'vitest-mock-extended';
|
||||
|
||||
import type { ErrorReporter } from '@/errors/error-reporter';
|
||||
import { ErrorReporter } from '@/errors/error-reporter';
|
||||
import type { InstanceSettings } from '@/instance-settings';
|
||||
import { Tracing } from '@/observability';
|
||||
|
||||
import { ActiveWorkflowTriggers } from '../active-workflow-triggers';
|
||||
import type { IGetExecuteTriggerFunctions } from '../interfaces';
|
||||
import type { PollContext } from '../node-execution-context';
|
||||
import { refreshOAuth2Token } from '../node-execution-context/utils/request-helpers/oauth';
|
||||
import { NoOpPollJobManager } from '../noop-poll-job-manager';
|
||||
import { PollJobManager } from '../poll-job-manager';
|
||||
import { PollTriggerExecutor } from '../poll-trigger-executor';
|
||||
@@ -322,6 +330,100 @@ describe('ActiveWorkflowTriggers', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('activation error level', () => {
|
||||
beforeEach(() => {
|
||||
getPollFunctions.mockReturnValue(pollFunctions);
|
||||
});
|
||||
|
||||
const activationErrorFor = async (options: TestOptions): Promise<WorkflowActivationError> =>
|
||||
await addWorkflow(options).then(
|
||||
() => expect.fail('activation succeeded'),
|
||||
(e: WorkflowActivationError) => e,
|
||||
);
|
||||
|
||||
it('keeps level error for a poll activation error caused by a node error', async () => {
|
||||
const pollError = new NodeOperationError(pollNode, 'Sheet with ID 0 not found');
|
||||
|
||||
const error = await activationErrorFor({ pollNodes: [pollNode], pollError });
|
||||
|
||||
expect(error.level).toBe('error');
|
||||
});
|
||||
|
||||
it('sets level warning for a poll activation error tagged credential-invalid by the node', async () => {
|
||||
const pollError = new NodeApiError(pollNode, { httpCode: '401' });
|
||||
pollError.failure = { cause: 'credential-invalid' };
|
||||
|
||||
const error = await activationErrorFor({ pollNodes: [pollNode], pollError });
|
||||
|
||||
expect(error.level).toBe('warning');
|
||||
});
|
||||
|
||||
const credentialsType = 'googleDriveOAuth2Api';
|
||||
const oauthNode = mock<INode>({
|
||||
id: 'oauth-node',
|
||||
name: 'Google Drive Trigger',
|
||||
credentials: { [credentialsType]: { id: 'credential-id', name: 'Google Drive account' } },
|
||||
});
|
||||
|
||||
const revokedGrantError = async (): Promise<Error> => {
|
||||
const oauthAdditionalData = mockDeep<IWorkflowExecuteAdditionalData>();
|
||||
(oauthAdditionalData as unknown as Record<string, unknown>)['oauth-jwe'] = undefined;
|
||||
oauthAdditionalData.ssrfBridge = undefined;
|
||||
oauthAdditionalData.credentialsHelper.getDecrypted.mockResolvedValue({
|
||||
oauthTokenData: { access_token: 'old-token' },
|
||||
} as unknown as ICredentialDataDecryptedObject);
|
||||
|
||||
const context = mockDeep<IAllExecuteFunctions>();
|
||||
context.getCredentials.mockResolvedValue({
|
||||
clientId: 'client-id',
|
||||
clientSecret: 'client-secret',
|
||||
grantType: 'authorizationCode',
|
||||
authUrl: 'https://oauth.example.com/auth',
|
||||
accessTokenUrl: 'https://oauth.example.com/token',
|
||||
authentication: 'body',
|
||||
oauthTokenData: { access_token: 'old-token', refresh_token: 'old-refresh-token' },
|
||||
});
|
||||
nock('https://oauth.example.com').post('/token').reply(400, { error: 'invalid_grant' });
|
||||
|
||||
return await refreshOAuth2Token
|
||||
.call(context, credentialsType, oauthNode, oauthAdditionalData)
|
||||
.then(
|
||||
() => expect.fail('token refresh succeeded'),
|
||||
(e: Error) => e,
|
||||
);
|
||||
};
|
||||
|
||||
it('does not send a poll activation error caused by a revoked OAuth2 grant to Sentry', async () => {
|
||||
const pollError = await revokedGrantError();
|
||||
const reporter = new ErrorReporter(logger, tracing);
|
||||
|
||||
const error = await activationErrorFor({ pollNodes: [pollNode], pollError });
|
||||
|
||||
expect(
|
||||
await reporter.beforeSend({} as ErrorEvent, { originalException: error }),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('sets level warning when a node rewraps the revoked-grant error in NodeApiError', async () => {
|
||||
const pollError = new NodeApiError(
|
||||
pollNode,
|
||||
(await revokedGrantError()) as unknown as JsonObject,
|
||||
);
|
||||
|
||||
const error = await activationErrorFor({ pollNodes: [pollNode], pollError });
|
||||
|
||||
expect(error.level).toBe('warning');
|
||||
});
|
||||
|
||||
it('sets level warning on a trigger activation error caused by a revoked OAuth2 grant', async () => {
|
||||
const triggerError = await revokedGrantError();
|
||||
|
||||
const error = await activationErrorFor({ triggerNodes: [triggerNode], triggerError });
|
||||
|
||||
expect(error.level).toBe('warning');
|
||||
});
|
||||
});
|
||||
|
||||
describe('should roll back partial activation on failure', () => {
|
||||
it('closes already-registered trigger responses when a later trigger node fails', async () => {
|
||||
const firstResponse = mock<ITriggerResponse>();
|
||||
|
||||
@@ -1,7 +1,22 @@
|
||||
import { ExecutionBaseError } from './abstract/execution-base.error';
|
||||
import type { ErrorLevel } from '@n8n/errors';
|
||||
import { errorChain, isObjectLike } from '@n8n/utils/errors/error-chain';
|
||||
import type { ActionableCause } from './failure';
|
||||
import type { INode } from '../interfaces';
|
||||
|
||||
/** Declared failure causes that make an activation error a warning. */
|
||||
const WARNING_FAILURE_CAUSES: readonly ActionableCause[] = [
|
||||
'credential-invalid',
|
||||
'configuration-invalid',
|
||||
];
|
||||
|
||||
function hasWarningFailure(cause: Error | undefined): boolean {
|
||||
return errorChain(cause).some(
|
||||
({ failure }) =>
|
||||
isObjectLike(failure) && WARNING_FAILURE_CAUSES.some((warning) => warning === failure.cause),
|
||||
);
|
||||
}
|
||||
|
||||
interface WorkflowActivationErrorOptions {
|
||||
cause?: Error;
|
||||
node?: INode;
|
||||
@@ -47,15 +62,20 @@ export class WorkflowActivationError extends ExecutionBaseError {
|
||||
? cause.description
|
||||
: cause.message;
|
||||
}
|
||||
this.setLevel(level);
|
||||
this.setLevel(level, cause);
|
||||
}
|
||||
|
||||
private setLevel(level?: ErrorLevel) {
|
||||
private setLevel(level?: ErrorLevel, cause?: Error) {
|
||||
if (level) {
|
||||
this.level = level;
|
||||
return;
|
||||
}
|
||||
|
||||
if (hasWarningFailure(cause)) {
|
||||
this.level = 'warning';
|
||||
return;
|
||||
}
|
||||
|
||||
if (
|
||||
[
|
||||
'etimedout', // Node.js
|
||||
|
||||
@@ -1,4 +1,11 @@
|
||||
import { WorkflowActivationError, WorkflowOperationError } from '../../src/errors';
|
||||
import { mock } from 'vitest-mock-extended';
|
||||
|
||||
import {
|
||||
NodeOperationError,
|
||||
WorkflowActivationError,
|
||||
WorkflowOperationError,
|
||||
} from '../../src/errors';
|
||||
import type { INode } from '../../src/interfaces';
|
||||
|
||||
describe('WorkflowActivationError', () => {
|
||||
it('should default to `error` level', () => {
|
||||
@@ -45,6 +52,63 @@ describe('WorkflowActivationError', () => {
|
||||
expect(error.description).toBe('Actionable detail');
|
||||
});
|
||||
|
||||
describe('level from the failure the cause declares', () => {
|
||||
const node = mock<INode>();
|
||||
|
||||
it('should keep level error for a node error cause without a declared failure', () => {
|
||||
const nodeCause = new NodeOperationError(node, 'Sheet with ID 0 not found');
|
||||
|
||||
const error = new WorkflowActivationError('activation failed', { cause: nodeCause });
|
||||
|
||||
expect(error.level).toBe('error');
|
||||
});
|
||||
|
||||
it('should keep level error when the cause declares node-defect', () => {
|
||||
const nodeCause = new NodeOperationError(node, 'Unexpected response shape', {
|
||||
failure: { cause: 'node-defect' },
|
||||
});
|
||||
|
||||
const error = new WorkflowActivationError('activation failed', { cause: nodeCause });
|
||||
|
||||
expect(error.level).toBe('error');
|
||||
});
|
||||
|
||||
const credentialInvalidCause = () =>
|
||||
new NodeOperationError(node, 'The credential needs to be reconnected.', {
|
||||
failure: { cause: 'credential-invalid' },
|
||||
});
|
||||
|
||||
it('should set level warning when the cause declares credential-invalid', () => {
|
||||
const error = new WorkflowActivationError('activation failed', {
|
||||
cause: credentialInvalidCause(),
|
||||
});
|
||||
|
||||
expect(error.level).toBe('warning');
|
||||
});
|
||||
|
||||
it('should set level warning when an error wrapped by the cause declares credential-invalid', () => {
|
||||
const wrapper = new Error('wrapper', { cause: credentialInvalidCause() });
|
||||
|
||||
const error = new WorkflowActivationError('activation failed', { cause: wrapper });
|
||||
|
||||
expect(error.level).toBe('warning');
|
||||
});
|
||||
|
||||
it('should set level warning when the cause declares configuration-invalid', () => {
|
||||
const nodeCause = new NodeOperationError(
|
||||
node,
|
||||
'The folder this node watches no longer exists.',
|
||||
{
|
||||
failure: { cause: 'configuration-invalid' },
|
||||
},
|
||||
);
|
||||
|
||||
const error = new WorkflowActivationError('activation failed', { cause: nodeCause });
|
||||
|
||||
expect(error.level).toBe('warning');
|
||||
});
|
||||
});
|
||||
|
||||
describe('with a frozen Error.prototype', () => {
|
||||
const descriptors = {
|
||||
name: Object.getOwnPropertyDescriptor(Error.prototype, 'name')!,
|
||||
|
||||
Reference in New Issue
Block a user