Skip to content

Conversation

@valya
Copy link
Contributor

@valya valya commented Apr 6, 2023

Throws errors in manual mode and ignores and logs values in production

Github issue / Community forum post (link here to close automatically):

Throws errors in manual mode and ignores and logs values in production
@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2023

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.

@codecov
Copy link

codecov bot commented Apr 6, 2023

Codecov Report

Patch coverage: 64.70% and project coverage change: +0.01 🎉

Comparison is base (c236d5d) 17.53% compared to head (9bd0a03) 17.54%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5920      +/-   ##
==========================================
+ Coverage   17.53%   17.54%   +0.01%     
==========================================
  Files        2500     2500              
  Lines      114336   114365      +29     
  Branches    17849    17859      +10     
==========================================
+ Hits        20050    20069      +19     
- Misses      93694    93704      +10     
  Partials      592      592              
Impacted Files Coverage Δ
packages/core/src/NodeExecuteFunctions.ts 10.56% <0.00%> (-0.10%) ⬇️
packages/core/src/WorkflowExecutionMetadata.ts 85.71% <91.66%> (+7.14%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Apr 6, 2023
@valya valya requested a review from krynble April 6, 2023 12:56
@valya valya requested a review from krynble April 10, 2023 08:46
@github-actions
Copy link
Contributor

✅ All Cypress E2E specs passed

}
executionData.resultData.metadata[String(key).slice(0, 50)] = String(value).slice(0, 255);
if (typeof key !== 'string') {
throw new ExecutionMetadataValidationError('key', key);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should also accept numbers. It's become very annoying, if I have a number that is generated I simply can't save it as is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use this workflow as an example to see the error.

{
  "meta": {
    "instanceId": "27cc9b56542ad45b38725555722c50a1c3fee1670bbb67980558314ee08517c4"
  },
  "nodes": [
    {
      "parameters": {
        "path": "a59cd802-3f21-4672-a208-c5bbb45711f6",
        "responseMode": "lastNode",
        "options": {}
      },
      "id": "5d434c5e-7794-46da-acee-1cee883c2d6e",
      "name": "Webhook",
      "type": "n8n-nodes-base.webhook",
      "typeVersion": 1,
      "position": [
        780,
        540
      ],
      "webhookId": "a59cd802-3f21-4672-a208-c5bbb45711f6"
    },
    {
      "parameters": {
        "jsCode": "// Loop over input items and add a new field\n// called 'myNewField' to the JSON of each one\n\nconst generatedNumber = Math.floor(Math.random() * 5);\n\nconst large = Math.floor(Math.random() * 1000);\n\n\n$execution.customData.set(`potato${generatedNumber}`, generatedNumber);\n$execution.customData.set(`name${generatedNumber}`, generatedNumber);\n\n\n$execution.customData.set(`large${large}`, '0');\n\nfor (const item of $input.all()) {\n  item.json.generatedNumber = generatedNumber;\n}\n\nreturn $input.all();"
      },
      "id": "3943cd3f-db65-468c-993a-4543fe340e6f",
      "name": "Code",
      "type": "n8n-nodes-base.code",
      "typeVersion": 1,
      "position": [
        980,
        540
      ]
    },
    {
      "parameters": {
        "keepOnlySet": true,
        "values": {
          "string": [
            {
              "name": "coolStuff",
              "value": "={{ $execution.customData.getAll() }}"
            }
          ]
        },
        "options": {}
      },
      "id": "dee79bb5-b0a8-4916-aa88-2a87f576f579",
      "name": "Set",
      "type": "n8n-nodes-base.set",
      "typeVersion": 1,
      "position": [
        1420,
        540
      ]
    },
    {
      "parameters": {
        "amount": 2,
        "unit": "seconds"
      },
      "id": "f085cf4f-b71b-46fe-96a6-4c201e8db2ab",
      "name": "Wait",
      "type": "n8n-nodes-base.wait",
      "typeVersion": 1,
      "position": [
        1200,
        540
      ],
      "webhookId": "c084fefc-6bcc-484d-b0c8-c16188d602f5",
      "disabled": true
    }
  ],
  "connections": {
    "Webhook": {
      "main": [
        [
          {
            "node": "Code",
            "type": "main",
            "index": 0
          }
        ]
      ]
    },
    "Code": {
      "main": [
        [
          {
            "node": "Wait",
            "type": "main",
            "index": 0
          }
        ]
      ]
    },
    "Wait": {
      "main": [
        [
          {
            "node": "Set",
            "type": "main",
            "index": 0
          }
        ]
      ]
    }
  }
}

Copy link
Contributor

@krynble krynble left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@github-actions
Copy link
Contributor

✅ All Cypress E2E specs passed

@valya valya merged commit 323e26a into master Apr 12, 2023
@valya valya deleted the pay-337-advanced-execution-filters-bugs branch April 12, 2023 08:18
MiloradFilipovic added a commit that referenced this pull request Apr 13, 2023
* master: (62 commits)
  fix(editor): Redirect to home page after saving data on SAML onboarding page (no-changelog) (#5961)
  feat: Replace Vue.extend with defineComponent in design system (no-changelog) (#5918)
  feat(MySQL Node): Overhaul
  fix(OpenAI Node): Update models to only show those supported (#5805)
  ci: Add test for wait node (no-changelog) (#5414)
  fix(Github Trigger Node): Remove content_reference event (#5830)
  ci: Validate load options methods in nodes-base (no-changelog) (#5862)
  ci: Use `--chown=node:node` in COPY commands in the custom docker image (no-changelog) (#5913)
  🚀 Release 0.224.0 (#5957)
  fix(NocoDB Node): Fix for updating or deleting rows with not default primary keys
  fix(HTTP Request Node): Show detailed error message in the UI again (#5959)
  ci: Prevent skipping of E2E fail job (no-changelog) (#5958)
  ci: Fix E2E tests on master (no-changelog) (#5960)
  refactor(core): Use injectable classes for db repositories (part-1) (no-changelog) (#5953)
  fix(core): Validate customData keys and values (#5920) (no-changelog)
  feat(editor): Add user activation survey (#5677)
  fix(editor): Update vite legacy-plugin browser target (no-changelog) (#5952)
  docs: Fix typo in AWS S3 and S3 nodes for parent folder key (#5933)
  fix(core): Update xml2js to address CVE-2023-0842 (#5948)
  fix(Code Node): Update vm2 to address CVE-2023-29017 (#5947)
  ...

# Conflicts:
#	packages/workflow/src/Interfaces.ts
MiloradFilipovic added a commit that referenced this pull request Apr 13, 2023
…rce-mapper-ui

* feature/resource-mapping-component: (62 commits)
  fix(editor): Redirect to home page after saving data on SAML onboarding page (no-changelog) (#5961)
  feat: Replace Vue.extend with defineComponent in design system (no-changelog) (#5918)
  feat(MySQL Node): Overhaul
  fix(OpenAI Node): Update models to only show those supported (#5805)
  ci: Add test for wait node (no-changelog) (#5414)
  fix(Github Trigger Node): Remove content_reference event (#5830)
  ci: Validate load options methods in nodes-base (no-changelog) (#5862)
  ci: Use `--chown=node:node` in COPY commands in the custom docker image (no-changelog) (#5913)
  🚀 Release 0.224.0 (#5957)
  fix(NocoDB Node): Fix for updating or deleting rows with not default primary keys
  fix(HTTP Request Node): Show detailed error message in the UI again (#5959)
  ci: Prevent skipping of E2E fail job (no-changelog) (#5958)
  ci: Fix E2E tests on master (no-changelog) (#5960)
  refactor(core): Use injectable classes for db repositories (part-1) (no-changelog) (#5953)
  fix(core): Validate customData keys and values (#5920) (no-changelog)
  feat(editor): Add user activation survey (#5677)
  fix(editor): Update vite legacy-plugin browser target (no-changelog) (#5952)
  docs: Fix typo in AWS S3 and S3 nodes for parent folder key (#5933)
  fix(core): Update xml2js to address CVE-2023-0842 (#5948)
  fix(Code Node): Update vm2 to address CVE-2023-29017 (#5947)
  ...

# Conflicts:
#	packages/workflow/src/Interfaces.ts
@janober
Copy link
Member

janober commented Apr 19, 2023

Got released with [email protected]

sunilrr pushed a commit to fl-g6/qp-n8n that referenced this pull request Apr 24, 2023
…gelog)

* fix(core): Validate customData keys and values

Throws errors in manual mode and ignores and logs values in production

* fix: validate customData key characters

* refactor: review changes

* fix: logger not initialised for metadata tests

* fix: allow numbers for values
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team Released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants