From 0d23a7fb5ba41545f70c4848d30b90af91b1e7e6 Mon Sep 17 00:00:00 2001 From: Eugene Date: Fri, 11 Oct 2024 17:12:03 +0200 Subject: [PATCH 001/164] fix(HTTP Request Tool Node): Respond with an error when receive binary response (#11219) --- .../ToolHttpRequest/ToolHttpRequest.node.ts | 1 + .../test/ToolHttpRequest.node.test.ts | 165 ++++++++++++++++++ .../nodes/tools/ToolHttpRequest/utils.ts | 48 +++-- packages/@n8n/nodes-langchain/package.json | 2 + pnpm-lock.yaml | 18 +- 5 files changed, 212 insertions(+), 22 deletions(-) create mode 100644 packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/test/ToolHttpRequest.node.test.ts diff --git a/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/ToolHttpRequest.node.ts b/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/ToolHttpRequest.node.ts index 32f6be42e7..aa294edadd 100644 --- a/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/ToolHttpRequest.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/ToolHttpRequest.node.ts @@ -281,6 +281,7 @@ export class ToolHttpRequest implements INodeType { 'User-Agent': undefined, }, body: {}, + returnFullResponse: true, }; const authentication = this.getNodeParameter('authentication', itemIndex, 'none') as diff --git a/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/test/ToolHttpRequest.node.test.ts b/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/test/ToolHttpRequest.node.test.ts new file mode 100644 index 0000000000..161aa140f5 --- /dev/null +++ b/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/test/ToolHttpRequest.node.test.ts @@ -0,0 +1,165 @@ +import get from 'lodash/get'; +import type { IDataObject, IExecuteFunctions } from 'n8n-workflow'; +import { jsonParse } from 'n8n-workflow'; + +import type { N8nTool } from '../../../../utils/N8nTool'; +import { ToolHttpRequest } from '../ToolHttpRequest.node'; + +const createExecuteFunctionsMock = (parameters: IDataObject, requestMock: any) => { + const nodeParameters = parameters; + + return { + getNodeParameter(parameter: string) { + return get(nodeParameters, parameter); + }, + getNode() { + return { + name: 'HTTP Request', + }; + }, + getInputData() { + return [{ json: {} }]; + }, + getWorkflow() { + return { + name: 'Test Workflow', + }; + }, + continueOnFail() { + return false; + }, + addInputData() { + return { index: 0 }; + }, + addOutputData() { + return; + }, + helpers: { + httpRequest: requestMock, + }, + } as unknown as IExecuteFunctions; +}; + +describe('ToolHttpRequest', () => { + let httpTool: ToolHttpRequest; + let mockRequest: jest.Mock; + + describe('Binary response', () => { + beforeEach(() => { + httpTool = new ToolHttpRequest(); + mockRequest = jest.fn(); + }); + + it('should return the error when receiving a binary response', async () => { + mockRequest.mockResolvedValue({ + body: Buffer.from(''), + headers: { + 'content-type': 'image/jpeg', + }, + }); + + const { response } = await httpTool.supplyData.call( + createExecuteFunctionsMock( + { + method: 'GET', + url: 'https://httpbin.org/image/jpeg', + options: {}, + placeholderDefinitions: { + values: [], + }, + }, + mockRequest, + ), + 0, + ); + + const res = await (response as N8nTool).invoke(''); + + expect(res).toContain('error'); + expect(res).toContain('Binary data is not supported'); + }); + + it('should return the response text when receiving a text response', async () => { + mockRequest.mockResolvedValue({ + body: 'Hello World', + headers: { + 'content-type': 'text/plain', + }, + }); + + const { response } = await httpTool.supplyData.call( + createExecuteFunctionsMock( + { + method: 'GET', + url: 'https://httpbin.org/text/plain', + options: {}, + placeholderDefinitions: { + values: [], + }, + }, + mockRequest, + ), + 0, + ); + + const res = await (response as N8nTool).invoke(''); + expect(res).toEqual('Hello World'); + }); + + it('should return the response text when receiving a text response with a charset', async () => { + mockRequest.mockResolvedValue({ + body: 'こんにちは世界', + headers: { + 'content-type': 'text/plain; charset=iso-2022-jp', + }, + }); + + const { response } = await httpTool.supplyData.call( + createExecuteFunctionsMock( + { + method: 'GET', + url: 'https://httpbin.org/text/plain', + options: {}, + placeholderDefinitions: { + values: [], + }, + }, + mockRequest, + ), + 0, + ); + + const res = await (response as N8nTool).invoke(''); + expect(res).toEqual('こんにちは世界'); + }); + + it('should return the response object when receiving a JSON response', async () => { + const mockJson = { hello: 'world' }; + + mockRequest.mockResolvedValue({ + body: mockJson, + headers: { + 'content-type': 'application/json', + }, + }); + + const { response } = await httpTool.supplyData.call( + createExecuteFunctionsMock( + { + method: 'GET', + url: 'https://httpbin.org/json', + options: {}, + placeholderDefinitions: { + values: [], + }, + }, + mockRequest, + ), + 0, + ); + + const res = await (response as N8nTool).invoke(''); + expect(jsonParse(res)).toEqual(mockJson); + }); + }); +}); diff --git a/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/utils.ts b/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/utils.ts index c06a869a8d..e637251a74 100644 --- a/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/utils.ts +++ b/packages/@n8n/nodes-langchain/nodes/tools/ToolHttpRequest/utils.ts @@ -1,3 +1,12 @@ +import { Readability } from '@mozilla/readability'; +import cheerio from 'cheerio'; +import { convert } from 'html-to-text'; +import { JSDOM } from 'jsdom'; +import get from 'lodash/get'; +import set from 'lodash/set'; +import unset from 'lodash/unset'; +import * as mime from 'mime-types'; +import { getOAuth2AdditionalParameters } from 'n8n-nodes-base/dist/nodes/HttpRequest/GenericFunctions'; import type { IExecuteFunctions, IDataObject, @@ -7,20 +16,8 @@ import type { NodeApiError, } from 'n8n-workflow'; import { NodeConnectionType, NodeOperationError, jsonParse } from 'n8n-workflow'; - -import { getOAuth2AdditionalParameters } from 'n8n-nodes-base/dist/nodes/HttpRequest/GenericFunctions'; - -import set from 'lodash/set'; -import get from 'lodash/get'; -import unset from 'lodash/unset'; - -import cheerio from 'cheerio'; -import { convert } from 'html-to-text'; - -import { Readability } from '@mozilla/readability'; -import { JSDOM } from 'jsdom'; import { z } from 'zod'; -import type { DynamicZodObject } from '../../../types/zod.types'; + import type { ParameterInputType, ParametersValues, @@ -29,6 +26,7 @@ import type { SendIn, ToolParameter, } from './interfaces'; +import type { DynamicZodObject } from '../../../types/zod.types'; const genericCredentialRequest = async (ctx: IExecuteFunctions, itemIndex: number) => { const genericType = ctx.getNodeParameter('genericAuthType', itemIndex) as string; @@ -176,6 +174,7 @@ const htmlOptimizer = (ctx: IExecuteFunctions, itemIndex: number, maxLength: num ); } const returnData: string[] = []; + const html = cheerio.load(response); const htmlElements = html(cssSelector); @@ -574,6 +573,7 @@ export const configureToolFunction = ( // Clone options and rawRequestOptions to avoid mutating the original objects const options: IHttpRequestOptions | null = structuredClone(requestOptions); const clonedRawRequestOptions: { [key: string]: string } = structuredClone(rawRequestOptions); + let fullResponse: any; let response: string = ''; let executionError: Error | undefined = undefined; @@ -732,8 +732,6 @@ export const configureToolFunction = ( } } } catch (error) { - console.error(error); - const errorMessage = 'Input provided by model is not valid'; if (error instanceof NodeOperationError) { @@ -749,11 +747,29 @@ export const configureToolFunction = ( if (options) { try { - response = optimizeResponse(await httpRequest(options)); + fullResponse = await httpRequest(options); } catch (error) { const httpCode = (error as NodeApiError).httpCode; response = `${httpCode ? `HTTP ${httpCode} ` : ''}There was an error: "${error.message}"`; } + + if (!response) { + try { + // Check if the response is binary data + if (fullResponse?.headers?.['content-type']) { + const contentType = fullResponse.headers['content-type'] as string; + const mimeType = contentType.split(';')[0].trim(); + + if (mime.charset(mimeType) !== 'UTF-8') { + throw new NodeOperationError(ctx.getNode(), 'Binary data is not supported'); + } + } + + response = optimizeResponse(fullResponse.body); + } catch (error) { + response = `There was an error: "${error.message}"`; + } + } } if (typeof response !== 'string') { diff --git a/packages/@n8n/nodes-langchain/package.json b/packages/@n8n/nodes-langchain/package.json index 4df337ed52..d3da518f10 100644 --- a/packages/@n8n/nodes-langchain/package.json +++ b/packages/@n8n/nodes-langchain/package.json @@ -124,6 +124,7 @@ "@types/cheerio": "^0.22.15", "@types/html-to-text": "^9.0.1", "@types/json-schema": "^7.0.15", + "@types/mime-types": "^2.1.0", "@types/pg": "^8.11.6", "@types/temp": "^0.9.1", "n8n-core": "workspace:*" @@ -171,6 +172,7 @@ "langchain": "0.3.2", "lodash": "catalog:", "mammoth": "1.7.2", + "mime-types": "2.1.35", "n8n-nodes-base": "workspace:*", "n8n-workflow": "workspace:*", "openai": "4.63.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0dca3aee05..6748091148 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -522,6 +522,9 @@ importers: mammoth: specifier: 1.7.2 version: 1.7.2 + mime-types: + specifier: 2.1.35 + version: 2.1.35 n8n-nodes-base: specifier: workspace:* version: link:../../nodes-base @@ -568,6 +571,9 @@ importers: '@types/json-schema': specifier: ^7.0.15 version: 7.0.15 + '@types/mime-types': + specifier: ^2.1.0 + version: 2.1.1 '@types/pg': specifier: ^8.11.6 version: 8.11.6 @@ -19287,7 +19293,7 @@ snapshots: eslint-import-resolver-node@0.3.9: dependencies: - debug: 3.2.7(supports-color@5.5.0) + debug: 3.2.7(supports-color@8.1.1) is-core-module: 2.13.1 resolve: 1.22.8 transitivePeerDependencies: @@ -19312,7 +19318,7 @@ snapshots: eslint-module-utils@2.8.0(@typescript-eslint/parser@7.2.0(eslint@8.57.0)(typescript@5.6.2))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@3.6.1(@typescript-eslint/parser@7.2.0(eslint@8.57.0)(typescript@5.6.2))(eslint-plugin-import@2.29.1)(eslint@8.57.0))(eslint@8.57.0): dependencies: - debug: 3.2.7(supports-color@5.5.0) + debug: 3.2.7(supports-color@8.1.1) optionalDependencies: '@typescript-eslint/parser': 7.2.0(eslint@8.57.0)(typescript@5.6.2) eslint: 8.57.0 @@ -19332,7 +19338,7 @@ snapshots: array.prototype.findlastindex: 1.2.3 array.prototype.flat: 1.3.2 array.prototype.flatmap: 1.3.2 - debug: 3.2.7(supports-color@5.5.0) + debug: 3.2.7(supports-color@8.1.1) doctrine: 2.1.0 eslint: 8.57.0 eslint-import-resolver-node: 0.3.9 @@ -20130,7 +20136,7 @@ snapshots: array-parallel: 0.1.3 array-series: 0.1.5 cross-spawn: 4.0.2 - debug: 3.2.7(supports-color@5.5.0) + debug: 3.2.7(supports-color@8.1.1) transitivePeerDependencies: - supports-color @@ -23033,7 +23039,7 @@ snapshots: pdf-parse@1.1.1: dependencies: - debug: 3.2.7(supports-color@5.5.0) + debug: 3.2.7(supports-color@8.1.1) node-ensure: 0.0.0 transitivePeerDependencies: - supports-color @@ -23862,7 +23868,7 @@ snapshots: rhea@1.0.24: dependencies: - debug: 3.2.7(supports-color@5.5.0) + debug: 3.2.7(supports-color@8.1.1) transitivePeerDependencies: - supports-color From 4f27b39b45b58779d363980241e6e5e11b58f5da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Fri, 11 Oct 2024 18:26:43 +0200 Subject: [PATCH 002/164] fix(editor): Make submit in ChangePasswordView work again (#11227) --- packages/editor-ui/src/views/ChangePasswordView.vue | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/editor-ui/src/views/ChangePasswordView.vue b/packages/editor-ui/src/views/ChangePasswordView.vue index 7c19749519..7a48c5867a 100644 --- a/packages/editor-ui/src/views/ChangePasswordView.vue +++ b/packages/editor-ui/src/views/ChangePasswordView.vue @@ -47,12 +47,7 @@ const getMfaEnabled = () => { return router.currentRoute.value.query.mfaEnabled === 'true' ? true : false; }; -const isFormWithMFAToken = (values: { [key: string]: string }): values is { mfaToken: string } => { - return 'mfaToken' in values; -}; - const onSubmit = async (values: { [key: string]: string }) => { - if (!isFormWithMFAToken(values)) return; try { loading.value = true; const token = getResetToken(); From 566529ca1149988a54a58b3c34bbe4d9f1add6db Mon Sep 17 00:00:00 2001 From: Jon Date: Mon, 14 Oct 2024 11:17:19 +0100 Subject: [PATCH 003/164] fix(Strava Trigger Node): Fix issue with webhook not being deleted (#11226) --- packages/nodes-base/nodes/Strava/GenericFunctions.ts | 3 +-- packages/nodes-base/nodes/Strava/StravaTrigger.node.ts | 10 +++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/nodes-base/nodes/Strava/GenericFunctions.ts b/packages/nodes-base/nodes/Strava/GenericFunctions.ts index d5488b7e0e..deaeb425b3 100644 --- a/packages/nodes-base/nodes/Strava/GenericFunctions.ts +++ b/packages/nodes-base/nodes/Strava/GenericFunctions.ts @@ -36,14 +36,13 @@ export async function stravaApiRequest( if (this.getNode().type.includes('Trigger') && resource.includes('/push_subscriptions')) { const credentials = await this.getCredentials('stravaOAuth2Api'); - if (method === 'GET') { + if (method === 'GET' || method === 'DELETE') { qs.client_id = credentials.clientId; qs.client_secret = credentials.clientSecret; } else { body.client_id = credentials.clientId; body.client_secret = credentials.clientSecret; } - return await this.helpers?.request(options); } else { return await this.helpers.requestOAuth2.call(this, 'stravaOAuth2Api', options, { diff --git a/packages/nodes-base/nodes/Strava/StravaTrigger.node.ts b/packages/nodes-base/nodes/Strava/StravaTrigger.node.ts index e5b24b19ea..f4eb8d8cf1 100644 --- a/packages/nodes-base/nodes/Strava/StravaTrigger.node.ts +++ b/packages/nodes-base/nodes/Strava/StravaTrigger.node.ts @@ -112,7 +112,7 @@ export class StravaTrigger implements INodeType { default: false, // eslint-disable-next-line n8n-nodes-base/node-param-description-boolean-without-whether description: - 'Strava allows just one subscription at all times. If you want to delete the current subscription to make room for a new subcription with the current parameters, set this parameter to true. Keep in mind this is a destructive operation.', + 'Strava allows just one subscription at all times. If you want to delete the current subscription to make room for a new subscription with the current parameters, set this parameter to true. Keep in mind this is a destructive operation.', }, ], }, @@ -155,9 +155,8 @@ export class StravaTrigger implements INodeType { try { responseData = await stravaApiRequest.call(this, 'POST', endpoint, body); } catch (error) { - const apiErrorResponse = error.cause.response; - if (apiErrorResponse?.body?.errors) { - const errors = apiErrorResponse.body.errors; + if (error?.cause?.error) { + const errors = error?.cause?.error?.errors; for (error of errors) { // if there is a subscription already created if (error.resource === 'PushSubscription' && error.code === 'already exists') { @@ -177,6 +176,7 @@ export class StravaTrigger implements INodeType { 'DELETE', `/push_subscriptions/${webhooks[0].id}`, ); + // now there is room create a subscription with the n8n data const requestBody = { callback_url: webhookUrl, @@ -190,7 +190,7 @@ export class StravaTrigger implements INodeType { requestBody, ); } else { - error.message = `A subscription already exists [${webhooks[0].callback_url}]. If you want to delete this subcription and create a new one with the current parameters please go to options and set delete if exist to true`; + error.message = `A subscription already exists [${webhooks[0].callback_url}]. If you want to delete this subscription and create a new one with the current parameters please go to options and set delete if exist to true`; throw error; } } From b7ee0c4087eae346bc7e5360130d6c812dbe99db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Mon, 14 Oct 2024 12:57:30 +0200 Subject: [PATCH 004/164] fix(MySQL Node): Fix "Maximum call stack size exceeded" error when handling a large number of rows (#11242) --- .../nodes/MySql/v2/helpers/utils.ts | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/packages/nodes-base/nodes/MySql/v2/helpers/utils.ts b/packages/nodes-base/nodes/MySql/v2/helpers/utils.ts index 6e50ae3eb1..60df354528 100644 --- a/packages/nodes-base/nodes/MySql/v2/helpers/utils.ts +++ b/packages/nodes-base/nodes/MySql/v2/helpers/utils.ts @@ -209,22 +209,22 @@ export function configureQueryRunner( return []; } - const returnData: INodeExecutionData[] = []; + let returnData: INodeExecutionData[] = []; const mode = (options.queryBatching as QueryMode) || BATCH_MODE.SINGLE; const connection = await pool.getConnection(); if (mode === BATCH_MODE.SINGLE) { - const formatedQueries = queries.map(({ query, values }) => connection.format(query, values)); + const formattedQueries = queries.map(({ query, values }) => connection.format(query, values)); try { - //releasing connection after formating queries, otherwise pool.query() will fail with timeout + //releasing connection after formatting queries, otherwise pool.query() will fail with timeout connection.release(); let singleQuery = ''; - if (formatedQueries.length > 1) { - singleQuery = formatedQueries.map((query) => query.trim().replace(/;$/, '')).join(';'); + if (formattedQueries.length > 1) { + singleQuery = formattedQueries.map((query) => query.trim().replace(/;$/, '')).join(';'); } else { - singleQuery = formatedQueries[0]; + singleQuery = formattedQueries[0]; } let response: IDataObject | IDataObject[] = ( @@ -249,11 +249,11 @@ export function configureQueryRunner( response = [response]; } - //because single query is used in this mode mapping itemIndex not posible, setting all items as paired + //because single query is used in this mode mapping itemIndex not possible, setting all items as paired const pairedItem = generatePairedItemData(queries.length); - returnData.push( - ...prepareOutput( + returnData = returnData.concat( + prepareOutput( response, options, statements, @@ -262,24 +262,24 @@ export function configureQueryRunner( ), ); } catch (err) { - const error = parseMySqlError.call(this, err, 0, formatedQueries); + const error = parseMySqlError.call(this, err, 0, formattedQueries); if (!this.continueOnFail()) throw error; returnData.push({ json: { message: error.message, error: { ...error } } }); } } else { if (mode === BATCH_MODE.INDEPENDENTLY) { - let formatedQuery = ''; + let formattedQuery = ''; for (const [index, queryWithValues] of queries.entries()) { try { const { query, values } = queryWithValues; - formatedQuery = connection.format(query, values); + formattedQuery = connection.format(query, values); let statements; if ((options?.nodeVersion as number) <= 2.3) { - statements = formatedQuery.split(';').map((q) => q.trim()); + statements = formattedQuery.split(';').map((q) => q.trim()); } else { - statements = splitQueryToStatements(formatedQuery, false); + statements = splitQueryToStatements(formattedQuery, false); } const responses: IDataObject[] = []; @@ -290,8 +290,8 @@ export function configureQueryRunner( responses.push(response); } - returnData.push( - ...prepareOutput( + returnData = returnData.concat( + prepareOutput( responses, options, statements, @@ -300,7 +300,7 @@ export function configureQueryRunner( ), ); } catch (err) { - const error = parseMySqlError.call(this, err, index, [formatedQuery]); + const error = parseMySqlError.call(this, err, index, [formattedQuery]); if (!this.continueOnFail()) { connection.release(); @@ -314,17 +314,17 @@ export function configureQueryRunner( if (mode === BATCH_MODE.TRANSACTION) { await connection.beginTransaction(); - let formatedQuery = ''; + let formattedQuery = ''; for (const [index, queryWithValues] of queries.entries()) { try { const { query, values } = queryWithValues; - formatedQuery = connection.format(query, values); + formattedQuery = connection.format(query, values); let statements; if ((options?.nodeVersion as number) <= 2.3) { - statements = formatedQuery.split(';').map((q) => q.trim()); + statements = formattedQuery.split(';').map((q) => q.trim()); } else { - statements = splitQueryToStatements(formatedQuery, false); + statements = splitQueryToStatements(formattedQuery, false); } const responses: IDataObject[] = []; @@ -335,8 +335,8 @@ export function configureQueryRunner( responses.push(response); } - returnData.push( - ...prepareOutput( + returnData = returnData.concat( + prepareOutput( responses, options, statements, @@ -345,7 +345,7 @@ export function configureQueryRunner( ), ); } catch (err) { - const error = parseMySqlError.call(this, err, index, [formatedQuery]); + const error = parseMySqlError.call(this, err, index, [formattedQuery]); if (connection) { await connection.rollback(); From e8c87c488f1762e46fca0542638b0302fce550f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Mon, 14 Oct 2024 13:18:31 +0200 Subject: [PATCH 005/164] chore: Stop reporting to Sentry leader key renewals (#11246) --- .../cli/src/services/orchestration/main/multi-main-setup.ee.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/cli/src/services/orchestration/main/multi-main-setup.ee.ts b/packages/cli/src/services/orchestration/main/multi-main-setup.ee.ts index bb1b52519c..3b104cc1e3 100644 --- a/packages/cli/src/services/orchestration/main/multi-main-setup.ee.ts +++ b/packages/cli/src/services/orchestration/main/multi-main-setup.ee.ts @@ -1,5 +1,4 @@ import { InstanceSettings } from 'n8n-core'; -import { ErrorReporterProxy as EventReporter } from 'n8n-workflow'; import { Service } from 'typedi'; import config from '@/config'; @@ -74,7 +73,7 @@ export class MultiMainSetup extends TypedEmitter { this.emit('leader-stepdown'); // lost leadership - stop triggers, pollers, pruning, wait-tracking, queue recovery - EventReporter.info('[Multi-main setup] Leader failed to renew leader key'); + this.logger.warn('[Multi-main setup] Leader failed to renew leader key'); } return; From a7fc7fc22997acec86dc94386c95349fd018f4ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Mon, 14 Oct 2024 13:26:13 +0200 Subject: [PATCH 006/164] fix(core): Ensure error reporter does not promote `info` to `error` messages (#11245) --- packages/workflow/src/ErrorReporterProxy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/workflow/src/ErrorReporterProxy.ts b/packages/workflow/src/ErrorReporterProxy.ts index 7d715d08b5..dd5fe9515c 100644 --- a/packages/workflow/src/ErrorReporterProxy.ts +++ b/packages/workflow/src/ErrorReporterProxy.ts @@ -35,7 +35,7 @@ export const error = (e: unknown, options?: ReportingOptions) => { export const info = (msg: string, options?: ReportingOptions) => { Logger.info(msg); - instance.report(msg, options); + instance.report(msg, { ...options, level: 'info' }); }; export const warn = (warning: Error | string, options?: ReportingOptions) => From 3d97f02a8d2b6e5bc7c97c5271bed97417ecacd2 Mon Sep 17 00:00:00 2001 From: Jon Date: Mon, 14 Oct 2024 14:13:15 +0100 Subject: [PATCH 007/164] fix(Google Ads Node): Update to use v17 api (#11243) --- packages/nodes-base/nodes/Google/Ads/CampaignDescription.ts | 4 ++-- packages/nodes-base/nodes/Google/Ads/GoogleAds.node.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/nodes-base/nodes/Google/Ads/CampaignDescription.ts b/packages/nodes-base/nodes/Google/Ads/CampaignDescription.ts index 204486c250..ca30e47a3a 100644 --- a/packages/nodes-base/nodes/Google/Ads/CampaignDescription.ts +++ b/packages/nodes-base/nodes/Google/Ads/CampaignDescription.ts @@ -41,7 +41,7 @@ export const campaignOperations: INodeProperties[] = [ routing: { request: { method: 'POST', - url: '={{"/v15/customers/" + $parameter["clientCustomerId"].toString().replace(/-/g, "") + "/googleAds:search"}}', + url: '={{"/v17/customers/" + $parameter["clientCustomerId"].toString().replace(/-/g, "") + "/googleAds:search"}}', body: { query: '={{ "' + @@ -89,7 +89,7 @@ export const campaignOperations: INodeProperties[] = [ routing: { request: { method: 'POST', - url: '={{"/v15/customers/" + $parameter["clientCustomerId"].toString().replace(/-/g, "") + "/googleAds:search"}}', + url: '={{"/v17/customers/" + $parameter["clientCustomerId"].toString().replace(/-/g, "") + "/googleAds:search"}}', returnFullResponse: true, body: { query: diff --git a/packages/nodes-base/nodes/Google/Ads/GoogleAds.node.ts b/packages/nodes-base/nodes/Google/Ads/GoogleAds.node.ts index 06e678e656..606e58e77d 100644 --- a/packages/nodes-base/nodes/Google/Ads/GoogleAds.node.ts +++ b/packages/nodes-base/nodes/Google/Ads/GoogleAds.node.ts @@ -23,7 +23,7 @@ export class GoogleAds implements INodeType { testedBy: { request: { method: 'GET', - url: '/v15/customers:listAccessibleCustomers', + url: '/v17/customers:listAccessibleCustomers', }, }, }, From 873851b54e431962ca523baace19af16d12f41fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Mon, 14 Oct 2024 15:15:42 +0200 Subject: [PATCH 008/164] refactor(core): Revamp logs for scaling mode (#11244) --- packages/cli/src/commands/start.ts | 7 +- packages/cli/src/commands/worker.ts | 24 ++--- .../worker-missing-encryption-key.error.ts | 14 +++ .../__tests__/publisher.service.test.ts | 12 ++- .../scaling/__tests__/worker-server.test.ts | 13 +-- packages/cli/src/scaling/job-processor.ts | 54 ++++++++--- .../src/scaling/pubsub/publisher.service.ts | 6 +- .../cli/src/scaling/pubsub/pubsub.types.ts | 6 +- .../src/scaling/pubsub/subscriber.service.ts | 35 ++++--- packages/cli/src/scaling/scaling.service.ts | 93 ++++++++++++++----- packages/cli/src/scaling/scaling.types.ts | 38 ++++++-- packages/cli/src/scaling/worker-server.ts | 8 ++ packages/cli/src/workflow-runner.ts | 3 +- packages/core/src/DirectoryLoader.ts | 4 +- packages/workflow/src/ErrorReporterProxy.ts | 9 +- .../workflow/src/errors/application.error.ts | 1 + 16 files changed, 230 insertions(+), 97 deletions(-) create mode 100644 packages/cli/src/errors/worker-missing-encryption-key.error.ts diff --git a/packages/cli/src/commands/start.ts b/packages/cli/src/commands/start.ts index 428f451fdc..21e277a5dc 100644 --- a/packages/cli/src/commands/start.ts +++ b/packages/cli/src/commands/start.ts @@ -174,8 +174,9 @@ export class Start extends BaseCommand { this.logger.info('Initializing n8n process'); if (config.getEnv('executions.mode') === 'queue') { - this.logger.debug('Main Instance running in queue mode'); - this.logger.debug(`Queue mode id: ${this.queueModeId}`); + const scopedLogger = this.logger.withScope('scaling'); + scopedLogger.debug('Starting main instance in scaling mode'); + scopedLogger.debug(`Host ID: ${this.queueModeId}`); } const { flags } = await this.parse(Start); @@ -260,6 +261,8 @@ export class Start extends BaseCommand { await subscriber.subscribe('n8n.commands'); await subscriber.subscribe('n8n.worker-response'); + this.logger.withScope('scaling').debug('Pubsub setup completed'); + if (!orchestrationService.isMultiMainSetupEnabled) return; orchestrationService.multiMainSetup diff --git a/packages/cli/src/commands/worker.ts b/packages/cli/src/commands/worker.ts index 528951be4a..1685fbb034 100644 --- a/packages/cli/src/commands/worker.ts +++ b/packages/cli/src/commands/worker.ts @@ -1,13 +1,13 @@ import { Flags, type Config } from '@oclif/core'; -import { ApplicationError } from 'n8n-workflow'; import { Container } from 'typedi'; import config from '@/config'; import { N8N_VERSION, inTest } from '@/constants'; +import { WorkerMissingEncryptionKey } from '@/errors/worker-missing-encryption-key.error'; import { EventMessageGeneric } from '@/eventbus/event-message-classes/event-message-generic'; import { MessageEventBus } from '@/eventbus/message-event-bus/message-event-bus'; import { LogStreamingEventRelay } from '@/events/relays/log-streaming.event-relay'; -import { JobProcessor } from '@/scaling/job-processor'; +import { Logger } from '@/logging/logger.service'; import { PubSubHandler } from '@/scaling/pubsub/pubsub-handler'; import { Subscriber } from '@/scaling/pubsub/subscriber.service'; import type { ScalingService } from '@/scaling/scaling.service'; @@ -39,8 +39,6 @@ export class Worker extends BaseCommand { scalingService: ScalingService; - jobProcessor: JobProcessor; - override needsCommunityPackages = true; /** @@ -49,25 +47,23 @@ export class Worker extends BaseCommand { * get removed. */ async stopProcess() { - this.logger.info('Stopping n8n...'); + this.logger.info('Stopping worker...'); try { await this.externalHooks?.run('n8n.stop', []); } catch (error) { - await this.exitWithCrash('There was an error shutting down n8n.', error); + await this.exitWithCrash('Error shutting down worker', error); } await this.exitSuccessFully(); } constructor(argv: string[], cmdConfig: Config) { + if (!process.env.N8N_ENCRYPTION_KEY) throw new WorkerMissingEncryptionKey(); + super(argv, cmdConfig); - if (!process.env.N8N_ENCRYPTION_KEY) { - throw new ApplicationError( - 'Missing encryption key. Worker started without the required N8N_ENCRYPTION_KEY env var. More information: https://docs.n8n.io/hosting/configuration/configuration-examples/encryption-key/', - ); - } + this.logger = Container.get(Logger).withScope('scaling'); this.setInstanceQueueModeId(); } @@ -84,7 +80,7 @@ export class Worker extends BaseCommand { await this.initCrashJournal(); this.logger.debug('Starting n8n worker...'); - this.logger.debug(`Queue mode id: ${this.queueModeId}`); + this.logger.debug(`Host ID: ${this.queueModeId}`); await this.setConcurrency(); await super.init(); @@ -133,6 +129,8 @@ export class Worker extends BaseCommand { Container.get(PubSubHandler).init(); await Container.get(Subscriber).subscribe('n8n.commands'); + + this.logger.withScope('scaling').debug('Pubsub setup ready'); } async setConcurrency() { @@ -150,8 +148,6 @@ export class Worker extends BaseCommand { await this.scalingService.setupQueue(); this.scalingService.setupWorker(this.concurrency); - - this.jobProcessor = Container.get(JobProcessor); } async run() { diff --git a/packages/cli/src/errors/worker-missing-encryption-key.error.ts b/packages/cli/src/errors/worker-missing-encryption-key.error.ts new file mode 100644 index 0000000000..88ec11877a --- /dev/null +++ b/packages/cli/src/errors/worker-missing-encryption-key.error.ts @@ -0,0 +1,14 @@ +import { ApplicationError } from 'n8n-workflow'; + +export class WorkerMissingEncryptionKey extends ApplicationError { + constructor() { + super( + [ + 'Failed to start worker because of missing encryption key.', + 'Please set the `N8N_ENCRYPTION_KEY` env var when starting the worker.', + 'See: https://docs.n8n.io/hosting/configuration/configuration-examples/encryption-key/', + ].join(''), + { level: 'warning' }, + ); + } +} diff --git a/packages/cli/src/scaling/__tests__/publisher.service.test.ts b/packages/cli/src/scaling/__tests__/publisher.service.test.ts index 05bb52bc6a..af8ff9f0c1 100644 --- a/packages/cli/src/scaling/__tests__/publisher.service.test.ts +++ b/packages/cli/src/scaling/__tests__/publisher.service.test.ts @@ -4,6 +4,7 @@ import { mock } from 'jest-mock-extended'; import config from '@/config'; import { generateNanoId } from '@/databases/utils/generators'; import type { RedisClientService } from '@/services/redis-client.service'; +import { mockLogger } from '@test/mocking'; import { Publisher } from '../pubsub/publisher.service'; import type { PubSub } from '../pubsub/pubsub.types'; @@ -18,18 +19,19 @@ describe('Publisher', () => { }); const client = mock(); + const logger = mockLogger(); const redisClientService = mock({ createClient: () => client }); describe('constructor', () => { it('should init Redis client in scaling mode', () => { - const publisher = new Publisher(mock(), redisClientService); + const publisher = new Publisher(logger, redisClientService); expect(publisher.getClient()).toEqual(client); }); it('should not init Redis client in regular mode', () => { config.set('executions.mode', 'regular'); - const publisher = new Publisher(mock(), redisClientService); + const publisher = new Publisher(logger, redisClientService); expect(publisher.getClient()).toBeUndefined(); }); @@ -37,7 +39,7 @@ describe('Publisher', () => { describe('shutdown', () => { it('should disconnect Redis client', () => { - const publisher = new Publisher(mock(), redisClientService); + const publisher = new Publisher(logger, redisClientService); publisher.shutdown(); expect(client.disconnect).toHaveBeenCalled(); }); @@ -45,7 +47,7 @@ describe('Publisher', () => { describe('publishCommand', () => { it('should publish command into `n8n.commands` pubsub channel', async () => { - const publisher = new Publisher(mock(), redisClientService); + const publisher = new Publisher(logger, redisClientService); const msg = mock({ command: 'reload-license' }); await publisher.publishCommand(msg); @@ -59,7 +61,7 @@ describe('Publisher', () => { describe('publishWorkerResponse', () => { it('should publish worker response into `n8n.worker-response` pubsub channel', async () => { - const publisher = new Publisher(mock(), redisClientService); + const publisher = new Publisher(logger, redisClientService); const msg = mock({ response: 'response-to-get-worker-status', }); diff --git a/packages/cli/src/scaling/__tests__/worker-server.test.ts b/packages/cli/src/scaling/__tests__/worker-server.test.ts index 778d403bf2..8bcdd3aa5c 100644 --- a/packages/cli/src/scaling/__tests__/worker-server.test.ts +++ b/packages/cli/src/scaling/__tests__/worker-server.test.ts @@ -8,6 +8,7 @@ import * as http from 'node:http'; import type { ExternalHooks } from '@/external-hooks'; import type { PrometheusMetricsService } from '@/metrics/prometheus-metrics.service'; import { bodyParser, rawBodyReader } from '@/middlewares'; +import { mockLogger } from '@test/mocking'; import { WorkerServer } from '../worker-server'; @@ -48,7 +49,7 @@ describe('WorkerServer', () => { () => new WorkerServer( globalConfig, - mock(), + mockLogger(), mock(), externalHooks, mock({ instanceType: 'webhook' }), @@ -73,7 +74,7 @@ describe('WorkerServer', () => { new WorkerServer( globalConfig, - mock(), + mockLogger(), mock(), externalHooks, instanceSettings, @@ -100,7 +101,7 @@ describe('WorkerServer', () => { const workerServer = new WorkerServer( globalConfig, - mock(), + mockLogger(), mock(), externalHooks, instanceSettings, @@ -135,7 +136,7 @@ describe('WorkerServer', () => { const workerServer = new WorkerServer( globalConfig, - mock(), + mockLogger(), mock(), externalHooks, instanceSettings, @@ -156,7 +157,7 @@ describe('WorkerServer', () => { const workerServer = new WorkerServer( globalConfig, - mock(), + mockLogger(), mock(), externalHooks, instanceSettings, @@ -174,7 +175,7 @@ describe('WorkerServer', () => { const workerServer = new WorkerServer( globalConfig, - mock(), + mockLogger(), mock(), externalHooks, instanceSettings, diff --git a/packages/cli/src/scaling/job-processor.ts b/packages/cli/src/scaling/job-processor.ts index 49e1383ac6..e11395002b 100644 --- a/packages/cli/src/scaling/job-processor.ts +++ b/packages/cli/src/scaling/job-processor.ts @@ -12,7 +12,14 @@ import { Logger } from '@/logging/logger.service'; import { NodeTypes } from '@/node-types'; import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data'; -import type { Job, JobId, JobResult, RunningJob } from './scaling.types'; +import type { + Job, + JobFinishedMessage, + JobId, + JobResult, + RespondToWebhookMessage, + RunningJob, +} from './scaling.types'; /** * Responsible for processing jobs from the queue, i.e. running enqueued executions. @@ -26,7 +33,9 @@ export class JobProcessor { private readonly executionRepository: ExecutionRepository, private readonly workflowRepository: WorkflowRepository, private readonly nodeTypes: NodeTypes, - ) {} + ) { + this.logger = this.logger.withScope('scaling'); + } async processJob(job: Job): Promise { const { executionId, loadStaticData } = job.data; @@ -37,15 +46,18 @@ export class JobProcessor { }); if (!execution) { - this.logger.error('[JobProcessor] Failed to find execution data', { executionId }); - throw new ApplicationError('Failed to find execution data. Aborting execution.', { - extra: { executionId }, - }); + throw new ApplicationError( + `Worker failed to find data for execution ${executionId} (job ${job.id})`, + { level: 'warning' }, + ); } const workflowId = execution.workflowData.id; - this.logger.info(`[JobProcessor] Starting job ${job.id} (execution ${executionId})`); + this.logger.info(`Worker started execution ${executionId} (job ${job.id})`, { + executionId, + jobId: job.id, + }); const startedAt = await this.executionRepository.setRunning(executionId); @@ -58,8 +70,10 @@ export class JobProcessor { }); if (workflowData === null) { - this.logger.error('[JobProcessor] Failed to find workflow', { workflowId, executionId }); - throw new ApplicationError('Failed to find workflow', { extra: { workflowId } }); + throw new ApplicationError( + `Worker failed to find workflow ${workflowId} to run execution ${executionId} (job ${job.id})`, + { level: 'warning' }, + ); } staticData = workflowData.staticData; @@ -102,11 +116,14 @@ export class JobProcessor { additionalData.hooks.hookFunctions.sendResponse = [ async (response: IExecuteResponsePromiseData): Promise => { - await job.progress({ + const msg: RespondToWebhookMessage = { kind: 'respond-to-webhook', executionId, response: this.encodeWebhookResponse(response), - }); + workerId: config.getEnv('redis.queueModeId'), + }; + + await job.progress(msg); }, ]; @@ -115,7 +132,7 @@ export class JobProcessor { additionalData.setExecutionStatus = (status: ExecutionStatus) => { // Can't set the status directly in the queued worker, but it will happen in InternalHook.onWorkflowPostExecute this.logger.debug( - `[JobProcessor] Queued worker execution status for ${executionId} is "${status}"`, + `Queued worker execution status for execution ${executionId} (job ${job.id}) is "${status}"`, ); }; @@ -148,7 +165,18 @@ export class JobProcessor { delete this.runningJobs[job.id]; - this.logger.debug('[JobProcessor] Job finished running', { jobId: job.id, executionId }); + this.logger.info(`Worker finished execution ${executionId} (job ${job.id})`, { + executionId, + jobId: job.id, + }); + + const msg: JobFinishedMessage = { + kind: 'job-finished', + executionId, + workerId: config.getEnv('redis.queueModeId'), + }; + + await job.progress(msg); /** * @important Do NOT call `workflowExecuteAfter` hook here. diff --git a/packages/cli/src/scaling/pubsub/publisher.service.ts b/packages/cli/src/scaling/pubsub/publisher.service.ts index 29d31989ff..cc25304e2c 100644 --- a/packages/cli/src/scaling/pubsub/publisher.service.ts +++ b/packages/cli/src/scaling/pubsub/publisher.service.ts @@ -24,6 +24,8 @@ export class Publisher { // @TODO: Once this class is only ever initialized in scaling mode, throw in the next line instead. if (config.getEnv('executions.mode') !== 'queue') return; + this.logger = this.logger.withScope('scaling'); + this.client = this.redisClientService.createClient({ type: 'publisher(n8n)' }); } @@ -55,11 +57,11 @@ export class Publisher { this.logger.debug(`Published ${msg.command} to command channel`); } - /** Publish a response for a command into the `n8n.worker-response` channel. */ + /** Publish a response to a command into the `n8n.worker-response` channel. */ async publishWorkerResponse(msg: PubSub.WorkerResponse) { await this.client.publish('n8n.worker-response', JSON.stringify(msg)); - this.logger.debug(`Published response ${msg.response} to worker response channel`); + this.logger.debug(`Published ${msg.response} to worker response channel`); } // #endregion diff --git a/packages/cli/src/scaling/pubsub/pubsub.types.ts b/packages/cli/src/scaling/pubsub/pubsub.types.ts index b4d6e1a962..eec0110201 100644 --- a/packages/cli/src/scaling/pubsub/pubsub.types.ts +++ b/packages/cli/src/scaling/pubsub/pubsub.types.ts @@ -88,7 +88,7 @@ export namespace PubSub { /** Content of worker response. */ response: WorkerResponseKey; - /** Whether the command should be debounced when received. */ + /** Whether the worker response should be debounced when received. */ debounce?: boolean; } & (PubSubWorkerResponseMap[WorkerResponseKey] extends never ? { payload?: never } // some responses carry no payload @@ -101,6 +101,10 @@ export namespace PubSub { /** Response sent via the `n8n.worker-response` pubsub channel. */ export type WorkerResponse = ToWorkerResponse<'response-to-get-worker-status'>; + // ---------------------------------- + // events + // ---------------------------------- + /** * Of all events emitted from pubsub messages, those whose handlers * are all present in main, worker, and webhook processes. diff --git a/packages/cli/src/scaling/pubsub/subscriber.service.ts b/packages/cli/src/scaling/pubsub/subscriber.service.ts index 7c7f90fb0e..207c726370 100644 --- a/packages/cli/src/scaling/pubsub/subscriber.service.ts +++ b/packages/cli/src/scaling/pubsub/subscriber.service.ts @@ -17,8 +17,6 @@ import type { PubSub } from './pubsub.types'; export class Subscriber { private readonly client: SingleNodeClient | MultiNodeClient; - // #region Lifecycle - constructor( private readonly logger: Logger, private readonly redisClientService: RedisClientService, @@ -27,6 +25,8 @@ export class Subscriber { // @TODO: Once this class is only ever initialized in scaling mode, throw in the next line instead. if (config.getEnv('executions.mode') !== 'queue') return; + this.logger = this.logger.withScope('scaling'); + this.client = this.redisClientService.createClient({ type: 'subscriber(n8n)' }); const handlerFn = (msg: PubSub.Command | PubSub.WorkerResponse) => { @@ -36,8 +36,8 @@ export class Subscriber { const debouncedHandlerFn = debounce(handlerFn, 300); - this.client.on('message', (_channel: PubSub.Channel, str) => { - const msg = this.parseMessage(str); + this.client.on('message', (channel: PubSub.Channel, str) => { + const msg = this.parseMessage(str, channel); if (!msg) return; if (msg.debounce) debouncedHandlerFn(msg); else handlerFn(msg); @@ -53,31 +53,27 @@ export class Subscriber { this.client.disconnect(); } - // #endregion - - // #region Subscribing - async subscribe(channel: PubSub.Channel) { await this.client.subscribe(channel, (error) => { if (error) { - this.logger.error('Failed to subscribe to channel', { channel, cause: error }); + this.logger.error(`Failed to subscribe to channel ${channel}`, { error }); return; } - this.logger.debug('Subscribed to channel', { channel }); + this.logger.debug(`Subscribed to channel ${channel}`); }); } - // #region Commands - - private parseMessage(str: string) { + private parseMessage(str: string, channel: PubSub.Channel) { const msg = jsonParse(str, { fallbackValue: null, }); if (!msg) { - this.logger.debug('Received invalid string via pubsub channel', { message: str }); - + this.logger.error(`Received malformed message via channel ${channel}`, { + msg: str, + channel, + }); return null; } @@ -91,10 +87,13 @@ export class Subscriber { return null; } - this.logger.debug('Received message via pubsub channel', msg); + const msgName = 'command' in msg ? msg.command : msg.response; + + this.logger.debug(`Received message ${msgName} via channel ${channel}`, { + msg, + channel, + }); return msg; } - - // #endregion } diff --git a/packages/cli/src/scaling/scaling.service.ts b/packages/cli/src/scaling/scaling.service.ts index f35b4348a6..5edf43eeac 100644 --- a/packages/cli/src/scaling/scaling.service.ts +++ b/packages/cli/src/scaling/scaling.service.ts @@ -6,6 +6,7 @@ import { sleep, jsonStringify, ErrorReporterProxy, + ensureError, } from 'n8n-workflow'; import type { IExecuteResponsePromiseData } from 'n8n-workflow'; import { strict } from 'node:assert'; @@ -20,6 +21,7 @@ import { MaxStalledCountError } from '@/errors/max-stalled-count.error'; import { EventService } from '@/events/event.service'; import { Logger } from '@/logging/logger.service'; import { OrchestrationService } from '@/services/orchestration.service'; +import { assertNever } from '@/utils'; import { JOB_TYPE_NAME, QUEUE_NAME } from './constants'; import { JobProcessor } from './job-processor'; @@ -31,7 +33,8 @@ import type { JobStatus, JobId, QueueRecoveryContext, - JobReport, + JobMessage, + JobFailedMessage, } from './scaling.types'; @Service() @@ -89,34 +92,46 @@ export class ScalingService { void this.queue.process(JOB_TYPE_NAME, concurrency, async (job: Job) => { try { await this.jobProcessor.processJob(job); - } catch (error: unknown) { - // Errors thrown here will be sent to the main instance by bull. Logging - // them out and rethrowing them allows to find out which worker had the - // issue. - this.logger.error('Executing a job errored', { - jobId: job.id, - executionId: job.data.executionId, - error, - }); - ErrorReporterProxy.error(error); - throw error; + } catch (error) { + await this.reportJobProcessingError(ensureError(error), job); } }); this.logger.debug('Worker setup completed'); } + private async reportJobProcessingError(error: Error, job: Job) { + const { executionId } = job.data; + + this.logger.error(`Worker errored while running execution ${executionId} (job ${job.id})`, { + error, + executionId, + jobId: job.id, + }); + + const msg: JobFailedMessage = { + kind: 'job-failed', + executionId, + workerId: config.getEnv('redis.queueModeId'), + errorMsg: error.message, + }; + + await job.progress(msg); + + ErrorReporterProxy.error(error, { executionId }); + + throw error; + } + @OnShutdown(HIGHEST_SHUTDOWN_PRIORITY) async stop() { - await this.queue.pause(true, true); + await this.queue.pause(true, true); // no more jobs will be picked up this.logger.debug('Queue paused'); this.stopQueueRecovery(); this.stopQueueMetrics(); - this.logger.debug('Queue recovery and metrics stopped'); - let count = 0; while (this.getRunningJobsCount() !== 0) { @@ -161,7 +176,10 @@ export class ScalingService { const job = await this.queue.add(JOB_TYPE_NAME, jobData, jobOptions); - this.logger.info(`Added job ${job.id} (execution ${jobData.executionId})`); + const { executionId } = jobData; + const jobId = job.id; + + this.logger.info(`Enqueued execution ${executionId} (job ${jobId})`, { executionId, jobId }); return job; } @@ -218,7 +236,7 @@ export class ScalingService { */ private registerWorkerListeners() { this.queue.on('global:progress', (jobId: JobId, msg: unknown) => { - if (!this.isPubSubMessage(msg)) return; + if (!this.isJobMessage(msg)) return; if (msg.kind === 'abort-job') this.jobProcessor.stopJob(jobId); }); @@ -258,12 +276,36 @@ export class ScalingService { throw error; }); - this.queue.on('global:progress', (_jobId: JobId, msg: unknown) => { - if (!this.isPubSubMessage(msg)) return; + this.queue.on('global:progress', (jobId: JobId, msg: unknown) => { + if (!this.isJobMessage(msg)) return; - if (msg.kind === 'respond-to-webhook') { - const decodedResponse = this.decodeWebhookResponse(msg.response); - this.activeExecutions.resolveResponsePromise(msg.executionId, decodedResponse); + // completion and failure are reported via `global:progress` to convey more details + // than natively provided by Bull in `global:completed` and `global:failed` events + + switch (msg.kind) { + case 'respond-to-webhook': + const decodedResponse = this.decodeWebhookResponse(msg.response); + this.activeExecutions.resolveResponsePromise(msg.executionId, decodedResponse); + break; + case 'job-finished': + this.logger.info(`Execution ${msg.executionId} (job ${jobId}) finished successfully`, { + workerId: msg.workerId, + executionId: msg.executionId, + jobId, + }); + break; + case 'job-failed': + this.logger.error(`Execution ${msg.executionId} (job ${jobId}) failed`, { + workerId: msg.workerId, + errorMsg: msg.errorMsg, + executionId: msg.executionId, + jobId, + }); + break; + case 'abort-job': + break; // only for worker + default: + assertNever(msg); } }); @@ -273,7 +315,8 @@ export class ScalingService { } } - private isPubSubMessage(candidate: unknown): candidate is JobReport { + /** Whether the argument is a message sent via Bull's internal pubsub setup. */ + private isJobMessage(candidate: unknown): candidate is JobMessage { return typeof candidate === 'object' && candidate !== null && 'kind' in candidate; } @@ -345,6 +388,8 @@ export class ScalingService { if (this.queueMetricsInterval) { clearInterval(this.queueMetricsInterval); this.queueMetricsInterval = undefined; + + this.logger.debug('Queue metrics collection stopped'); } } @@ -379,6 +424,8 @@ export class ScalingService { private stopQueueRecovery() { clearTimeout(this.queueRecoveryContext.timeout); + + this.logger.debug('Queue recovery stopped'); } /** diff --git a/packages/cli/src/scaling/scaling.types.ts b/packages/cli/src/scaling/scaling.types.ts index fa8210450f..38cb6805de 100644 --- a/packages/cli/src/scaling/scaling.types.ts +++ b/packages/cli/src/scaling/scaling.types.ts @@ -23,19 +23,43 @@ export type JobStatus = Bull.JobStatus; export type JobOptions = Bull.JobOptions; -export type JobReport = JobReportToMain | JobReportToWorker; +/** + * Message sent by main to worker and vice versa about a job. `JobMessage` is + * sent via Bull's internal pubsub setup - do not confuse with `PubSub.Command` + * and `PubSub.Response`, which are sent via n8n's own pubsub setup to keep + * main and worker processes in sync outside of a job's lifecycle. + */ +export type JobMessage = + | RespondToWebhookMessage + | JobFinishedMessage + | JobFailedMessage + | AbortJobMessage; -type JobReportToMain = RespondToWebhookMessage; - -type JobReportToWorker = AbortJobMessage; - -type RespondToWebhookMessage = { +/** Message sent by worker to main to respond to a webhook. */ +export type RespondToWebhookMessage = { kind: 'respond-to-webhook'; executionId: string; response: IExecuteResponsePromiseData; + workerId: string; }; -type AbortJobMessage = { +/** Message sent by worker to main to report a job has finished successfully. */ +export type JobFinishedMessage = { + kind: 'job-finished'; + executionId: string; + workerId: string; +}; + +/** Message sent by worker to main to report a job has failed. */ +export type JobFailedMessage = { + kind: 'job-failed'; + executionId: string; + workerId: string; + errorMsg: string; +}; + +/** Message sent by main to worker to abort a job. */ +export type AbortJobMessage = { kind: 'abort-job'; }; diff --git a/packages/cli/src/scaling/worker-server.ts b/packages/cli/src/scaling/worker-server.ts index 3cf6995882..0af948670f 100644 --- a/packages/cli/src/scaling/worker-server.ts +++ b/packages/cli/src/scaling/worker-server.ts @@ -58,6 +58,8 @@ export class WorkerServer { ) { assert(this.instanceSettings.instanceType === 'worker'); + this.logger = this.logger.withScope('scaling'); + this.app = express(); this.app.disable('x-powered-by'); @@ -84,6 +86,10 @@ export class WorkerServer { await this.mountEndpoints(); + this.logger.debug('Worker server initialized', { + endpoints: Object.keys(this.endpointsConfig), + }); + await new Promise((resolve) => this.server.listen(this.port, this.address, resolve)); await this.externalHooks.run('worker.ready'); @@ -141,6 +147,8 @@ export class WorkerServer { this.overwritesLoaded = true; + this.logger.debug('Worker loaded credentials overwrites'); + ResponseHelper.sendSuccessResponse(res, { success: true }, true, 200); } } diff --git a/packages/cli/src/workflow-runner.ts b/packages/cli/src/workflow-runner.ts index 8d1e147e85..0f1f37b71d 100644 --- a/packages/cli/src/workflow-runner.ts +++ b/packages/cli/src/workflow-runner.ts @@ -64,7 +64,7 @@ export class WorkflowRunner { executionId: string, hooks?: WorkflowHooks, ) { - ErrorReporter.error(error); + ErrorReporter.error(error, { executionId }); const isQueueMode = config.getEnv('executions.mode') === 'queue'; @@ -476,7 +476,6 @@ export class WorkflowRunner { clearWatchdogInterval(); } } catch (error) { - ErrorReporter.error(error); // We use "getWorkflowHooksWorkerExecuter" as "getWorkflowHooksWorkerMain" does not contain the // "workflowExecuteAfter" which we require. const hooks = WorkflowExecuteAdditionalData.getWorkflowHooksWorkerExecuter( diff --git a/packages/core/src/DirectoryLoader.ts b/packages/core/src/DirectoryLoader.ts index a1401a8fb5..b0e77125a7 100644 --- a/packages/core/src/DirectoryLoader.ts +++ b/packages/core/src/DirectoryLoader.ts @@ -448,9 +448,9 @@ export class LazyPackageDirectoryLoader extends PackageDirectoryLoader { ); } - Logger.debug(`Lazy Loading credentials and nodes from ${this.packageJson.name}`, { - credentials: this.types.credentials?.length ?? 0, + Logger.debug(`Lazy-loading nodes and credentials from ${this.packageJson.name}`, { nodes: this.types.nodes?.length ?? 0, + credentials: this.types.credentials?.length ?? 0, }); this.isLazyLoaded = true; diff --git a/packages/workflow/src/ErrorReporterProxy.ts b/packages/workflow/src/ErrorReporterProxy.ts index dd5fe9515c..cedb921d5e 100644 --- a/packages/workflow/src/ErrorReporterProxy.ts +++ b/packages/workflow/src/ErrorReporterProxy.ts @@ -6,12 +6,17 @@ interface ErrorReporter { } const instance: ErrorReporter = { - report: (error) => { + report: (error, options) => { if (error instanceof Error) { let e = error; + + const { executionId } = options ?? {}; + const context = executionId ? ` (execution ${executionId})` : ''; + do { + const msg = [e.message + context, e.stack ? `\n${e.stack}\n` : ''].join(''); const meta = e instanceof ApplicationError ? e.extra : undefined; - Logger.error(`${e.constructor.name}: ${e.message}`, meta); + Logger.error(msg, meta); e = e.cause as Error; } while (e); } diff --git a/packages/workflow/src/errors/application.error.ts b/packages/workflow/src/errors/application.error.ts index 7cd3095cf0..b8f54cf8b4 100644 --- a/packages/workflow/src/errors/application.error.ts +++ b/packages/workflow/src/errors/application.error.ts @@ -5,6 +5,7 @@ export type Level = 'warning' | 'error' | 'fatal' | 'info'; export type ReportingOptions = { level?: Level; + executionId?: string; } & Pick; export class ApplicationError extends Error { From b028d81390dd6374b7effea07b8c1b086f3ffed9 Mon Sep 17 00:00:00 2001 From: Val <68596159+valya@users.noreply.github.com> Date: Mon, 14 Oct 2024 14:19:17 +0100 Subject: [PATCH 009/164] feat: Start Task Runner via Launcher (no-changelog) (#11071) --- docker/images/n8n-custom/Dockerfile | 24 ++++++ docker/images/n8n/Dockerfile | 24 ++++++ docker/images/n8n/n8n-task-runners.json | 19 +++++ .../@n8n/config/src/configs/runners.config.ts | 10 +++ packages/@n8n/config/test/config.test.ts | 3 + .../cli/src/runners/task-runner-process.ts | 77 ++++++++++++++++--- .../runners/task-runner-process.test.ts | 52 +++++++++++++ 7 files changed, 198 insertions(+), 11 deletions(-) create mode 100644 docker/images/n8n/n8n-task-runners.json diff --git a/docker/images/n8n-custom/Dockerfile b/docker/images/n8n-custom/Dockerfile index ba271017d1..a533cdbdab 100644 --- a/docker/images/n8n-custom/Dockerfile +++ b/docker/images/n8n-custom/Dockerfile @@ -31,6 +31,30 @@ WORKDIR /home/node COPY --from=builder /compiled /usr/local/lib/node_modules/n8n COPY docker/images/n8n/docker-entrypoint.sh / +# Setup the Task Runner Launcher +ARG TARGETPLATFORM +ARG LAUNCHER_VERSION=0.1.1 +ENV N8N_RUNNERS_USE_LAUNCHER=true \ + N8N_RUNNERS_LAUNCHER_PATH=/usr/local/bin/task-runner-launcher +COPY docker/images/n8n/n8n-task-runners.json /etc/n8n-task-runners.json +# First, download, verify, then extract the launcher binary +# Second, chmod with 4555 to allow the use of setuid +# Third, create a new user and group to execute the Task Runners under +RUN \ + if [[ "$TARGETPLATFORM" = "linux/amd64" ]]; then export ARCH_NAME="x86_64"; \ + elif [[ "$TARGETPLATFORM" = "linux/arm64" ]]; then export ARCH_NAME="aarch64"; fi; \ + mkdir /launcher-temp && \ + cd /launcher-temp && \ + wget https://github.com/n8n-io/task-runner-launcher/releases/download/${LAUNCHER_VERSION}/task-runner-launcher-$ARCH_NAME-unknown-linux-musl.zip && \ + wget https://github.com/n8n-io/task-runner-launcher/releases/download/${LAUNCHER_VERSION}/task-runner-launcher-$ARCH_NAME-unknown-linux-musl.sha256 && \ + sha256sum -c task-runner-launcher-$ARCH_NAME-unknown-linux-musl.sha256 && \ + unzip -d $(dirname ${N8N_RUNNERS_LAUNCHER_PATH}) task-runner-launcher-$ARCH_NAME-unknown-linux-musl.zip task-runner-launcher && \ + cd - && \ + rm -r /launcher-temp && \ + chmod 4555 ${N8N_RUNNERS_LAUNCHER_PATH} && \ + addgroup -g 2000 task-runner && \ + adduser -D -u 2000 -g "Task Runner User" -G task-runner task-runner + RUN \ cd /usr/local/lib/node_modules/n8n && \ npm rebuild sqlite3 && \ diff --git a/docker/images/n8n/Dockerfile b/docker/images/n8n/Dockerfile index 2da1bc1f47..08e031cf5f 100644 --- a/docker/images/n8n/Dockerfile +++ b/docker/images/n8n/Dockerfile @@ -22,6 +22,30 @@ RUN set -eux; \ find /usr/local/lib/node_modules/n8n -type f -name "*.ts" -o -name "*.js.map" -o -name "*.vue" | xargs rm -f && \ rm -rf /root/.npm +# Setup the Task Runner Launcher +ARG TARGETPLATFORM +ARG LAUNCHER_VERSION=0.1.1 +ENV N8N_RUNNERS_USE_LAUNCHER=true \ + N8N_RUNNERS_LAUNCHER_PATH=/usr/local/bin/task-runner-launcher +COPY n8n-task-runners.json /etc/n8n-task-runners.json +# First, download, verify, then extract the launcher binary +# Second, chmod with 4555 to allow the use of setuid +# Third, create a new user and group to execute the Task Runners under +RUN \ + if [[ "$TARGETPLATFORM" = "linux/amd64" ]]; then export ARCH_NAME="x86_64"; \ + elif [[ "$TARGETPLATFORM" = "linux/arm64" ]]; then export ARCH_NAME="aarch64"; fi; \ + mkdir /launcher-temp && \ + cd /launcher-temp && \ + wget https://github.com/n8n-io/task-runner-launcher/releases/download/${LAUNCHER_VERSION}/task-runner-launcher-$ARCH_NAME-unknown-linux-musl.zip && \ + wget https://github.com/n8n-io/task-runner-launcher/releases/download/${LAUNCHER_VERSION}/task-runner-launcher-$ARCH_NAME-unknown-linux-musl.sha256 && \ + sha256sum -c task-runner-launcher-$ARCH_NAME-unknown-linux-musl.sha256 && \ + unzip -d $(dirname ${N8N_RUNNERS_LAUNCHER_PATH}) task-runner-launcher-$ARCH_NAME-unknown-linux-musl.zip task-runner-launcher && \ + cd - && \ + rm -r /launcher-temp && \ + chmod 4555 ${N8N_RUNNERS_LAUNCHER_PATH} && \ + addgroup -g 2000 task-runner && \ + adduser -D -u 2000 -g "Task Runner User" -G task-runner task-runner + COPY docker-entrypoint.sh / RUN \ diff --git a/docker/images/n8n/n8n-task-runners.json b/docker/images/n8n/n8n-task-runners.json new file mode 100644 index 0000000000..2dd65b67c8 --- /dev/null +++ b/docker/images/n8n/n8n-task-runners.json @@ -0,0 +1,19 @@ +{ + "task-runners": [ + { + "runner-type": "javascript", + "workdir": "/home/task-runner", + "command": "/usr/local/bin/node", + "args": ["/usr/local/lib/node_modules/n8n/node_modules/@n8n/task-runner/dist/start.js"], + "allowed-env": [ + "PATH", + "N8N_RUNNERS_GRANT_TOKEN", + "N8N_RUNNERS_N8N_URI", + "NODE_FUNCTION_ALLOW_BUILTIN", + "NODE_FUNCTION_ALLOW_EXTERNAL" + ], + "uid": 2000, + "gid": 2000 + } + ] +} diff --git a/packages/@n8n/config/src/configs/runners.config.ts b/packages/@n8n/config/src/configs/runners.config.ts index e7335e8827..14d1b01d1a 100644 --- a/packages/@n8n/config/src/configs/runners.config.ts +++ b/packages/@n8n/config/src/configs/runners.config.ts @@ -19,4 +19,14 @@ export class TaskRunnersConfig { /** IP address task runners server should listen on */ @Env('N8N_RUNNERS_SERVER_LISTEN_ADDRESS') listen_address: string = '127.0.0.1'; + + @Env('N8N_RUNNERS_USE_LAUNCHER') + useLauncher: boolean = false; + + @Env('N8N_RUNNERS_LAUNCHER_PATH') + launcherPath: string = ''; + + /** Which task runner to launch from the config */ + @Env('N8N_RUNNERS_LAUNCHER_RUNNER') + launcherRunner: string = 'javascript'; } diff --git a/packages/@n8n/config/test/config.test.ts b/packages/@n8n/config/test/config.test.ts index 301022ca3e..56f3bc6de7 100644 --- a/packages/@n8n/config/test/config.test.ts +++ b/packages/@n8n/config/test/config.test.ts @@ -228,6 +228,9 @@ describe('GlobalConfig', () => { authToken: '', listen_address: '127.0.0.1', port: 5679, + useLauncher: false, + launcherPath: '', + launcherRunner: 'javascript', }, sentry: { backendDsn: '', diff --git a/packages/cli/src/runners/task-runner-process.ts b/packages/cli/src/runners/task-runner-process.ts index 9f570fcb38..857d581127 100644 --- a/packages/cli/src/runners/task-runner-process.ts +++ b/packages/cli/src/runners/task-runner-process.ts @@ -39,17 +39,11 @@ export class TaskRunnerProcess { a.ok(!this.process, 'Task Runner Process already running'); const grantToken = await this.authService.createGrantToken(); - const startScript = require.resolve('@n8n/task-runner'); - this.process = spawn('node', [startScript], { - env: { - PATH: process.env.PATH, - N8N_RUNNERS_GRANT_TOKEN: grantToken, - N8N_RUNNERS_N8N_URI: `127.0.0.1:${this.globalConfig.taskRunners.port}`, - NODE_FUNCTION_ALLOW_BUILTIN: process.env.NODE_FUNCTION_ALLOW_BUILTIN, - NODE_FUNCTION_ALLOW_EXTERNAL: process.env.NODE_FUNCTION_ALLOW_EXTERNAL, - }, - }); + const n8nUri = `127.0.0.1:${this.globalConfig.taskRunners.port}`; + this.process = this.globalConfig.taskRunners.useLauncher + ? this.startLauncher(grantToken, n8nUri) + : this.startNode(grantToken, n8nUri); this.process.stdout?.pipe(process.stdout); this.process.stderr?.pipe(process.stderr); @@ -57,6 +51,38 @@ export class TaskRunnerProcess { this.monitorProcess(this.process); } + startNode(grantToken: string, n8nUri: string) { + const startScript = require.resolve('@n8n/task-runner'); + + return spawn('node', [startScript], { + env: { + PATH: process.env.PATH, + N8N_RUNNERS_GRANT_TOKEN: grantToken, + N8N_RUNNERS_N8N_URI: n8nUri, + NODE_FUNCTION_ALLOW_BUILTIN: process.env.NODE_FUNCTION_ALLOW_BUILTIN, + NODE_FUNCTION_ALLOW_EXTERNAL: process.env.NODE_FUNCTION_ALLOW_EXTERNAL, + }, + }); + } + + startLauncher(grantToken: string, n8nUri: string) { + return spawn( + this.globalConfig.taskRunners.launcherPath, + ['launch', this.globalConfig.taskRunners.launcherRunner], + { + env: { + PATH: process.env.PATH, + N8N_RUNNERS_GRANT_TOKEN: grantToken, + N8N_RUNNERS_N8N_URI: n8nUri, + NODE_FUNCTION_ALLOW_BUILTIN: process.env.NODE_FUNCTION_ALLOW_BUILTIN, + NODE_FUNCTION_ALLOW_EXTERNAL: process.env.NODE_FUNCTION_ALLOW_EXTERNAL, + // For debug logging if enabled + RUST_LOG: process.env.RUST_LOG, + }, + }, + ); + } + @OnShutdown() async stop() { if (!this.process) { @@ -66,12 +92,41 @@ export class TaskRunnerProcess { this.isShuttingDown = true; // TODO: Timeout & force kill - this.process.kill(); + if (this.globalConfig.taskRunners.useLauncher) { + await this.killLauncher(); + } else { + this.killNode(); + } await this.runPromise; this.isShuttingDown = false; } + killNode() { + if (!this.process) { + return; + } + this.process.kill(); + } + + async killLauncher() { + if (!this.process?.pid) { + return; + } + + const killProcess = spawn(this.globalConfig.taskRunners.launcherPath, [ + 'kill', + this.globalConfig.taskRunners.launcherRunner, + this.process.pid.toString(), + ]); + + await new Promise((resolve) => { + killProcess.on('exit', () => { + resolve(); + }); + }); + } + private monitorProcess(taskRunnerProcess: ChildProcess) { this.runPromise = new Promise((resolve) => { taskRunnerProcess.on('exit', (code) => { diff --git a/packages/cli/test/integration/runners/task-runner-process.test.ts b/packages/cli/test/integration/runners/task-runner-process.test.ts index f517ee6398..e623d5f371 100644 --- a/packages/cli/test/integration/runners/task-runner-process.test.ts +++ b/packages/cli/test/integration/runners/task-runner-process.test.ts @@ -18,6 +18,11 @@ describe('TaskRunnerProcess', () => { const taskBroker = Container.get(TaskBroker); const taskRunnerService = Container.get(TaskRunnerService); + const startLauncherSpy = jest.spyOn(runnerProcess, 'startLauncher'); + const startNodeSpy = jest.spyOn(runnerProcess, 'startNode'); + const killLauncherSpy = jest.spyOn(runnerProcess, 'killLauncher'); + const killNodeSpy = jest.spyOn(runnerProcess, 'killNode'); + beforeAll(async () => { await taskRunnerServer.start(); // Set the port to the actually used port @@ -30,6 +35,11 @@ describe('TaskRunnerProcess', () => { afterEach(async () => { await runnerProcess.stop(); + + startLauncherSpy.mockClear(); + startNodeSpy.mockClear(); + killLauncherSpy.mockClear(); + killNodeSpy.mockClear(); }); const getNumConnectedRunners = () => taskRunnerService.runnerConnections.size; @@ -88,4 +98,46 @@ describe('TaskRunnerProcess', () => { expect(getNumConnectedRunners()).toBe(1); expect(getNumRegisteredRunners()).toBe(1); }); + + it('should launch runner directly if not using a launcher', async () => { + globalConfig.taskRunners.useLauncher = false; + + await runnerProcess.start(); + + expect(startLauncherSpy).toBeCalledTimes(0); + expect(startNodeSpy).toBeCalledTimes(1); + }); + + it('should use a launcher if configured', async () => { + globalConfig.taskRunners.useLauncher = true; + globalConfig.taskRunners.launcherPath = 'node'; + + await runnerProcess.start(); + + expect(startLauncherSpy).toBeCalledTimes(1); + expect(startNodeSpy).toBeCalledTimes(0); + globalConfig.taskRunners.useLauncher = false; + }); + + it('should kill the process directly if not using a launcher', async () => { + globalConfig.taskRunners.useLauncher = false; + + await runnerProcess.start(); + await runnerProcess.stop(); + + expect(killLauncherSpy).toBeCalledTimes(0); + expect(killNodeSpy).toBeCalledTimes(1); + }); + + it('should kill the process using a launcher if configured', async () => { + globalConfig.taskRunners.useLauncher = true; + globalConfig.taskRunners.launcherPath = 'node'; + + await runnerProcess.start(); + await runnerProcess.stop(); + + expect(killLauncherSpy).toBeCalledTimes(1); + expect(killNodeSpy).toBeCalledTimes(0); + globalConfig.taskRunners.useLauncher = false; + }); }); From 4bd4b6977a7df6805d43546b39c7ad97aa3c0379 Mon Sep 17 00:00:00 2001 From: Michael Kret <88898367+michael-radency@users.noreply.github.com> Date: Mon, 14 Oct 2024 16:53:16 +0300 Subject: [PATCH 010/164] feat: Enable code capture in ai transform node (no-changelog) (#11241) --- .../editor-ui/src/components/JsEditor/JsEditor.vue | 7 ++++++- packages/editor-ui/src/components/ParameterInput.vue | 10 ++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/editor-ui/src/components/JsEditor/JsEditor.vue b/packages/editor-ui/src/components/JsEditor/JsEditor.vue index 2c89c83e08..906a8476db 100644 --- a/packages/editor-ui/src/components/JsEditor/JsEditor.vue +++ b/packages/editor-ui/src/components/JsEditor/JsEditor.vue @@ -30,6 +30,7 @@ type Props = { isReadOnly?: boolean; fillParent?: boolean; rows?: number; + posthogCapture?: boolean; }; const props = withDefaults(defineProps(), { fillParent: false, isReadOnly: false, rows: 4 }); @@ -74,6 +75,10 @@ const jsEditorRef = ref(); const editor = ref(null); const editorState = ref(null); +const generatedCodeCapture = computed(() => { + return props.posthogCapture ? '' : 'ph-no-capture '; +}); + const extensions = computed(() => { const extensionsToApply: Extension[] = [ javascript(), @@ -119,7 +124,7 @@ const extensions = computed(() => { diff --git a/packages/editor-ui/src/components/ParameterInput.vue b/packages/editor-ui/src/components/ParameterInput.vue index 313f62fe33..2b71b523b3 100644 --- a/packages/editor-ui/src/components/ParameterInput.vue +++ b/packages/editor-ui/src/components/ParameterInput.vue @@ -40,6 +40,7 @@ import { hasExpressionMapping, isValueExpression } from '@/utils/nodeTypesUtils' import { isResourceLocatorValue } from '@/utils/typeGuards'; import { + AI_TRANSFORM_NODE_TYPE, APP_MODALS_ELEMENT_ID, CORE_NODES_CATEGORY, CUSTOM_API_CALL_KEY, @@ -541,6 +542,13 @@ const showDragnDropTip = computed( ndvStore.isInputParentOfActiveNode, ); +const shouldCaptureForPosthog = computed(() => { + if (node.value?.type) { + return [AI_TRANSFORM_NODE_TYPE].includes(node.value?.type); + } + return false; +}); + function isRemoteParameterOption(option: INodePropertyOptions) { return remoteParameterOptionsKeys.value.includes(option.name); } @@ -1124,6 +1132,7 @@ onUpdated(async () => { :model-value="modelValueString" :is-read-only="isReadOnly" :rows="editorRows" + :posthog-capture="shouldCaptureForPosthog" fill-parent @update:model-value="valueChangedDebounced" /> @@ -1223,6 +1232,7 @@ onUpdated(async () => { :model-value="modelValueString" :is-read-only="isReadOnly || editorIsReadOnly" :rows="editorRows" + :posthog-capture="shouldCaptureForPosthog" @update:model-value="valueChangedDebounced" > + + From 76724c3be6e001792433045c2b2aac0ef16d4b8a Mon Sep 17 00:00:00 2001 From: Csaba Tuncsik Date: Mon, 21 Oct 2024 10:02:18 +0200 Subject: [PATCH 059/164] fix(editor): Open Community+ enrollment modal only for the instance owner (#11292) --- packages/@n8n/permissions/src/constants.ts | 1 + packages/@n8n/permissions/src/types.ts | 2 + packages/cli/src/permissions/global-roles.ts | 1 + .../src/components/PersonalizationModal.vue | 40 ++++++++++++------- packages/editor-ui/src/permissions.spec.ts | 2 + packages/editor-ui/src/stores/rbac.store.ts | 1 + .../src/views/SettingsUsageAndPlan.test.ts | 7 ++++ .../src/views/SettingsUsageAndPlan.vue | 9 ++++- packages/editor-ui/src/views/SetupView.vue | 2 +- 9 files changed, 48 insertions(+), 17 deletions(-) diff --git a/packages/@n8n/permissions/src/constants.ts b/packages/@n8n/permissions/src/constants.ts index c43677e843..7a0ebf2cb1 100644 --- a/packages/@n8n/permissions/src/constants.ts +++ b/packages/@n8n/permissions/src/constants.ts @@ -3,6 +3,7 @@ export const RESOURCES = { annotationTag: [...DEFAULT_OPERATIONS] as const, auditLogs: ['manage'] as const, banner: ['dismiss'] as const, + community: ['register'] as const, communityPackage: ['install', 'uninstall', 'update', 'list', 'manage'] as const, credential: ['share', 'move', ...DEFAULT_OPERATIONS] as const, externalSecretsProvider: ['sync', ...DEFAULT_OPERATIONS] as const, diff --git a/packages/@n8n/permissions/src/types.ts b/packages/@n8n/permissions/src/types.ts index 1a78f79f15..07ed750f91 100644 --- a/packages/@n8n/permissions/src/types.ts +++ b/packages/@n8n/permissions/src/types.ts @@ -13,6 +13,7 @@ export type WildcardScope = `${Resource}:*` | '*'; export type AnnotationTagScope = ResourceScope<'annotationTag'>; export type AuditLogsScope = ResourceScope<'auditLogs', 'manage'>; export type BannerScope = ResourceScope<'banner', 'dismiss'>; +export type CommunityScope = ResourceScope<'community', 'register'>; export type CommunityPackageScope = ResourceScope< 'communityPackage', 'install' | 'uninstall' | 'update' | 'list' | 'manage' @@ -48,6 +49,7 @@ export type Scope = | AnnotationTagScope | AuditLogsScope | BannerScope + | CommunityScope | CommunityPackageScope | CredentialScope | ExternalSecretProviderScope diff --git a/packages/cli/src/permissions/global-roles.ts b/packages/cli/src/permissions/global-roles.ts index 6315c3c617..7ea1b575da 100644 --- a/packages/cli/src/permissions/global-roles.ts +++ b/packages/cli/src/permissions/global-roles.ts @@ -15,6 +15,7 @@ export const GLOBAL_OWNER_SCOPES: Scope[] = [ 'credential:list', 'credential:share', 'credential:move', + 'community:register', 'communityPackage:install', 'communityPackage:uninstall', 'communityPackage:update', diff --git a/packages/editor-ui/src/components/PersonalizationModal.vue b/packages/editor-ui/src/components/PersonalizationModal.vue index ee7e1ff4d3..53e40f1cfb 100644 --- a/packages/editor-ui/src/components/PersonalizationModal.vue +++ b/packages/editor-ui/src/components/PersonalizationModal.vue @@ -93,6 +93,7 @@ import { useExternalHooks } from '@/composables/useExternalHooks'; import { useI18n } from '@/composables/useI18n'; import { useRoute, useRouter } from 'vue-router'; import { useUIStore } from '@/stores/ui.store'; +import { getResourcePermissions } from '@/permissions'; const SURVEY_VERSION = 'v4'; @@ -110,7 +111,9 @@ const uiStore = useUIStore(); const formValues = ref>({}); const isSaving = ref(false); - +const userPermissions = computed(() => + getResourcePermissions(usersStore.currentUser?.globalScopes), +); const survey = computed(() => [ { name: COMPANY_TYPE_KEY, @@ -548,23 +551,30 @@ const onSave = () => { formBus.emit('submit'); }; +const closeCallback = () => { + const isPartOfOnboardingExperiment = + posthogStore.getVariant(MORE_ONBOARDING_OPTIONS_EXPERIMENT.name) === + MORE_ONBOARDING_OPTIONS_EXPERIMENT.control; + // In case the redirect to homepage for new users didn't happen + // we try again after closing the modal + if (route.name !== VIEWS.HOMEPAGE && !isPartOfOnboardingExperiment) { + void router.replace({ name: VIEWS.HOMEPAGE }); + } +}; + const closeDialog = () => { modalBus.emit('close'); - uiStore.openModalWithData({ - name: COMMUNITY_PLUS_ENROLLMENT_MODAL, - data: { - closeCallback: () => { - const isPartOfOnboardingExperiment = - posthogStore.getVariant(MORE_ONBOARDING_OPTIONS_EXPERIMENT.name) === - MORE_ONBOARDING_OPTIONS_EXPERIMENT.control; - // In case the redirect to homepage for new users didn't happen - // we try again after closing the modal - if (route.name !== VIEWS.HOMEPAGE && !isPartOfOnboardingExperiment) { - void router.replace({ name: VIEWS.HOMEPAGE }); - } + + if (userPermissions.value.community.register) { + uiStore.openModalWithData({ + name: COMMUNITY_PLUS_ENROLLMENT_MODAL, + data: { + closeCallback, }, - }, - }); + }); + } else { + closeCallback(); + } }; const onSubmit = async (values: IPersonalizationLatestVersion) => { diff --git a/packages/editor-ui/src/permissions.spec.ts b/packages/editor-ui/src/permissions.spec.ts index e7946ce421..ab3952fbeb 100644 --- a/packages/editor-ui/src/permissions.spec.ts +++ b/packages/editor-ui/src/permissions.spec.ts @@ -8,6 +8,7 @@ describe('permissions', () => { annotationTag: {}, auditLogs: {}, banner: {}, + community: {}, communityPackage: {}, credential: {}, externalSecretsProvider: {}, @@ -62,6 +63,7 @@ describe('permissions', () => { annotationTag: {}, auditLogs: {}, banner: {}, + community: {}, communityPackage: {}, credential: { create: true, diff --git a/packages/editor-ui/src/stores/rbac.store.ts b/packages/editor-ui/src/stores/rbac.store.ts index d51bbc8538..a38b0f674b 100644 --- a/packages/editor-ui/src/stores/rbac.store.ts +++ b/packages/editor-ui/src/stores/rbac.store.ts @@ -27,6 +27,7 @@ export const useRBACStore = defineStore(STORES.RBAC, () => { eventBusDestination: {}, auditLogs: {}, banner: {}, + community: {}, communityPackage: {}, ldap: {}, license: {}, diff --git a/packages/editor-ui/src/views/SettingsUsageAndPlan.test.ts b/packages/editor-ui/src/views/SettingsUsageAndPlan.test.ts index bd88c02228..e33d2a080e 100644 --- a/packages/editor-ui/src/views/SettingsUsageAndPlan.test.ts +++ b/packages/editor-ui/src/views/SettingsUsageAndPlan.test.ts @@ -6,6 +6,8 @@ import { useUsageStore } from '@/stores/usage.store'; import SettingsUsageAndPlan from '@/views/SettingsUsageAndPlan.vue'; import { useUIStore } from '@/stores/ui.store'; import { COMMUNITY_PLUS_ENROLLMENT_MODAL } from '@/constants'; +import { useUsersStore } from '@/stores/users.store'; +import type { IUser } from '@/Interface'; vi.mock('vue-router', () => { return { @@ -23,6 +25,7 @@ vi.mock('vue-router', () => { let usageStore: ReturnType>; let uiStore: ReturnType>; +let usersStore: ReturnType>; const renderComponent = createComponentRenderer(SettingsUsageAndPlan); @@ -31,6 +34,7 @@ describe('SettingsUsageAndPlan', () => { createTestingPinia(); usageStore = mockedStore(useUsageStore); uiStore = mockedStore(useUIStore); + usersStore = mockedStore(useUsersStore); usageStore.viewPlansUrl = 'https://subscription.n8n.io'; usageStore.managePlanUrl = 'https://subscription.n8n.io'; @@ -49,6 +53,9 @@ describe('SettingsUsageAndPlan', () => { it('should not show badge but unlock notice', async () => { usageStore.isLoading = false; usageStore.planName = 'Community'; + usersStore.currentUser = { + globalScopes: ['community:register'], + } as IUser; const { getByRole, container } = renderComponent(); expect(getByRole('heading', { level: 3 })).toHaveTextContent('Community'); expect(container.querySelector('.n8n-badge')).toBeNull(); diff --git a/packages/editor-ui/src/views/SettingsUsageAndPlan.vue b/packages/editor-ui/src/views/SettingsUsageAndPlan.vue index b1df9d2fa9..82a6da8b39 100644 --- a/packages/editor-ui/src/views/SettingsUsageAndPlan.vue +++ b/packages/editor-ui/src/views/SettingsUsageAndPlan.vue @@ -11,11 +11,14 @@ import { useDocumentTitle } from '@/composables/useDocumentTitle'; import { hasPermission } from '@/utils/rbac/permissions'; import N8nInfoTip from 'n8n-design-system/components/N8nInfoTip'; import { COMMUNITY_PLUS_ENROLLMENT_MODAL } from '@/constants'; +import { useUsersStore } from '@/stores/users.store'; +import { getResourcePermissions } from '@/permissions'; const usageStore = useUsageStore(); const route = useRoute(); const router = useRouter(); const uiStore = useUIStore(); +const usersStore = useUsersStore(); const toast = useToast(); const documentTitle = useDocumentTitle(); @@ -48,6 +51,10 @@ const isCommunityEditionRegistered = computed( () => usageStore.planName.toLowerCase() === 'registered community', ); +const canUserRegisterCommunityPlus = computed( + () => getResourcePermissions(usersStore.currentUser?.globalScopes).community.register, +); + const showActivationSuccess = () => { toast.showMessage({ type: 'success', @@ -173,7 +180,7 @@ const openCommunityRegisterModal = () => { - +