refactor: Remove usless catch blocks, and add a linting rule to preve…

…nt them (no-changelog) (#12730)
n8n-io · Jan 20, 2025 · 202da76 · 202da76
1 parent 4ee4552
commit 202da76
Show file tree

Hide file tree

Showing 17 changed files with 224 additions and 246 deletions.
diff --git a/packages/@n8n/nodes-langchain/nodes/vector_store/shared/createVectorStoreNode.ts b/packages/@n8n/nodes-langchain/nodes/vector_store/shared/createVectorStoreNode.ts
@@ -392,13 +392,9 @@ export const createVectorStoreNode = (args: VectorStoreNodeConstructorArgs) =>
 					);
 					resultData.push(...serializedDocuments);
 
-					try {
-						await args.populateVectorStore(this, embeddings, processedDocuments, itemIndex);
+					await args.populateVectorStore(this, embeddings, processedDocuments, itemIndex);
 
-						logAiEvent(this, 'ai-vector-store-populated');
-					} catch (error) {
-						throw error;
-					}
+					logAiEvent(this, 'ai-vector-store-populated');
 				}
 
 				return [resultData];
@@ -443,16 +439,12 @@ export const createVectorStoreNode = (args: VectorStoreNodeConstructorArgs) =>
 
 					resultData.push(...serializedDocuments);
 
-					try {
-						// Use ids option to upsert instead of insert
-						await vectorStore.addDocuments(processedDocuments, {
-							ids: [documentId],
-						});
+					// Use ids option to upsert instead of insert
+					await vectorStore.addDocuments(processedDocuments, {
+						ids: [documentId],
+					});
 
-						logAiEvent(this, 'ai-vector-store-updated');
-					} catch (error) {
-						throw error;
-					}
+					logAiEvent(this, 'ai-vector-store-updated');
 				}
 
 				return [resultData];

diff --git a/packages/@n8n_io/eslint-config/base.js b/packages/@n8n_io/eslint-config/base.js
@@ -369,6 +369,8 @@ const config = (module.exports = {
 
 		'n8n-local-rules/no-unused-param-in-catch-clause': 'error',
 
+		'n8n-local-rules/no-useless-catch-throw': 'error',
+
 		'n8n-local-rules/no-plain-errors': 'error',
 
 		// ******************************************************************

diff --git a/packages/@n8n_io/eslint-config/local-rules.js b/packages/@n8n_io/eslint-config/local-rules.js
@@ -172,6 +172,49 @@ module.exports = {
 		},
 	},
 
+	'no-useless-catch-throw': {
+		meta: {
+			type: 'problem',
+			docs: {
+				description: 'Disallow `try-catch` blocks where the `catch` only contains a `throw error`.',
+				recommended: 'error',
+			},
+			messages: {
+				noUselessCatchThrow: 'Remove useless `catch` block.',
+			},
+			fixable: 'code',
+		},
+		create(context) {
+			return {
+				CatchClause(node) {
+					if (
+						node.body.body.length === 1 &&
+						node.body.body[0].type === 'ThrowStatement' &&
+						node.body.body[0].argument.type === 'Identifier' &&
+						node.body.body[0].argument.name === node.param.name
+					) {
+						context.report({
+							node,
+							messageId: 'noUselessCatchThrow',
+							fix(fixer) {
+								const tryStatement = node.parent;
+								const tryBlock = tryStatement.block;
+								const sourceCode = context.getSourceCode();
+								const tryBlockText = sourceCode.getText(tryBlock);
+								const tryBlockTextWithoutBraces = tryBlockText.slice(1, -1).trim();
+								const indentedTryBlockText = tryBlockTextWithoutBraces
+									.split('\n')
+									.map((line) => line.replace(/\t/, ''))
+									.join('\n');
+								return fixer.replaceText(tryStatement, indentedTryBlockText);
+							},
+						});
+					}
+				},
+			};
+		},
+	},
+
 	'no-skipped-tests': {
 		meta: {
 			type: 'problem',

diff --git a/packages/@n8n_io/eslint-config/local-rules.test.js b/packages/@n8n_io/eslint-config/local-rules.test.js
@@ -51,3 +51,33 @@ ruleTester.run('no-json-parse-json-stringify', rules['no-json-parse-json-stringi
 		},
 	],
 });
+
+ruleTester.run('no-useless-catch-throw', rules['no-useless-catch-throw'], {
+	valid: [
+		{
+			code: 'try { foo(); } catch (e) { console.error(e); }',
+		},
+		{
+			code: 'try { foo(); } catch (e) { throw new Error("Custom error"); }',
+		},
+	],
+	invalid: [
+		{
+			code: `
+try {
+	// Some comment
+	if (foo) {
+		bar();
+	}
+} catch (e) {
+	throw e;
+}`,
+			errors: [{ messageId: 'noUselessCatchThrow' }],
+			output: `
+// Some comment
+if (foo) {
+	bar();
+}`,
+		},
+	],
+});
diff --git a/packages/editor-ui/src/components/ButtonParameter/utils.ts b/packages/editor-ui/src/components/ButtonParameter/utils.ts
@@ -161,28 +161,24 @@ export function reducePayloadSizeOrThrow(
 	error: Error,
 	averageTokenLength = 4,
 ) {
-	try {
-		let remainingTokensToReduce = calculateRemainingTokens(error);
-
-		const [remaining, parentNodesTokenCount] = trimParentNodesSchema(
-			payload,
-			remainingTokensToReduce,
-			averageTokenLength,
-		);
-
-		remainingTokensToReduce = remaining;
-
-		remainingTokensToReduce = trimInputSchemaProperties(
-			payload,
-			remainingTokensToReduce,
-			averageTokenLength,
-			parentNodesTokenCount,
-		);
-
-		if (remainingTokensToReduce > 0) throw error;
-	} catch (e) {
-		throw e;
-	}
+	let remainingTokensToReduce = calculateRemainingTokens(error);
+
+	const [remaining, parentNodesTokenCount] = trimParentNodesSchema(
+		payload,
+		remainingTokensToReduce,
+		averageTokenLength,
+	);
+
+	remainingTokensToReduce = remaining;
+
+	remainingTokensToReduce = trimInputSchemaProperties(
+		payload,
+		remainingTokensToReduce,
+		averageTokenLength,
+		parentNodesTokenCount,
+	);
+
+	if (remainingTokensToReduce > 0) throw error;
 }
 
 export async function generateCodeForAiTransform(prompt: string, path: string, retries = 1) {

diff --git a/packages/editor-ui/src/components/CanvasChat/CanvasChat.vue b/packages/editor-ui/src/components/CanvasChat/CanvasChat.vue
@@ -200,29 +200,25 @@ async function createExecutionPromise() {
 }
 
 async function onRunChatWorkflow(payload: RunWorkflowChatPayload) {
-	try {
-		const runWorkflowOptions: Parameters<typeof runWorkflow>[0] = {
-			triggerNode: payload.triggerNode,
-			nodeData: payload.nodeData,
-			source: payload.source,
-		};
-
-		if (workflowsStore.chatPartialExecutionDestinationNode) {
-			runWorkflowOptions.destinationNode = workflowsStore.chatPartialExecutionDestinationNode;
-			workflowsStore.chatPartialExecutionDestinationNode = null;
-		}
+	const runWorkflowOptions: Parameters<typeof runWorkflow>[0] = {
+		triggerNode: payload.triggerNode,
+		nodeData: payload.nodeData,
+		source: payload.source,
+	};
 
-		const response = await runWorkflow(runWorkflowOptions);
+	if (workflowsStore.chatPartialExecutionDestinationNode) {
+		runWorkflowOptions.destinationNode = workflowsStore.chatPartialExecutionDestinationNode;
+		workflowsStore.chatPartialExecutionDestinationNode = null;
+	}
 
-		if (response) {
-			await createExecutionPromise();
-			workflowsStore.appendChatMessage(payload.message);
-			return response;
-		}
-		return;
-	} catch (error) {
-		throw error;
+	const response = await runWorkflow(runWorkflowOptions);
+
+	if (response) {
+		await createExecutionPromise();
+		workflowsStore.appendChatMessage(payload.message);
+		return response;
 	}
+	return;
 }
 
 // Initialize chat config

diff --git a/packages/editor-ui/src/stores/communityNodes.store.ts b/packages/editor-ui/src/stores/communityNodes.store.ts
@@ -55,21 +55,13 @@ export const useCommunityNodesStore = defineStore(STORES.COMMUNITY_NODES, () =>
 	};
 
 	const installPackage = async (packageName: string): Promise<void> => {
-		try {
-			await communityNodesApi.installNewPackage(rootStore.restApiContext, packageName);
-			await fetchInstalledPackages();
-		} catch (error) {
-			throw error;
-		}
+		await communityNodesApi.installNewPackage(rootStore.restApiContext, packageName);
+		await fetchInstalledPackages();
 	};
 
 	const uninstallPackage = async (packageName: string): Promise<void> => {
-		try {
-			await communityNodesApi.uninstallPackage(rootStore.restApiContext, packageName);
-			removePackageByName(packageName);
-		} catch (error) {
-			throw error;
-		}
+		await communityNodesApi.uninstallPackage(rootStore.restApiContext, packageName);
+		removePackageByName(packageName);
 	};
 
 	const removePackageByName = (name: string): void => {
@@ -82,16 +74,12 @@ export const useCommunityNodesStore = defineStore(STORES.COMMUNITY_NODES, () =>
 	};
 
 	const updatePackage = async (packageName: string): Promise<void> => {
-		try {
-			const packageToUpdate = installedPackages.value[packageName];
-			const updatedPackage = await communityNodesApi.updatePackage(
-				rootStore.restApiContext,
-				packageToUpdate.packageName,
-			);
-			updatePackageObject(updatedPackage);
-		} catch (error) {
-			throw error;
-		}
+		const packageToUpdate = installedPackages.value[packageName];
+		const updatedPackage = await communityNodesApi.updatePackage(
+			rootStore.restApiContext,
+			packageToUpdate.packageName,
+		);
+		updatePackageObject(updatedPackage);
 	};
 
 	return {

diff --git a/packages/editor-ui/src/stores/executions.store.ts b/packages/editor-ui/src/stores/executions.store.ts
@@ -173,8 +173,6 @@ export const useExecutionsStore = defineStore('executions', () => {
 			executionsCount.value = data.count;
 			executionsCountEstimated.value = data.estimated;
 			return data;
-		} catch (e) {
-			throw e;
 		} finally {
 			loading.value = false;
 		}