fix(core): Add check that queue is defined and remove cyclic dependency #7404

flipswitchingmonkey · 2023-10-11T12:24:08Z

In a rare edge case an undefined queue could be returned - this should not happen and now an error is thrown.
Also using the opportunity to remove a cyclic dependency from the Queue.

github-actions · 2023-10-11T12:24:24Z

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.

netroy · 2023-10-11T12:37:35Z

packages/cli/src/Queue.ts

-	async add(jobData: JobData, jobOptions: object): Promise<Job> {
-		return this.jobQueue.add(jobData, jobOptions);
-	}
-
-	async getJob(jobId: JobId): Promise<Job | null> {
-		return this.jobQueue.getJob(jobId);
-	}


instead of removing these, can we update the code elsewhere to use these methods instead of using the jobQueue directly?

do these methods offer any value? they don't do any additional checks or operations but just tunnel the method call through the service

I'm working on adding metrics for queue for auto-scaling, and in that branch, it was helpful to have all bull specific code in a single class, instead of being spread across multiple files.

codecov · 2023-10-11T12:46:08Z

Codecov Report

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

Comparison is base (2b075bf) 33.50% compared to head (33caafd) 33.50%.
Report is 12 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7404      +/-   ##
==========================================
- Coverage   33.50%   33.50%   -0.01%     
==========================================
  Files        3389     3390       +1     
  Lines      207020   207027       +7     
  Branches    22337    22339       +2     
==========================================
  Hits        69361    69361              
- Misses     136538   136545       +7     
  Partials     1121     1121

Files	Coverage Δ
packages/cli/src/WebhookHelpers.ts	`32.68% <ø> (+0.11%)`	⬆️
packages/cli/src/WorkflowRunner.ts	`9.25% <33.33%> (+0.03%)`	⬆️
packages/cli/src/helpers/decodeWebhookResponse.ts	`40.00% <40.00%> (ø)`
packages/cli/src/commands/worker.ts	`26.85% <0.00%> (+0.12%)`	⬆️
packages/cli/src/Queue.ts	`13.63% <11.11%> (-3.04%)`	⬇️

... and 3 files with indirect coverage changes

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

cypress · 2023-10-13T09:46:26Z

1 flaky test on run #2489 ↗︎

0	251	0	0	1

Details:

🌳 🖥️ browsers:node18.12.0-chrome107 🤖 flipswitchingmonkey 🗃️ e2e/*
Project: n8n	Commit: `33caafde6a`
Status: Passed	Duration: 08:50 💡
Started: Oct 13, 2023 9:37 AM	Ended: Oct 13, 2023 9:46 AM

cypress/e2e/24-ndv-paired-item.cy.ts • 1 flaky test

View Output Video

Test		Artifacts
NDV > resolves expression with default item when input node is not parent, while still pairing items		`Output` `Screenshots` `Video`

Review all test suite changes for PR #7404 ↗︎

github-actions · 2023-10-13T09:46:30Z

✅ All Cypress E2E specs passed

# [1.12.0](https://github.com/n8n-io/n8n/compare/n8n@1.11.0...n8n@1.12.0) (2023-10-18) ### Bug Fixes * **core:** Add check that queue is defined and remove cyclic dependency ([#7404](#7404)) ([45f2ef3](45f2ef3)) * **core:** Do not throw when deleting workflows with executions without binary-data ([#7411](#7411)) ([2b6a15e](2b6a15e)) * **core:** Fix expression with paired item with multi-input node ([#7424](#7424)) ([ec14141](ec14141)) * **core:** Fix ignoring crashed executions without event msgs ([#7368](#7368)) ([2f4d91b](2f4d91b)) * **core:** Pg-promise de-initialization fix ([#7417](#7417)) ([7703904](7703904)) * **core:** Prevent false stalled jobs in queue mode from displaying as errored ([#7435](#7435)) ([e01b9e5](e01b9e5)) * **core:** Prevent undefined issues when restoring binary data ([#7419](#7419)) ([46977a2](46977a2)) * **editor:** Fix remote options fetching on every keystroke ([#7320](#7320)) ([367255a](367255a)) * **editor:** Open only one tab with plans page ([#7377](#7377)) ([c599006](c599006)) * **Google Sheets Node:** Update by row_number, restored 'Handling Extra Data Option', updated Cell Format default ([#7357](#7357)) ([d8531a5](d8531a5)) * **Ldap Node:** Fix issue with connections not closing correctly ([#7432](#7432)) ([c3f0be8](c3f0be8)) * **Set Node:** Null should not throw an error ([#7416](#7416)) ([e9b6ab0](e9b6ab0)) * **TheHive 5 Node:** Observable encoding in alert > create fix ([#7450](#7450)) ([a2d2e3d](a2d2e3d)) ### Features * **core:** Make executions pruning interval configurable ([#7439](#7439)) ([40707fa](40707fa)) * **Google Calendar Trigger Node:** Add support for cancelled events ([#7436](#7436)) ([9d241a0](9d241a0)) * **HubSpot Trigger Node:** Add support for ticket related events ([#7156](#7156)) ([57c6093](57c6093)) * **n8n Form Trigger Node:** New node ([#7130](#7130)) ([3ddc176](3ddc176)) * **Spreadsheet File Node:** Improve CSV parsing ([#7448](#7448)) ([79f23fb](79f23fb)) Co-authored-by: netroy <netroy@users.noreply.github.com>

janober · 2023-10-18T16:26:38Z

Got released with n8n@1.12.0

…cy (#7404) In a rare edge case an undefined queue could be returned - this should not happen and now an error is thrown. Also using the opportunity to remove a cyclic dependency from the Queue.

# [1.12.0](https://github.com/n8n-io/n8n/compare/n8n@1.11.0...n8n@1.12.0) (2023-10-18) ### Bug Fixes * **core:** Add check that queue is defined and remove cyclic dependency ([#7404](#7404)) ([45f2ef3](45f2ef3)) * **core:** Do not throw when deleting workflows with executions without binary-data ([#7411](#7411)) ([2b6a15e](2b6a15e)) * **core:** Fix expression with paired item with multi-input node ([#7424](#7424)) ([ec14141](ec14141)) * **core:** Fix ignoring crashed executions without event msgs ([#7368](#7368)) ([2f4d91b](2f4d91b)) * **core:** Pg-promise de-initialization fix ([#7417](#7417)) ([7703904](7703904)) * **core:** Prevent false stalled jobs in queue mode from displaying as errored ([#7435](#7435)) ([e01b9e5](e01b9e5)) * **core:** Prevent undefined issues when restoring binary data ([#7419](#7419)) ([46977a2](46977a2)) * **editor:** Fix remote options fetching on every keystroke ([#7320](#7320)) ([367255a](367255a)) * **editor:** Open only one tab with plans page ([#7377](#7377)) ([c599006](c599006)) * **Google Sheets Node:** Update by row_number, restored 'Handling Extra Data Option', updated Cell Format default ([#7357](#7357)) ([d8531a5](d8531a5)) * **Ldap Node:** Fix issue with connections not closing correctly ([#7432](#7432)) ([c3f0be8](c3f0be8)) * **Set Node:** Null should not throw an error ([#7416](#7416)) ([e9b6ab0](e9b6ab0)) * **TheHive 5 Node:** Observable encoding in alert > create fix ([#7450](#7450)) ([a2d2e3d](a2d2e3d)) ### Features * **core:** Make executions pruning interval configurable ([#7439](#7439)) ([40707fa](40707fa)) * **Google Calendar Trigger Node:** Add support for cancelled events ([#7436](#7436)) ([9d241a0](9d241a0)) * **HubSpot Trigger Node:** Add support for ticket related events ([#7156](#7156)) ([57c6093](57c6093)) * **n8n Form Trigger Node:** New node ([#7130](#7130)) ([3ddc176](3ddc176)) * **Spreadsheet File Node:** Improve CSV parsing ([#7448](#7448)) ([79f23fb](79f23fb)) Co-authored-by: netroy <netroy@users.noreply.github.com>

check queue defined and remove cyclic dependcy

3ad41bd

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

netroy reviewed Oct 11, 2023

View reviewed changes

wrap queue methods

33caafd

flipswitchingmonkey requested a review from netroy October 11, 2023 15:11

netroy changed the title ~~fix(core): Add check that queue is defined and remove cyclic dependcy~~ fix(core): Add check that queue is defined and remove cyclic dependency Oct 11, 2023

netroy approved these changes Oct 13, 2023

View reviewed changes

flipswitchingmonkey merged commit 45f2ef3 into master Oct 13, 2023
55 of 56 checks passed

flipswitchingmonkey deleted the pay-744-queue-mode-execution-failing branch October 13, 2023 09:54

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

🚀 Release 1.12.0 #7459

Merged

janober added the Released label Oct 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(core): Add check that queue is defined and remove cyclic dependency #7404

fix(core): Add check that queue is defined and remove cyclic dependency #7404

flipswitchingmonkey commented Oct 11, 2023

github-actions bot commented Oct 11, 2023

netroy Oct 11, 2023

flipswitchingmonkey Oct 11, 2023

netroy Oct 11, 2023

codecov bot commented Oct 11, 2023 •

edited

Loading

cypress bot commented Oct 13, 2023

github-actions bot commented Oct 13, 2023

janober commented Oct 18, 2023

fix(core): Add check that queue is defined and remove cyclic dependency #7404

fix(core): Add check that queue is defined and remove cyclic dependency #7404

Conversation

flipswitchingmonkey commented Oct 11, 2023

github-actions bot commented Oct 11, 2023

netroy Oct 11, 2023

Choose a reason for hiding this comment

flipswitchingmonkey Oct 11, 2023

Choose a reason for hiding this comment

netroy Oct 11, 2023

Choose a reason for hiding this comment

codecov bot commented Oct 11, 2023 • edited Loading

Codecov Report

cypress bot commented Oct 13, 2023

1 flaky test on run #2489 ↗︎

Review all test suite changes for PR #7404 ↗︎

github-actions bot commented Oct 13, 2023

janober commented Oct 18, 2023

codecov bot commented Oct 11, 2023 •

edited

Loading