Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

logic/17-split - join module not honoring the timeout delay #1957

Open
5 tasks done
Daryes opened this issue Nov 1, 2018 · 13 comments
Open
5 tasks done

logic/17-split - join module not honoring the timeout delay #1957

Daryes opened this issue Nov 1, 2018 · 13 comments
Labels

Comments

@Daryes
Copy link

Daryes commented Nov 1, 2018

What are the steps to reproduce?

I've set the join module right after the template module (mustache mode, output as text) to have in a single string in a message the result of multiple messages

The join module is set to :

  • manual mode
  • combine msg.payload
  • create a string
  • joined using : \n

What happens?

If I set After a number of message parts to a number, I'll get a single message as expected after the amount of message set was received, with the payload combining all those received message payload.
But, as the number of received messages can varies, using a timeout is more suited.

Problem is, if using only After a timeout following the first message, any number set will always result with all received message immediately transmitted, without any joining done.
Even if the number is assumed in ms (which seems to be in sec), setting a timeout of 10000 will still result to each received message transmitted in the next ms.

What do you expect to happen?

According to the timeout value set, a single message XX sec after the first received by join, containing the payload of multiple messages.

Please tell us about your environment:

  • Node-RED version: 0.19.4
  • node.js version: 8.12.0
  • npm version: 6.4.1
  • Platform/OS: Linux Ubuntu 18.04
  • Browser: Firefox 62
@dceejay
Copy link
Member

dceejay commented Dec 7, 2018

Sorry for long delay getting back to you.

A simple test flow like below seems to work just fine for me... nothing appears until after the timeout (in seconds)... unless I am missing something...

[{"id":"33576105.ba4cbe","type":"inject","z":"68704546.3d0e0c","name":"","topic":"","payload":"Hello ","payloadType":"str","repeat":"","crontab":"","once":false,"onceDelay":0.1,"hide":false,"x":185,"y":225,"wires":[["cd46b661.aadf68"]]},{"id":"cd46b661.aadf68","type":"join","z":"68704546.3d0e0c","name":"","mode":"custom","build":"string","property":"payload","propertyType":"msg","key":"topic","joiner":"\\n","joinerType":"str","accumulate":false,"timeout":"2","count":"","reduceRight":false,"reduceExp":"","reduceInit":"","reduceInitType":"","reduceFixup":"","x":395,"y":225,"wires":[["ceadbab8.f02838"]]},{"id":"ceadbab8.f02838","type":"debug","z":"68704546.3d0e0c","name":"","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"false","x":610,"y":225,"wires":[]}]

@Daryes
Copy link
Author

Daryes commented Dec 7, 2018

Your test flow helped me find the problem, linked to more complex messages:
I'm using multiple html nodes, to extract a subelement using a CSS selector. Given the huge html pages, even if I'm getting a single message from the html extract, it will always have the sub-object parts present in the message, and set like this :

parts: object
	id: "5fd8804a.58bac"
	index: 0
	count: 1
	type: "string"
	ch: ""

So, the problem is that given the documentation, the join node set in manual mode shouldn't check the parts object in the message, yet it still does it, like if it was in automatic mode.
When a new node was set into the workflow to remove the parts information, the join node worked as expected.
Dunno if this warrant a fix, and how. At least a note in the documentation, explaining the manual mode behavior is the same as in the automatic mode if a parts object is present.

I've put an extra node to simulate the parts object in your test flow, the result is working as described : the messages are processed by the join node (manual mode) while skipping the timeout setting (increased to 5 sec).

[{"id": "43d78536.bb4284","type": "inject","z": "bd330b54.58941","name": "","topic": "","payload": "hello","payloadType": "str","repeat": "","crontab": "","once": false,"onceDelay": 0.1,"x": 170,"y": 100,"wires": [["28044337.e3da64"]]},{"id": "7631b548.145afc","type": "join","z": "bd330b54.58941","name": "","mode": "custom","build": "string","property": "payload","propertyType": "msg","key": "topic","joiner": "\\n","joinerType": "str","accumulate": false,"timeout": "5","count": "","reduceRight": false,"reduceExp": "","reduceInit": "","reduceInitType": "","reduceFixup": "","x": 380,"y": 100,"wires": [["bd78fbc6.4a8f5"]]},{"id": "bd78fbc6.4a8f5","type": "debug","z": "bd330b54.58941","name": "","active": true,"tosidebar": true,"console": false,"tostatus": false,"complete": "false","x": 595,"y": 100,"wires": []},{"id": "28044337.e3da64","type": "change","z": "bd330b54.58941","name": "","rules": [{"t": "set","p": "parts","pt": "msg","to": "{\"id\":\"5fd8804a.58bac\",\"index\":0,\"count\":1,\"type\":\"string\",\"ch\":\"\"}","tot": "json"}],"action": "","property": "","from": "","to": "","reg": false,"x": 260,"y": 180,"wires": [["7631b548.145afc"]]}]

@AndreKR
Copy link

AndreKR commented Oct 26, 2023

I think I'm having the same issue, here is a very simple flow that reproduces the issue:

[{"id":"399d6730279fee66","type":"join","z":"b50fb95c24b7db8f","name":"","mode":"custom","build":"string","property":"payload","propertyType":"msg","key":"topic","joiner":"\\n","joinerType":"str","accumulate":false,"timeout":"5","count":"","reduceRight":false,"reduceExp":"","reduceInit":"","reduceInitType":"","reduceFixup":"","x":2290,"y":820,"wires":[["5ebb448adb929ef5"]]},{"id":"5ebb448adb929ef5","type":"debug","z":"b50fb95c24b7db8f","name":"debug 27","active":true,"tosidebar":true,"console":false,"tostatus":false,"complete":"true","targetType":"full","statusVal":"","statusType":"auto","x":2420,"y":820,"wires":[]},{"id":"23dba08dd0e12627","type":"function","z":"b50fb95c24b7db8f","name":"function 4","func":"return {\n    \"payload\": \"foo\",\n    \"parts\": {\n        \"count\": 1\n    },\n};","outputs":1,"timeout":0,"noerr":0,"initialize":"","finalize":"","libs":[],"x":2160,"y":820,"wires":[["399d6730279fee66"]]},{"id":"d726508b8cda167b","type":"inject","z":"b50fb95c24b7db8f","name":"","props":[],"repeat":"0.5","crontab":"","once":false,"onceDelay":0.1,"topic":"","x":2030,"y":820,"wires":[["23dba08dd0e12627"]]}]

image

Every 500 ms a message with payload foo is created, for the join node to join after a buffer period of 5 seconds.

Expected behavior would be one message every 5 seconds with payload foo↵foo↵foo↵foo↵foo↵foo↵foo↵foo↵foo↵foo.

Actual behavior is one message every 500 ms with payload foo.

The expected behavior can be achieved by removing the snippet "parts": { "count": 1 }, from the function node's code.

@Just-another-pleb
Copy link

Sorry, but WHY ON EARTH would have have a parts field in the message being sent into the join node anyway?
To me that is forcing the join node to think that each message received is the first/only message.

As much as a good find it may be....

@colinl
Copy link
Contributor

colinl commented Oct 26, 2023

@AndreKR can you add which versions of node-red and nodejs are you using please. If this is still present in the latest node-red then it is indeed a bug. The node should not take any notice of msg.parts when in manual mode.

@Just-another-pleb
Copy link

Greatest apologies.

NR 3.0.2
NODE version 16.20.2

@colinl
Copy link
Contributor

colinl commented Oct 26, 2023

It would be good to test it with the latest node red

@Just-another-pleb
Copy link

I was only mentioning that it would seem strange to inject repeated messages with the parts field set with the same value.

@colinl
Copy link
Contributor

colinl commented Oct 26, 2023

Obviously one would not intentionally do that, but the message might have a parts property from a previous node. The flow is a test flow only.

@AndreKR
Copy link

AndreKR commented Oct 26, 2023

@Just-another-pleb In this test flow it is there to demonstrate the issue obviously. In the real flow it's because it comes from a split node. I have a device that sends a combined status update as a JSON array and I split it up and only process some parts of it.

@colinl I'm using the Docker image nodered/node-red:3.1-debian which contains Node-RED 3.1.0 and Node.js v16.20.2.

@colinl
Copy link
Contributor

colinl commented Oct 26, 2023

I can also confirm seeing the problem with the test flow and node red 3.1.0, with nodejs 20.5.1.
Removing the parts property removes the problem.

@colinl
Copy link
Contributor

colinl commented Oct 26, 2023

@dceejay would you consider removing the needs info tag. I don't seem to be able to do that. I think it is pretty clear that this is a reproducible bug.

@dceejay dceejay added bug and removed needs-info labels Oct 26, 2023
@dceejay
Copy link
Member

dceejay commented Oct 26, 2023

The obvious fix is for us to remove the parts property if we are in manual mode. But need to be careful a) that we aren't using it for something even if we shouldn't be. IE that deleting doesn't break existing and b) do need to preserve msg.parts.parts. IE we do stack recurring splits and mustn't break that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants