## Summary
Fixes the issue that pinned data gets also used for production executions.
---------
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
## Summary
When a workflow gets started via a Test-Webhook the pinned data does get
ignored and the nodes executed anyway.
## Related tickets and issues
> Include links to **Linear ticket** or Github issue or Community forum
post. Important in order to close *automatically* and provide context to
reviewers.
## Review / Merge checklist
- [x] PR title and summary are descriptive. **Remember, the title
automatically goes into the changelog. Use `(no-changelog)` otherwise.**
([conventions](https://github.com/n8n-io/n8n/blob/master/.github/pull_request_title_conventions.md))
- [ ] [Docs updated](https://github.com/n8n-io/n8n-docs) or follow-up
ticket created.
- [X] Tests included.
> A bug is not considered fixed, unless a test is added to prevent it
from happening again.
> A feature is not complete without tests.
---------
Co-authored-by: Mutasem Aldmour <mutasem@n8n.io>
This change kept coming up in #6713, #7773, and #8135.
So this PR moves the existing code without actually changing anything,
to help get rid of some of the circular dependencies.
## Review / Merge checklist
- [x] PR title and summary are descriptive.
## Summary
A circular dependency between `WorkflowService` and
`ActiveWorkflowRunner` is sometimes causing `this.activeWorkflowRunner`
to be `undefined` in `WorkflowService`.
Breaking this circular dependency should hopefully fix this issue.
## Related tickets and issues
#8122
## Review / Merge checklist
- [x] PR title and summary are descriptive
- [ ] Tests included
Refactor static workflow service classes into DI-compatible classes
Context: https://n8nio.slack.com/archives/C069HS026UF/p1702466571648889
Up next:
- Inject dependencies into workflow services
- Consolidate workflow controllers into one
- Make workflow controller injectable
- Inject dependencies into workflow controller
---------
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
Github issue / Community forum post (link here to close automatically):
---------
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
Co-authored-by: Giulio Andreini <andreini@netseven.it>
Ensure all errors in `cli` inherit from `ApplicationError` to continue
normalizing all the errors we report to Sentry
Follow-up to: https://github.com/n8n-io/n8n/pull/7820
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.
Story: [PAY-846](https://linear.app/n8n/issue/PAY-846) | Related:
https://github.com/n8n-io/n8n/pull/7225
For the S3 backend for external storage of binary data and execution
data, the `getAsStream` method in the binary data manager interface used
by FS and S3 will need to become async. This is a breaking change for
nodes-base.
Depends on: #7092 | Story:
[PAY-768](https://linear.app/n8n/issue/PAY-768)
This PR:
- Generalizes the `IBinaryDataManager` interface.
- Adjusts `Filesystem.ts` to satisfy the interface.
- Sets up an S3 client stub to be filled in in the next PR.
- Turns `BinaryDataManager` into an injectable service.
- Adjusts the config schema and adds new validators.
Note that the PR looks large but all the main changes are in
`packages/core/src/binaryData`.
Out of scope:
- `BinaryDataManager` (now `BinaryDataService`) and `Filesystem.ts` (now
`fs.client.ts`) were slightly refactored for maintainability, but fully
overhauling them is **not** the focus of this PR, which is meant to
clear the way for the S3 implementation. Future improvements for these
two should include setting up a backwards-compatible dir structure that
makes it easier to locate binary data files to delete, removing
duplication, simplifying cloning methods, using integers for binary data
size instead of `prettyBytes()`, writing tests for existing binary data
logic, etc.
---------
Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
# Motivation
In Queue mode, finished executions would cause the main instance to
always pull all execution data from the database, unflatten it and then
use it to send out event log events and telemetry events, as well as
required returns to Respond to Webhook nodes etc.
This could cause OOM errors when the data was large, since it had to be
fully unpacked and transformed on the main instance’s side, using up a
lot of memory (and time).
This PR attempts to limit this behaviour to only happen in those
required cases where the data has to be forwarded to some waiting
webhook, for example.
# Changes
Execution data is only required in cases, where the active execution has
a `postExecutePromise` attached to it. These usually forward the data to
some other endpoint (e.g. a listening webhook connection).
By adding a helper `getPostExecutePromiseCount()`, we can decide that in
cases where there is nothing listening at all, there is no reason to
pull the data on the main instance.
Previously, there would always be postExecutePromises because the
telemetry events were called. Now, these have been moved into the
workers, which have been given the various InternalHooks calls to their
hook function arrays, so they themselves issue these telemetry and event
calls.
This results in all event log messages to now be logged on the worker’s
event log, as well as the worker’s eventbus being the one to send out
the events to destinations. The main event log does…pretty much nothing.
We are not logging executions on the main event log any more, because
this would require all events to be replicated 1:1 from the workers to
the main instance(s) (this IS possible and implemented, see the worker’s
`replicateToRedisEventLogFunction` - but it is not enabled to reduce the
amount of traffic over redis).
Partial events in the main log could confuse the recovery process and
would result in, ironically, the recovery corrupting the execution data
by considering them crashed.
# Refactor
I have also used the opportunity to reduce duplicate code and move some
of the hook functionality into
`packages/cli/src/executionLifecycleHooks/shared/sharedHookFunctions.ts`
in preparation for a future full refactor of the hooks
Changes in https://github.com/n8n-io/n8n/pull/6394 removed xml body parsing for all non-webhook routes. This broken SAML endpoints as they need the XML body parser to function correctly.
Also,
1. Consistent CORS support ~on all three webhook types~ waiting webhooks never supported CORS. I'll fix that in another PR
2. [Fixes binary-data handling when request body is text, json, or xml](https://linear.app/n8n/issue/NODE-505/webhook-binary-data-handling-fails-for-textplain-files).
3. Reduced number of middleware that each request has to go through.
4. Removed the need to maintain webhook endpoints in the auth-exception list.
5. Skip all middlewares (apart from `compression`) on Webhook routes.
6. move `multipart/form-data` support out of individual nodes
7. upgrade `formidable`
8. fix the filenames on binary-data in webhooks nodes
9. add unit tests and integration tests for webhook request handling, and increase test coverage
* refactor: Set up ownership service
* refactor: Specify cache keys and values
* refactor: Replace util with service calls
* test: Mock service in tests
* refactor: Use dependency injection
* test: Write tests
* refactor: Apply feedback from Omar and Micha
* test: Fix tests
* test: Fix missing spot
* refactor: Return user entity from cache
* refactor: More dependency injection!
* add typedi
* convert ActiveWorkflowRunner into an injectable service
* convert ExternalHooks into an injectable service
* convert InternalHooks into an injectable service
* convert LoadNodesAndCredentials into an injectable service
* convert NodeTypes and CredentialTypes into an injectable service
* convert ActiveExecutions into an injectable service
* convert WaitTracker into an injectable service
* convert Push into an injectable service
* convert ActiveWebhooks and TestWebhooks into an injectable services
* handle circular references, and log errors when a circular dependency is found
