fix(core): Ensure execution deletion in worker lifecycle hook #7481

ivov · 2023-10-20T14:58:40Z

Reported by customer here, apparently a very old long-standing bug for queue mode. Please review closely as I am not familiar with queue mode.

github-actions · 2023-10-20T14:58:56Z

Great PR! Please pay attention to the following items before merging:

Files matching packages/**:

If fixing bug, added test to cover scenario.
If addressing forum or Github issue, added link to description.

Files matching packages/**/*.ts:

Added unit tests to cover new or updated functionality.

Make sure to check off this list before asking for review.

ivov · 2023-10-20T14:59:01Z

packages/cli/src/WorkflowExecuteAdditionalData.ts

+					// Check config to know if execution should be saved or not
+					let saveDataErrorExecution = config.getEnv('executions.saveDataOnError') as string;
+					let saveDataSuccessExecution = config.getEnv('executions.saveDataOnSuccess') as string;
+					if (this.workflowData.settings !== undefined) {
+						saveDataErrorExecution =
+							(this.workflowData.settings.saveDataErrorExecution as string) ||
+							saveDataErrorExecution;
+						saveDataSuccessExecution =
+							(this.workflowData.settings.saveDataSuccessExecution as string) ||
+							saveDataSuccessExecution;
+					}
+


Duplication will be removed at #7370

codecov · 2023-10-20T16:48:55Z

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (0b42d1a) 33.55% compared to head (c02f3f7) 33.60%.
Report is 25 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7481      +/-   ##
==========================================
+ Coverage   33.55%   33.60%   +0.05%     
==========================================
  Files        3398     3399       +1     
  Lines      207577   207475     -102     
  Branches    22421    22416       -5     
==========================================
+ Hits        69649    69727      +78     
+ Misses     136806   136615     -191     
- Partials     1122     1133      +11

Files	Coverage Δ
packages/cli/src/WorkflowExecuteAdditionalData.ts	`12.57% <0.00%> (-0.37%)`	⬇️

... and 64 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

krynble · 2023-10-24T07:52:49Z

packages/cli/src/WorkflowExecuteAdditionalData.ts

+							await Container.get(ExecutionRepository).hardDelete({
+								workflowId: this.workflowData.id as string,
+								executionId: this.executionId,
+							});


Unfortunately this deletion cannot happen here.

Due to the nature of queue mode, we save the execution data to DB and notify via Redis that a given execution has finished.

This in turn allows the caller (be it main or webhook) to load the execution from DB (if needed) and react (such as sending an HTTP response or removing an item from a queue).

This code would need to be part of the array returned by getWorkflowHooksWorkerMain instead.

ivov · 2023-10-24T09:55:12Z

packages/cli/src/WorkflowExecuteAdditionalData.ts

+			let saveDataErrorExecution = config.getEnv('executions.saveDataOnError') as string;
+			let saveDataSuccessExecution = config.getEnv('executions.saveDataOnSuccess') as string;
+			if (this.workflowData.settings !== undefined) {
+				saveDataErrorExecution =
+					(this.workflowData.settings.saveDataErrorExecution as string) || saveDataErrorExecution;
+				saveDataSuccessExecution =
+					(this.workflowData.settings.saveDataSuccessExecution as string) ||
+					saveDataSuccessExecution;
+			}


Duplication will be removed at #7370

cypress · 2023-10-25T15:55:40Z

1 flaky test on run #2577 ↗︎

0	257	0	0	1

Details:

🌳 🖥️ browsers:node18.12.0-chrome107 🤖 ivov 🗃️ e2e/*
Project: n8n	Commit: `c02f3f720b`
Status: Passed	Duration: 09:31 💡
Started: Oct 25, 2023 3:46 PM	Ended: Oct 25, 2023 3:55 PM

cypress/e2e/5-ndv.cy.ts • 1 flaky test

View Output Video

Test		Artifacts
NDV > can link and unlink run selectors between input and output		`Output` `Screenshots` `Video`

Review all test suite changes for PR #7481 ↗︎

github-actions · 2023-10-25T15:55:40Z

✅ All Cypress E2E specs passed

Reported by customer [here](https://n8nio.slack.com/archives/C05PUALKZHD/p1697446945481249?thread_ts=1697196557.638169&cid=C05PUALKZHD), apparently a very old long-standing bug for queue mode. Please review closely as I am not familiar with queue mode.

## [1.14.1](https://github.com/n8n-io/n8n/compare/n8n@1.14.0...n8n@1.14.1) (2023-10-26) ### Bug Fixes * **core:** Ensure execution deletion in worker lifecycle hook ([#7481](#7481)) ([b81e5b9](b81e5b9)) * **core:** Upgrade crypto-js to address CVE-2023-46233 ([#7519](#7519)) ([0c3ee47](0c3ee47)) * **editor:** Fixes the issue that Switch Node can not be created ([#7516](#7516)) ([b64146b](b64146b)) Co-authored-by: netroy <netroy@users.noreply.github.com>

janober · 2023-10-26T12:28:24Z

Got released with n8n@1.14.2

# [1.15.0](https://github.com/n8n-io/n8n/compare/n8n@1.14.0...n8n@1.15.0) (2023-11-02) ### Bug Fixes * **core:** Ensure execution deletion in worker lifecycle hook ([#7481](#7481)) ([742c8a8](742c8a8)) * **core:** Fix data encryption on credentials import ([#7560](#7560)) ([b350568](b350568)) * **core:** Fix issue that prevents owner logging in when using ldap ([#7408](#7408)) ([479f902](479f902)) * **core:** Handle missing resultData in runData ([#7523](#7523)) ([1055bd3](1055bd3)) * **core:** Permission check for subworkflow properly checking for workflow settings ([#7576](#7576)) ([437c95e](437c95e)) * **core:** Prevent executions from becoming forever running ([#7569](#7569)) ([9bdb85c](9bdb85c)) * **core:** Upgrade crypto-js to address CVE-2023-46233 ([#7519](#7519)) ([65e5593](65e5593)) * **editor:** Do not truncate form inputs ([#7528](#7528)) ([ae616f1](ae616f1)) * **editor:** Fix NDV close after using input select ([#7544](#7544)) ([3b5e181](3b5e181)) * **editor:** Fix NDV unexpected re-render ([#7532](#7532)) ([2853fcf](2853fcf)) * **editor:** Fix route component caching, incorrect use of array reduce method and enable WF history feature ([#7434](#7434)) ([12a89e6](12a89e6)) * **editor:** Fixes the issue that Switch Node can not be created ([#7516](#7516)) ([df89685](df89685)) * **editor:** Handle `localStorage` being blocked/unavailable ([#7348](#7348)) ([c05bc67](c05bc67)) * **Jira Software Node:** Handle missing issue types in issue types loader ([#7534](#7534)) ([9762705](9762705)) * **Switch Node:** Allow sortable Switch rules ([#7555](#7555)) ([7a56e58](7a56e58)) ### Features * **core:** Add optional Error-Output ([#7460](#7460)) ([655efea](655efea)) * **core:** Make queue mode settings configurable ([#7526](#7526)) ([3d95b24](3d95b24)) * **core:** Set up leader selection for multiple main instances ([#7527](#7527)) ([442c73e](442c73e)) * **editor:** Implement the `UserStack` design system component ([#7559](#7559)) ([ce14f62](ce14f62)) * **HTTP Request Node:** Add pagination support ([#5993](#5993)) ([cc2bd2e](cc2bd2e)) * **HTTP Request Node:** Update icon and default color ([#7572](#7572)) ([ff279ab](ff279ab)) * **n8n Form Trigger Node:** Add text area and password input types ([#7474](#7474)) ([b72040a](b72040a)) * **editor:** Dark mode is here! You can change it under personal settings.([#6980](#6980)) ([0746783](0746783)) --------- Co-authored-by: krynble <krynble@users.noreply.github.com> Co-authored-by: Omar Ajoue <krynble@gmail.com>

## [1.15.1](https://github.com/n8n-io/n8n/compare/n8n@1.14.0...n8n@1.15.1) (2023-11-02) ### Bug Fixes * **core:** Ensure execution deletion in worker lifecycle hook ([#7481](#7481)) ([742c8a8](742c8a8)) * **core:** Fix data encryption on credentials import ([#7560](#7560)) ([b350568](b350568)) * **core:** Fix issue that prevents owner logging in when using ldap ([#7408](#7408)) ([479f902](479f902)) * **core:** Handle missing resultData in runData ([#7523](#7523)) ([1055bd3](1055bd3)) * **core:** Permission check for subworkflow properly checking for workflow settings ([#7576](#7576)) ([437c95e](437c95e)) * **core:** Prevent executions from becoming forever running ([#7569](#7569)) ([9bdb85c](9bdb85c)) * **core:** Upgrade crypto-js to address CVE-2023-46233 ([#7519](#7519)) ([65e5593](65e5593)) * **editor:** Do not truncate form inputs ([#7528](#7528)) ([ae616f1](ae616f1)) * **editor:** Fix NDV close after using input select ([#7544](#7544)) ([3b5e181](3b5e181)) * **editor:** Fix NDV unexpected re-render ([#7532](#7532)) ([2853fcf](2853fcf)) * **editor:** Fix route component caching, incorrect use of array reduce method and enable WF history feature ([#7434](#7434)) ([12a89e6](12a89e6)) * **editor:** Fixes the issue that Switch Node can not be created ([#7516](#7516)) ([df89685](df89685)) * **editor:** Handle `localStorage` being blocked/unavailable ([#7348](#7348)) ([c05bc67](c05bc67)) * Fix dark mode small issues ([#7573](#7573)) ([1d81afc](1d81afc)) * **Jira Software Node:** Handle missing issue types in issue types loader ([#7534](#7534)) ([9762705](9762705)) * **Switch Node:** Allow sortable Switch rules ([#7555](#7555)) ([7a56e58](7a56e58)) ### Features * **core:** Add optional Error-Output ([#7460](#7460)) ([655efea](655efea)) * **core:** Make queue mode settings configurable ([#7526](#7526)) ([3d95b24](3d95b24)) * **core:** Set up leader selection for multiple main instances ([#7527](#7527)) ([442c73e](442c73e)) * **editor:** Implement the `UserStack` design system component ([#7559](#7559)) ([ce14f62](ce14f62)) * **HTTP Request Node:** Add pagination support ([#5993](#5993)) ([cc2bd2e](cc2bd2e)) * **HTTP Request Node:** Update icon and default color ([#7572](#7572)) ([ff279ab](ff279ab)) * **n8n Form Trigger Node:** Add text area and password input types ([#7474](#7474)) ([b72040a](b72040a)) * **editor:** Dark mode is here! You can change it under personal settings.([#6980](#6980)) ([0746783](0746783)) --------- Co-authored-by: krynble <krynble@users.noreply.github.com> Co-authored-by: Omar Ajoue <krynble@gmail.com>

fix(core): Ensure execution deletion in worker lifecycle hook

1e351f2

ivov commented Oct 20, 2023

View reviewed changes

n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Oct 20, 2023

krynble suggested changes Oct 24, 2023

View reviewed changes

ivov added 2 commits October 24, 2023 11:53

Address feedback

2d1a5eb

Remove unneeded settings

c02f3f7

ivov commented Oct 24, 2023

View reviewed changes

krynble approved these changes Oct 25, 2023

View reviewed changes

ivov merged commit 742c8a8 into master Oct 25, 2023
57 of 58 checks passed

ivov deleted the ensure-execution-deletion-in-worker-lifecycle-hook branch October 25, 2023 17:13

github-actions bot mentioned this pull request Oct 26, 2023

🚀 Release 1.14.1 #7522

Merged

janober added the Released label Oct 26, 2023

github-actions bot mentioned this pull request Nov 2, 2023

🚀 Release 1.15.0 #7580

Merged

Joffcom mentioned this pull request Nov 2, 2023

No longer shows executions #7373

Closed

github-actions bot mentioned this pull request Nov 2, 2023

🚀 Release 1.15.1 #7592

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(core): Ensure execution deletion in worker lifecycle hook #7481

fix(core): Ensure execution deletion in worker lifecycle hook #7481

ivov commented Oct 20, 2023

github-actions bot commented Oct 20, 2023

ivov Oct 20, 2023

codecov bot commented Oct 20, 2023 •

edited

Loading

krynble Oct 24, 2023

ivov Oct 24, 2023

cypress bot commented Oct 25, 2023

github-actions bot commented Oct 25, 2023

janober commented Oct 26, 2023

fix(core): Ensure execution deletion in worker lifecycle hook #7481

fix(core): Ensure execution deletion in worker lifecycle hook #7481

Conversation

ivov commented Oct 20, 2023

github-actions bot commented Oct 20, 2023

ivov Oct 20, 2023

Choose a reason for hiding this comment

codecov bot commented Oct 20, 2023 • edited Loading

Codecov Report

krynble Oct 24, 2023

Choose a reason for hiding this comment

ivov Oct 24, 2023

Choose a reason for hiding this comment

cypress bot commented Oct 25, 2023

1 flaky test on run #2577 ↗︎

Review all test suite changes for PR #7481 ↗︎

github-actions bot commented Oct 25, 2023

janober commented Oct 26, 2023

codecov bot commented Oct 20, 2023 •

edited

Loading