ci: Introduce no-plain-errors lint rule for BE packages (no-changelog) (#7961)

## Summary
Require `ApplicationError` or its child classes instead of plain `Error`
in BE packages. This ensures the error will be normalized when reported
to Sentry, if applicable.

Follow-up to:
https://github.com/n8n-io/n8n/pulls?q=is%3Apr+is%3Aclosed+applicationerror

...

#### How to test the change:
1. ...


## Issues fixed
Include links to Github issue or Community forum post or **Linear
ticket**:
> Important in order to close automatically and provide context to
reviewers

...


## Review / Merge checklist
- [ ] PR title and summary are descriptive. **Remember, the title
automatically goes into the changelog. Use `(no-changelog)` otherwise.**
([conventions](https://github.com/n8n-io/n8n/blob/master/.github/pull_request_title_conventions.md))
- [ ] [Docs updated](https://github.com/n8n-io/n8n-docs) or follow-up
ticket created.
- [ ] Tests included.
> A bug is not considered fixed, unless a test is added to prevent it
from happening again. A feature is not complete without tests.
  >
> *(internal)* You can use Slack commands to trigger [e2e
tests](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#a39f9e5ba64a48b58a71d81c837e8227)
or [deploy test
instance](https://www.notion.so/n8n/How-to-use-Test-Instances-d65f49dfc51f441ea44367fb6f67eb0a?pvs=4#f6a177d32bde4b57ae2da0b8e454bfce)
or [deploy early access version on
Cloud](https://www.notion.so/n8n/Cloudbot-3dbe779836004972b7057bc989526998?pvs=4#fef2d36ab02247e1a0f65a74f6fb534e).
This commit is contained in:
Iván Ovejero 2023-12-08 12:51:49 +01:00 committed by GitHub
parent 675ec21d33
commit 8cb9c6b3ea
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 80 additions and 21 deletions

View file

@ -10,5 +10,6 @@ module.exports = {
rules: { rules: {
'@typescript-eslint/consistent-type-imports': 'error', '@typescript-eslint/consistent-type-imports': 'error',
'n8n-local-rules/no-plain-errors': 'off',
}, },
}; };

View file

@ -1,4 +1,4 @@
import { NodeConnectionType, NodeOperationError } from 'n8n-workflow'; import { ApplicationError, NodeConnectionType, NodeOperationError } from 'n8n-workflow';
import type { import type {
IBinaryData, IBinaryData,
IDataObject, IDataObject,
@ -105,7 +105,9 @@ async function getChainPromptTemplate(
if (!messageClass) { if (!messageClass) {
// eslint-disable-next-line n8n-nodes-base/node-execute-block-wrong-error-thrown // eslint-disable-next-line n8n-nodes-base/node-execute-block-wrong-error-thrown
throw new Error(`Invalid message type "${message.type}"`); throw new ApplicationError('Invalid message type', {
extra: { messageType: message.type },
});
} }
if (messageClass === HumanMessagePromptTemplate && message.messageType !== 'text') { if (messageClass === HumanMessagePromptTemplate && message.messageType !== 'text') {

View file

@ -330,7 +330,7 @@ const config = (module.exports = {
{ {
selector: 'import', selector: 'import',
format: ['camelCase', 'PascalCase'], format: ['camelCase', 'PascalCase'],
} },
], ],
// ---------------------------------- // ----------------------------------
@ -360,6 +360,7 @@ const config = (module.exports = {
// ---------------------------------- // ----------------------------------
// eslint-plugin-n8n-local-rules // eslint-plugin-n8n-local-rules
// ---------------------------------- // ----------------------------------
'n8n-local-rules/no-uncaught-json-parse': 'error', 'n8n-local-rules/no-uncaught-json-parse': 'error',
'n8n-local-rules/no-json-parse-json-stringify': 'error', 'n8n-local-rules/no-json-parse-json-stringify': 'error',
@ -370,6 +371,8 @@ const config = (module.exports = {
'n8n-local-rules/no-unused-param-in-catch-clause': 'error', 'n8n-local-rules/no-unused-param-in-catch-clause': 'error',
'n8n-local-rules/no-plain-errors': 'error',
// ****************************************************************** // ******************************************************************
// overrides to base ruleset // overrides to base ruleset
// ****************************************************************** // ******************************************************************
@ -469,6 +472,7 @@ const config = (module.exports = {
{ {
files: ['test/**/*.ts', 'src/__tests__/*.ts'], files: ['test/**/*.ts', 'src/__tests__/*.ts'],
rules: { rules: {
'n8n-local-rules/no-plain-errors': 'off',
'n8n-local-rules/no-skipped-tests': 'n8n-local-rules/no-skipped-tests':
process.env.NODE_ENV === 'development' ? 'warn' : 'error', process.env.NODE_ENV === 'development' ? 'warn' : 'error',

View file

@ -52,5 +52,6 @@ module.exports = {
'vue/no-side-effects-in-computed-properties': 'warn', 'vue/no-side-effects-in-computed-properties': 'warn',
'vue/no-v-text-v-html-on-component': 'warn', 'vue/no-v-text-v-html-on-component': 'warn',
'vue/return-in-computed-property': 'warn', 'vue/return-in-computed-property': 'warn',
'n8n-local-rules/no-plain-errors': 'off',
}, },
}; };

View file

@ -353,6 +353,49 @@ module.exports = {
}; };
}, },
}, },
'no-plain-errors': {
meta: {
type: 'problem',
docs: {
description:
'Only `ApplicationError` (from the `workflow` package) or its child classes must be thrown. This ensures the error will be normalized when reported to Sentry, if applicable.',
recommended: 'error',
},
messages: {
useApplicationError:
'Throw an `ApplicationError` (from the `workflow` package) or its child classes.',
},
fixable: 'code',
},
create(context) {
return {
ThrowStatement(node) {
if (!node.argument) return;
const isNewError =
node.argument.type === 'NewExpression' && node.argument.callee.name === 'Error';
const isNewlessError =
node.argument.type === 'CallExpression' && node.argument.callee.name === 'Error';
if (isNewError || isNewlessError) {
return context.report({
messageId: 'useApplicationError',
node,
fix: (fixer) =>
fixer.replaceText(
node,
`throw new ApplicationError(${node.argument.arguments
.map((arg) => arg.raw)
.join(', ')})`,
),
});
}
},
};
},
},
}; };
const isJsonParseCall = (node) => const isJsonParseCall = (node) =>

View file

@ -117,7 +117,7 @@ export class SourceControlService {
this.gitService.resetService(); this.gitService.resetService();
return this.sourceControlPreferencesService.sourceControlPreferences; return this.sourceControlPreferencesService.sourceControlPreferences;
} catch (error) { } catch (error) {
throw Error(`Failed to disconnect from source control: ${(error as Error).message}`); throw new ApplicationError('Failed to disconnect from source control', { cause: error });
} }
} }

View file

@ -6,7 +6,7 @@ import {
SOURCE_CONTROL_TAGS_EXPORT_FILE, SOURCE_CONTROL_TAGS_EXPORT_FILE,
SOURCE_CONTROL_WORKFLOW_EXPORT_FOLDER, SOURCE_CONTROL_WORKFLOW_EXPORT_FOLDER,
} from './constants'; } from './constants';
import type { ICredentialDataDecryptedObject } from 'n8n-workflow'; import { ApplicationError, type ICredentialDataDecryptedObject } from 'n8n-workflow';
import { writeFile as fsWriteFile, rm as fsRm } from 'fs/promises'; import { writeFile as fsWriteFile, rm as fsRm } from 'fs/promises';
import { rmSync } from 'fs'; import { rmSync } from 'fs';
import { Credentials, InstanceSettings } from 'n8n-core'; import { Credentials, InstanceSettings } from 'n8n-core';
@ -138,7 +138,7 @@ export class SourceControlExportService {
})), })),
}; };
} catch (error) { } catch (error) {
throw Error(`Failed to export workflows to work folder: ${(error as Error).message}`); throw new ApplicationError('Failed to export workflows to work folder', { cause: error });
} }
} }
@ -168,7 +168,9 @@ export class SourceControlExportService {
], ],
}; };
} catch (error) { } catch (error) {
throw Error(`Failed to export variables to work folder: ${(error as Error).message}`); throw new ApplicationError('Failed to export variables to work folder', {
cause: error,
});
} }
} }
@ -208,7 +210,7 @@ export class SourceControlExportService {
], ],
}; };
} catch (error) { } catch (error) {
throw Error(`Failed to export variables to work folder: ${(error as Error).message}`); throw new ApplicationError('Failed to export variables to work folder', { cause: error });
} }
} }
@ -280,7 +282,7 @@ export class SourceControlExportService {
missingIds, missingIds,
}; };
} catch (error) { } catch (error) {
throw Error(`Failed to export credentials to work folder: ${(error as Error).message}`); throw new ApplicationError('Failed to export credentials to work folder', { cause: error });
} }
} }
} }

View file

@ -108,7 +108,7 @@ export class SourceControlPreferencesService {
}); });
await fsWriteFile(this.sshKeyName, keyPair.privateKey, { encoding: 'utf8', mode: 0o600 }); await fsWriteFile(this.sshKeyName, keyPair.privateKey, { encoding: 'utf8', mode: 0o600 });
} catch (error) { } catch (error) {
throw Error(`Failed to save key pair: ${(error as Error).message}`); throw new ApplicationError('Failed to save key pair', { cause: error });
} }
} }
// update preferences only after generating key pair to prevent endless loop // update preferences only after generating key pair to prevent endless loop

View file

@ -12,5 +12,6 @@ module.exports = {
rules: { rules: {
'import/order': 'off', // TODO: remove this 'import/order': 'off', // TODO: remove this
'@typescript-eslint/ban-ts-comment': ['warn', { 'ts-ignore': true }], '@typescript-eslint/ban-ts-comment': ['warn', { 'ts-ignore': true }],
'n8n-local-rules/no-plain-errors': 'off',
}, },
}; };

View file

@ -259,11 +259,8 @@ export function findMatches(
if (disableDotNotation && skipFields.some((field) => field.includes('.'))) { if (disableDotNotation && skipFields.some((field) => field.includes('.'))) {
const fieldToSkip = skipFields.find((field) => field.includes('.')); const fieldToSkip = skipFields.find((field) => field.includes('.'));
throw new Error( const msg = `Dot notation is disabled, but field to skip comparing '${fieldToSkip}' contains dot`;
`Dot notation is disabled, but field to skip comparing '${ throw new ApplicationError(msg, { level: 'warning' });
fieldToSkip as string
}' contains dot`,
);
} }
const filteredData = { const filteredData = {
@ -403,16 +400,18 @@ export function findMatches(
export function checkMatchFieldsInput(data: IDataObject[]) { export function checkMatchFieldsInput(data: IDataObject[]) {
if (data.length === 1 && data[0].field1 === '' && data[0].field2 === '') { if (data.length === 1 && data[0].field1 === '' && data[0].field2 === '') {
throw new Error( throw new ApplicationError(
'You need to define at least one pair of fields in "Fields to Match" to match on', 'You need to define at least one pair of fields in "Fields to Match" to match on',
{ level: 'warning' },
); );
} }
for (const [index, pair] of data.entries()) { for (const [index, pair] of data.entries()) {
if (pair.field1 === '' || pair.field2 === '') { if (pair.field1 === '' || pair.field2 === '') {
throw new Error( throw new ApplicationError(
`You need to define both fields in "Fields to Match" for pair ${index + 1}, `You need to define both fields in "Fields to Match" for pair ${index + 1},
field 1 = '${pair.field1}' field 1 = '${pair.field1}'
field 2 = '${pair.field2}'`, field 2 = '${pair.field2}'`,
{ level: 'warning' },
); );
} }
} }
@ -447,7 +446,10 @@ export function checkInputAndThrowError(
return get(entry.json, field, undefined) !== undefined; return get(entry.json, field, undefined) !== undefined;
}); });
if (!isPresent) { if (!isPresent) {
throw new Error(`Field '${field}' is not present in any of items in '${inputLabel}'`); throw new ApplicationError(
`Field '${field}' is not present in any of items in '${inputLabel}'`,
{ level: 'warning' },
);
} }
} }
return input; return input;

View file

@ -14,7 +14,7 @@ import type {
INodeType, INodeType,
INodeTypeDescription, INodeTypeDescription,
} from 'n8n-workflow'; } from 'n8n-workflow';
import { NodeOperationError } from 'n8n-workflow'; import { ApplicationError, NodeOperationError } from 'n8n-workflow';
import { generatePairedItemData } from '../../utils/utilities'; import { generatePairedItemData } from '../../utils/utilities';
export class Kafka implements INodeType { export class Kafka implements INodeType {
@ -229,7 +229,10 @@ export class Kafka implements INodeType {
}; };
if (credentials.authentication === true) { if (credentials.authentication === true) {
if (!(credentials.username && credentials.password)) { if (!(credentials.username && credentials.password)) {
throw Error('Username and password are required for authentication'); // eslint-disable-next-line n8n-nodes-base/node-execute-block-wrong-error-thrown
throw new ApplicationError('Username and password are required for authentication', {
level: 'warning',
});
} }
config.sasl = { config.sasl = {
username: credentials.username as string, username: credentials.username as string,

View file

@ -119,7 +119,7 @@ export async function prepareCustomFields(
return customFields; return customFields;
} else if (customFieldsJson) { } else if (customFieldsJson) {
throw Error('customFieldsJson value is invalid'); throw new ApplicationError('customFieldsJson value is invalid', { level: 'warning' });
} }
} else if (additionalFields.customFieldsUi) { } else if (additionalFields.customFieldsUi) {
// Get Custom Field Types from TheHive // Get Custom Field Types from TheHive