guidone/node-red-contrib-chatbot

Total Crash caused by Params node

Iciir opened this issue · 6 comments

Iciir commented

I updated to the latest RedBot version and now this happens:

node-red_1  | 9 Feb 08:09:56 - [red] Uncaught Exception:
node-red_1  | 9 Feb 08:09:56 - [error] TypeError: Cannot read properties of undefined (reading 'params')
node-red_1  |     at /data/node_modules/node-red-contrib-chatbot/nodes/chatbot-params.js:42:36
node-red_1  | 9 Feb 08:10:05 - [info] 
node-red_1  | 
node-red_1  | Welcome to Node-RED
node-red_1  | ===================
node-red_1  | 
node-red_1  | 9 Feb 08:10:05 - [info] Node-RED version: v3.0.2
node-red_1  | 9 Feb 08:10:05 - [info] Node.js  version: v18.7.0
node-red_1  | 9 Feb 08:10:05 - [info] Linux 5.19.12-zen1-1-zen x64 LE

Which completely restarts Node-RED.

Regardless of how my flow is designed, I don't think it's supposed to completely crash Node-RED.

I don't know how to reproduce it and my flows are enormous, but it seems to be related to the "Delete Message" parameter, on Telegram and using {{outboundMessageId}} from the drop down (might be misspelled, I just wrote it from memory right now).

If it's not an easy fix, I'll spend some time trying to reliably reproduce it, please let me know if necessary.

Iciir commented

Here's a flow to reproduce it:

[
    {
        "id": "cb71e84b9eed7d69",
        "type": "inject",
        "z": "fbebfbce6edfcfa3",
        "name": "",
        "props": [
            {
                "p": "intent",
                "v": "normal",
                "vt": "str"
            },
            {
                "p": "response",
                "v": "Saving AI Model, stand by...",
                "vt": "str"
            },
            {
                "p": "delBotMessage",
                "v": "true",
                "vt": "bool"
            }
        ],
        "repeat": "3600",
        "crontab": "",
        "once": true,
        "onceDelay": 0.1,
        "topic": "",
        "x": 155,
        "y": 500,
        "wires": [
            [
                "bd8d0ab292a5c016"
            ]
        ],
        "l": false
    },
    {
        "id": "772990ef1257dea0",
        "type": "link out",
        "z": "fbebfbce6edfcfa3",
        "name": "link out 3",
        "mode": "link",
        "links": [
            "d3e8b63dbe4ff393"
        ],
        "x": 355,
        "y": 500,
        "wires": [],
        "icon": "font-awesome/fa-save"
    },
    {
        "id": "d3e8b63dbe4ff393",
        "type": "link in",
        "z": "fbebfbce6edfcfa3",
        "name": "link in 3",
        "links": [
            "772990ef1257dea0"
        ],
        "x": 195,
        "y": 320,
        "wires": [
            [
                "77496d2f3b33d879"
            ]
        ],
        "icon": "font-awesome/fa-save"
    },
    {
        "id": "77496d2f3b33d879",
        "type": "chatbot-conversation",
        "z": "fbebfbce6edfcfa3",
        "name": "",
        "botDevelopment": "3eac5c32152546a7",
        "botProduction": "3eac5c32152546a7",
        "chatId": "-1001865771488",
        "userId": "",
        "transport": "telegram",
        "x": 315,
        "y": 320,
        "wires": [
            [
                "5da96cb5ea7c3caa"
            ]
        ],
        "l": false
    },
    {
        "id": "5da96cb5ea7c3caa",
        "type": "chatbot-message",
        "z": "fbebfbce6edfcfa3",
        "name": "",
        "message": [
            {
                "message": "{{response}}"
            }
        ],
        "language": "none",
        "x": 355,
        "y": 320,
        "wires": [
            [
                "b3516935f549f719"
            ]
        ],
        "l": false
    },
    {
        "id": "b3516935f549f719",
        "type": "chatbot-params",
        "z": "fbebfbce6edfcfa3",
        "name": "",
        "params": [
            {
                "platform": "telegram",
                "name": "parseMode",
                "value": "HTML"
            }
        ],
        "outputs": 1,
        "x": 395,
        "y": 320,
        "wires": [
            [
                "4a36d3209837f769"
            ]
        ],
        "l": false
    },
    {
        "id": "4a36d3209837f769",
        "type": "chatbot-telegram-send",
        "z": "fbebfbce6edfcfa3",
        "bot": "3eac5c32152546a7",
        "botProduction": "3eac5c32152546a7",
        "track": false,
        "passThrough": true,
        "errorOutput": false,
        "outputs": 1,
        "x": 435,
        "y": 320,
        "wires": [
            [
                "81b4e43714d2f8ae"
            ]
        ],
        "l": false
    },
    {
        "id": "81b4e43714d2f8ae",
        "type": "switch",
        "z": "fbebfbce6edfcfa3",
        "name": "",
        "property": "delBotMessage",
        "propertyType": "msg",
        "rules": [
            {
                "t": "true"
            }
        ],
        "checkall": "true",
        "repair": false,
        "outputs": 1,
        "x": 475,
        "y": 320,
        "wires": [
            [
                "d24f2ddc227724ed"
            ]
        ],
        "l": false
    },
    {
        "id": "d24f2ddc227724ed",
        "type": "chatbot-params",
        "z": "fbebfbce6edfcfa3",
        "name": "",
        "params": [
            {
                "platform": "telegram",
                "name": "deleteMessageId",
                "value": "{{outboundMessageId}}"
            }
        ],
        "outputs": 1,
        "x": 535,
        "y": 320,
        "wires": [
            [
                "df767573efcad7a7"
            ]
        ],
        "l": false
    },
    {
        "id": "df767573efcad7a7",
        "type": "chatbot-telegram-send",
        "z": "fbebfbce6edfcfa3",
        "bot": "3eac5c32152546a7",
        "botProduction": "3eac5c32152546a7",
        "track": false,
        "passThrough": false,
        "errorOutput": false,
        "outputs": 0,
        "x": 575,
        "y": 320,
        "wires": [],
        "l": false
    },
    {
        "id": "3eac5c32152546a7",
        "type": "chatbot-telegram-node",
        "botname": "Nova Neon",
        "usernames": "",
        "polling": "1000",
        "store": "c3b2120bfd5d64aa",
        "log": "",
        "debug": false,
        "skipMediaFiles": false,
        "webHook": "",
        "connectMode": "polling"
    },
    {
        "id": "c3b2120bfd5d64aa",
        "type": "chatbot-context-store",
        "name": "Neon Collection",
        "contextStorage": "memory",
        "contextParams": ""
    }
]

The Inject node must be connected to the Link Out thing.

Iciir commented

msg.payload seems to be lost after passing through [Telegram Out] Node.

Fixed it by restoring msg.payload with:

[
    {
        "id": "b75401e9022511e3",
        "type": "change",
        "z": "fbebfbce6edfcfa3",
        "name": "",
        "rules": [
            {
                "t": "set",
                "p": "payload",
                "pt": "msg",
                "to": "sentMessage",
                "tot": "msg"
            }
        ],
        "action": "",
        "property": "",
        "from": "",
        "to": "",
        "reg": false,
        "x": 575,
        "y": 320,
        "wires": [
            [
                "d24f2ddc227724ed"
            ]
        ],
        "icon": "font-awesome/fa-refresh",
        "l": false
    }
]

So the bug is that msg.payload is not present after passing through [Telegram Out].

Thanks, I'll check

TLDR; I'll put a guard in chatbot-params.js:42 to avoid the crash, send out nodes clearing the msg.payload is intended.

Explanation:

Telegram Out resets correctly the msg.payload, it's in some way "consumed" by it, having a different behavior would break a lot of flows: all nodes in RedBot (Message, Document, Photo, etc) pile up messages from upstream nodes, in this way it's possible - for example - to chain a message, then an image and then another message and send everything to just one send out node (which takes the three chained payloads and sends them in the correct order).

Not clearing the output of the Telegram out node would result in the next send out node to send these messages again, since the payload was not cleared, causing duplicates.

With the recent change the outcome of the Params node was moved from msg.params to msg.payload.params, which I think it's correct, params are "consumed" by the output nodes the same way as the payloads for text / photo messages.

To fix it will be enough to put a guard in this part of the code.

fixed in 1.2.2

Iciir commented

Not clearing the output of the Telegram out node would result in the next send out node to send these messages again, since the payload was not cleared, causing duplicates.

Yes, I am using RedBot for a long time (you know), and everything is designed with a Template Node to clear up the Payload after it passes through [Telegram Out]. 😂

I was adapted to the old ways.

Would it not also (for some compatibility thing maybe - because I'll have to find all the errors and fix them now), be possible to... In case [Params] or another RedBot node, checks for things inside payload, and the payload does not exist, to copy msg.sentMessage to a new msg.payload?

Some kind of...

for (let _attempt = 0; _attempt < 1; _attempt++) {  
   try {
      <params code>
      break
      } catch(e) { msg.payload = msg.sentMessage }
   }

(I'm just trying to express the idea, I'm not very aware how it's built)