feat(core): Unify application components shutdown #8097

tomi · 2023-12-19T15:07:11Z

Summary

Add ShutdownService and OnShutdown decorator for more unified way to shutdown different components. Use this new way in the following components:

HTTP(S) server
Pruning service
Push connection
License

Related tickets and issues

https://linear.app/n8n/issue/ADO-1617/unify-application-component-shutdown

Review / Merge checklist

PR title and summary are descriptive.
Tests included.

netroy

Thanks. This looks great ❤️

Ideas for next steps:

Move all shutdown logic out of commands, and into specific services
Convert @/Db into a service class, that handles closing of the DB connection itself
Move SIGINT and SIGTERM registration/handling into ShutdownService, and add integration tests to make sure that process.emit('SIGINT') causes all OnShutdown methods to be called in the correct order.
Move graceful shutdown timer and process.exit into ShutdownService
Start adding a OnStartup decorator to do initialization the same way, and significantly simplify app startup sequence.

cypress · 2023-12-22T09:18:43Z

1 flaky test on run #3468 ↗︎

0	316	5	0	1

Details:

🌳 🖥️ browsers:node18.12.0-chrome107 🤖 tomi 🗃️ e2e/*
Project: n8n	Commit: `fe47e193e2`
Status: Passed	Duration: 06:53 💡
Started: Dec 22, 2023 9:55 AM	Ended: Dec 22, 2023 10:02 AM

cypress/e2e/17-sharing.cy.ts • 1 flaky test

View Output Video

Test		Artifacts
Sharing > should work for admin role on credentials created by others (also can share it with themselves)		`Screenshots` `Video`

Review all test suite changes for PR #8097 ↗︎

github-actions · 2023-12-22T09:18:44Z

✅ All Cypress E2E specs passed

krynble

One relatively small thing that was on my to-do list for a while and might easily be addressed in this PR is adding the @OnShutdown annotation to the removeAllTriggerAndPollerBasedWorkflows method that is located in packages/cli/src/ActiveWorkflowRunner.ts.

Currently when n8n is shutting down it still continues to start new executions and start new jobs, even if it's theoretically stopping.

This new approach makes it trivial to fix this issue.

Thanks for the amazing work @tomi

tomi · 2023-12-22T09:33:29Z

One relatively small thing that was on my to-do list for a while and might easily be addressed in this PR is adding the @OnShutdown annotation to the removeAllTriggerAndPollerBasedWorkflows method that is located in packages/cli/src/ActiveWorkflowRunner.ts.

Currently when n8n is shutting down it still continues to start new executions and start new jobs, even if it's theoretically stopping.

This new approach makes it trivial to fix this issue.

Thanks for the amazing work @tomi

@krynble Thanks! I added the annotation to ActiveWorkflowRunner.removeAllTriggerAndPollerBasedWorkflows method. Like you mentioned, it was a trivial fix.

@netroy Could you re-approve 🙏

ivov

Loved this, amazing work!

Edit: Reviewed up to c3d1144

packages/cli/src/License.ts

packages/cli/src/decorators/OnShutdown.ts

packages/cli/src/shutdown/Shutdown.service.ts

github-actions · 2023-12-22T10:02:42Z

✅ All Cypress E2E specs passed

* master: feat(core): Unify application components shutdown (#8097) refactor(core): Upgrade more dependencies to remove axios 0.x (no-changelog) (#8105) refactor(core): Move active workflows endpoints to a decorated controller class (no-changelog) (#8101) fix(editor): Prevent canvas undo/redo when NDV is open (#8118) fix(editor): Fix operation change failing in certain conditions (#8114) fix(Asana Node): Omit body from GET, HEAD, and DELETE requests (#8057) fix(Redis Trigger Node): Activating a workflow with a Redis trigger fails (#8129) refactor(Peekalink Node): Stricter typing for Peekalink api call + Tests (no-changelog) (#8125) fix(core): Remove circular dependency in WorkflowService and ActiveWorkflowRunner (#8128) fix: Show public API upgrade CTA when feature is not enabled (#8109) feat(Snowflake Node): Update snowflake-sdk (no-changelog) (#8087) fix(core): Handle empty executions table in pruning in migrations (#8121) feat(core): Add closeFunction support to Sub-Nodes (#7708) refactor(core): Move error execution creation to execution service (no-changelog) (#8006) ci: Add lint rule `no-dynamic-import-template` (no-changelog) (#8089) feat(core): Remove discontinued crypto-js (#8104) 🚀 Release 1.22.0 (#8115) fix(core): Prevent axios from force setting a form-urlencoded content-type (#8117) feat(Respond to Webhook Node): Overhaul with improvements like returning all items (#8093)

* upstream/master: (32 commits) refactor(core): Move `typeorm` operators from `PruningService` to `ExecutionRepository` (no-changelog) (n8n-io#8145) refactor(core): Move more `typeorm` operators to repositories (no-changelog) (n8n-io#8143) refactor(core): Delete unused code, and fix typings in tests (no-changelog) (n8n-io#8142) docs: Improve documentation for the toTitleCase function (no-changelog) (n8n-io#8140) refactor(core): Move all base URLs to UrlService (no-changelog) (n8n-io#8141) feat(editor): Add template Id to workflow metadata (n8n-io#8088) fix(editor): Avoid sanitizing output to search node data (n8n-io#8126) fix(editor): Prevent browser zoom when scrolling inside sticky edit mode (n8n-io#8116) refactor(core): Move some `typeorm` operators to repositories (no-changelog) (n8n-io#8139) feat(editor): Add node execution status indicator to output panel (n8n-io#8124) feat(core): Unify application components shutdown (n8n-io#8097) refactor(core): Upgrade more dependencies to remove axios 0.x (no-changelog) (n8n-io#8105) refactor(core): Move active workflows endpoints to a decorated controller class (no-changelog) (n8n-io#8101) fix(editor): Prevent canvas undo/redo when NDV is open (n8n-io#8118) fix(editor): Fix operation change failing in certain conditions (n8n-io#8114) fix(Asana Node): Omit body from GET, HEAD, and DELETE requests (n8n-io#8057) fix(Redis Trigger Node): Activating a workflow with a Redis trigger fails (n8n-io#8129) refactor(Peekalink Node): Stricter typing for Peekalink api call + Tests (no-changelog) (n8n-io#8125) fix(core): Remove circular dependency in WorkflowService and ActiveWorkflowRunner (n8n-io#8128) fix: Show public API upgrade CTA when feature is not enabled (n8n-io#8109) ...

# [1.23.0](https://github.com/n8n-io/n8n/compare/n8n@1.22.0...n8n@1.23.0) (2024-01-03) ### Bug Fixes * **Asana Node:** Omit body from GET, HEAD, and DELETE requests ([#8057](#8057)) ([15ffd4f](15ffd4f)) * **core:** Better input validation for the changeRole endpoint ([#8189](#8189)) ([cfe9525](cfe9525)) * **core:** Fix issue that pinnedData is not used with Test-Webhooks ([#8123](#8123)) ([fa8bd8b](fa8bd8b)) * **core:** Handle empty executions table in pruning in migrations ([#8121](#8121)) ([ffaa30d](ffaa30d)) * **core:** Remove circular dependency in WorkflowService and ActiveWorkflowRunner ([#8128](#8128)) ([21788d9](21788d9)) * **core:** Use pinned data only for manual mode ([#8164](#8164)) ([ea7e76f](ea7e76f)) * **Discord Node:** Remove unnecessary requirement on parameters ([#8060](#8060)) ([ef3a577](ef3a577)) * **editor:** Avoid sanitizing output to search node data ([#8126](#8126)) ([c83d9f4](c83d9f4)) * **editor:** Enable explicit undo keyboard shortcut across all code editors ([#8178](#8178)) ([cf7f668](cf7f668)) * **editor:** Fix operation change failing in certain conditions ([#8114](#8114)) ([711fa2b](711fa2b)) * **editor:** Fix templates view layout ([#8196](#8196)) ([d01e42a](d01e42a)) * **editor:** Fix UI urls when hosted behind a path prefix ([#8198](#8198)) ([5c078f1](5c078f1)) * **editor:** Prevent browser zoom when scrolling inside sticky edit mode ([#8116](#8116)) ([e928210](e928210)) * **editor:** Prevent canvas undo/redo when NDV is open ([#8118](#8118)) ([39e45d8](39e45d8)) * **editor:** Prevent storing pairedItem data inside of pinData ([#8173](#8173)) ([405e267](405e267)) * **GitHub Node:** Fix issue that File->Get did not run once per item ([#8190](#8190)) ([11cda41](11cda41)) * **Invoice Ninja Node:** Fix issue with custom invoice numbers not working with v5 ([#8200](#8200)) ([3b6ae2d](3b6ae2d)) * **Microsoft Excel 365 Node:** Ensure arg is string during worksheet table search ([#8154](#8154)) ([8e873ca](8e873ca)) * **Notion Node:** Ensure arg is string during page ID extraction ([#8153](#8153)) ([e94b8a6](e94b8a6)) * **Redis Trigger Node:** Activating a workflow with a Redis trigger fails ([#8129](#8129)) ([a169b74](a169b74)) * **Schedule Trigger Node:** Use the correct `moment` import ([#8185](#8185)) ([17a4e2e](17a4e2e)) * Show public API upgrade CTA when feature is not enabled ([#8109](#8109)) ([e9c7fd7](e9c7fd7)) ### Features * **core:** Add closeFunction support to Sub-Nodes ([#7708](#7708)) ([bec0fae](bec0fae)) * **core:** Add user.profile.beforeUpdate hook ([#8144](#8144)) ([e126ed7](e126ed7)) * **core:** Improvements/overhaul for nodes working with binary data ([#7651](#7651)) ([5e16dd4](5e16dd4)) * **core:** Remove discontinued crypto-js ([#8104](#8104)) ([01e9a79](01e9a79)) * **core:** Unify application components shutdown ([#8097](#8097)) ([3a881be](3a881be)) * **editor:** Add node execution status indicator to output panel ([#8124](#8124)) ([ab74bad](ab74bad)) * **editor:** Add template Id to workflow metadata ([#8088](#8088)) ([517b050](517b050)) * **Home Assistant Node:** Use the new Home Assistant logo ([#8150](#8150)) ([518a99e](518a99e)) * **Qdrant Vector Store Node:** Qdrant vector store support ([#8080](#8080)) ([66460f6](66460f6)) * **Wordpress Node:** Add option to ignore error when using self signed certificates ([#8199](#8199)) ([65c8e12](65c8e12)) Co-authored-by: ivov <ivov@users.noreply.github.com>

Joffcom · 2024-01-03T13:07:55Z

Got released with n8n@1.23.0

tomi requested a review from netroy December 19, 2023 15:07

tomi changed the title ~~Ado 1617 unify application component shutdown~~ feature(core): Unify application component shutdown Dec 19, 2023

tomi changed the title ~~feature(core): Unify application component shutdown~~ feature(core): Unify application components shutdown Dec 19, 2023

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

tomi changed the title ~~feature(core): Unify application components shutdown~~ feat(core): Unify application components shutdown Dec 20, 2023

tomi added 8 commits December 20, 2023 10:42

feat(core): Add ShutdownService and ShutdownListener decorator

44a0918

fix(core): Gracefully shutdown http(s) server

47d8d0c

fix(core): shutdown push connections gracefully

329a5f4

fix(core): shutdown PruningService on signal

18198df

fix(core): unify license shutdown

68b4b8f

refactor(core): Add explicit name for shutdown listeners

6bd5cba

test(core): Add unit tests for ShutdownListener

40231d8

fix: lint errors

c31a958

tomi force-pushed the ado-1617-unify-application-component-shutdown branch from db9b069 to c31a958 Compare December 20, 2023 08:48

tomi and others added 5 commits December 20, 2023 11:09

test: fix test

b4ff6c7

WIP

bb211c9

add @service to all the server classes

7b92e90

Merge remote-tracking branch 'origin/master' into 8097-proposal

43ca89c

make it work

7bc8594

netroy force-pushed the ado-1617-unify-application-component-shutdown branch from d0dabfa to 7bc8594 Compare December 21, 2023 21:49

tomi added 2 commits December 22, 2023 10:44

fix shutdown tests

1048af4

don't wait for server connections to close on shutdown

c3d1144

netroy previously approved these changes Dec 22, 2023

View reviewed changes

krynble reviewed Dec 22, 2023

View reviewed changes

fix(core): Deactive triggers and poller based workflows on shutdown

3f2281c

tomi dismissed netroy’s stale review via 3f2281c December 22, 2023 09:30

krynble previously approved these changes Dec 22, 2023

View reviewed changes

netroy dismissed krynble’s stale review via 2da7e1d December 22, 2023 09:35

ivov reviewed Dec 22, 2023

View reviewed changes

netroy force-pushed the ado-1617-unify-application-component-shutdown branch from 2da7e1d to 3f2281c Compare December 22, 2023 09:40

netroy previously approved these changes Dec 22, 2023

View reviewed changes

review fixes

fe47e19

tomi dismissed netroy’s stale review via fe47e19 December 22, 2023 09:48

netroy approved these changes Dec 22, 2023

View reviewed changes

ivov changed the title ~~feat(core): Unify application components shutdown~~ feat(core): Unify application components shutdown (no-changelog) Dec 22, 2023

ivov changed the title ~~feat(core): Unify application components shutdown (no-changelog)~~ feat(core): Unify application components shutdown Dec 22, 2023

tomi merged commit 3a881be into master Dec 22, 2023
22 checks passed

tomi deleted the ado-1617-unify-application-component-shutdown branch December 22, 2023 10:39

This was referenced Dec 27, 2023

🚀 Release 1.22.1 #8157

Closed

🚀 Release 1.23.0 #8205

Merged

Joffcom added the Released label Jan 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(core): Unify application components shutdown #8097

feat(core): Unify application components shutdown #8097

tomi commented Dec 19, 2023 •

edited by netroy

Loading

netroy left a comment •

edited

Loading

cypress bot commented Dec 22, 2023 •

edited

Loading

github-actions bot commented Dec 22, 2023

krynble left a comment

tomi commented Dec 22, 2023

ivov left a comment •

edited

Loading

github-actions bot commented Dec 22, 2023

Joffcom commented Jan 3, 2024

feat(core): Unify application components shutdown #8097

feat(core): Unify application components shutdown #8097

Conversation

tomi commented Dec 19, 2023 • edited by netroy Loading

Summary

Related tickets and issues

Review / Merge checklist

netroy left a comment • edited Loading

Choose a reason for hiding this comment

cypress bot commented Dec 22, 2023 • edited Loading

1 flaky test on run #3468 ↗︎

Review all test suite changes for PR #8097 ↗︎

github-actions bot commented Dec 22, 2023

krynble left a comment

Choose a reason for hiding this comment

tomi commented Dec 22, 2023

ivov left a comment • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Dec 22, 2023

Joffcom commented Jan 3, 2024

tomi commented Dec 19, 2023 •

edited by netroy

Loading

netroy left a comment •

edited

Loading

cypress bot commented Dec 22, 2023 •

edited

Loading

ivov left a comment •

edited

Loading