windkh / node-red-contrib-telegrambot

Telegram bot nodes for node-red.
Other
266 stars 119 forks source link

Sending message using MarkdownV2 with full-stop does not send msg #153

Closed TotallyInformation closed 3 years ago

TotallyInformation commented 3 years ago

Using parse mode of Markdown works but MarkdownV2 does not.

This does not work with v2, it does with v1:

{
   "chatId": "0000000000",
   "content": "__*Simple String Example*__\nMessage . Body",
   "type":"message",
   "options":{"parse_mode":"MarkdownV2"}
}

Taking out the . in the content works in either.

telegrambot v8.9.6 Node-RED v1.2.9

windkh commented 3 years ago

Telegram issues the following error: image

I did not know that until now, .... how should this be handled in future?

Right now the node falls back to plain mode when the error contains "can't parse entities in message text:"

Any suggestions?

TotallyInformation commented 3 years ago

Hi, some more testing. If you include a dot inside a markdown link as in [BBC](bbc.co.uk) - that works. It is only if it is not inside a link that it fails.

Could you also try { as well, I think that also fails.

Not sure why I didn't check the Node-RED log! You get this:

Unhandled rejection TypeError: nodeSend is not a function
    at TelegramOutNode.processError (/home/home/nrmain/data/node_modules/node-red-contrib-telegrambot/telegrambot/99-telegrambot.js:1265:17)
    at /home/home/nrmain/data/node_modules/node-red-contrib-telegrambot/telegrambot/99-telegrambot.js:1355:50
    at tryCatcher (/home/home/nrmain/data/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/home/home/nrmain/data/node_modules/bluebird/js/release/promise.js:547:31)
    at Promise._settlePromise (/home/home/nrmain/data/node_modules/bluebird/js/release/promise.js:604:18)
    at Promise._settlePromise0 (/home/home/nrmain/data/node_modules/bluebird/js/release/promise.js:649:10)
    at Promise._settlePromises (/home/home/nrmain/data/node_modules/bluebird/js/release/promise.js:725:18)
    at _drainQueueStep (/home/home/nrmain/data/node_modules/bluebird/js/release/async.js:93:12)
    at _drainQueue (/home/home/nrmain/data/node_modules/bluebird/js/release/async.js:86:9)
    at Async._drainQueues (/home/home/nrmain/data/node_modules/bluebird/js/release/async.js:102:5)
    at Immediate.Async.drainQueues [as _onImmediate] (/home/home/nrmain/data/node_modules/bluebird/js/release/async.js:15:14)
    at processImmediate (internal/timers.js:461:21)

I think that the error should be trapped and returned back to Node-RED and output as a node.error() so that the flow author can deal with it. Indeed, any of those kinds of errors should appear in the debug output not just the log so that they can be dealt with.

Might also be good to put a comment into the help somewhere.

Thanks for the quick response.

And thanks for a great node! :-) I've used it for some years now as my main alerting and external control interface with my home automation Node-RED instance. I wouldn't want to be without it.


Incidentally, I discovered this while writing a wrapper function. I use an MQTT topic that I can send to that automatically outputs to Telegram. The topic is converted to a "title" of sorts (bold on a line of its own or bold/underlined for MarkdownV2), the payload is used as the rest of the content. However, I also allow <br> to be imbedded in the payload and this is converted to a new line. Makes it nice and easy to output to Telegram from anywhere in a flow or indeed direct from a microprocessor system if it supports MQTT.

windkh commented 3 years ago

Hi Julian, thank for your input. proper error handling is always an issue... I added a check-box for enabling a second error output so that the user can handle errors in the flow (btw by looking at your problem I found a bug in the processError function).

I am not sure what to do in this case

  1. fallback to plain text if markdown problems are discovered
  2. issue an error

...

TotallyInformation commented 3 years ago

After some thought I think that option 2 is the right approach. Then users can handle that in their flows if they need to.

windkh commented 3 years ago

Here are my findings after some testing:

Adding a catch node will show the exception in the debug window: image

windkh commented 3 years ago

Adding a second output to the sender node will reveal the error, too:

image

windkh commented 3 years ago

Without catch not second output in sender node you will get the following:

image

windkh commented 3 years ago

@TotallyInformation I tested with version 8.10.0. So I am not sure what to change in the error handling. Maybe you try updating to the latest version? I will close this for now. Please open this again if you have the feeling that I should have a second look at it.

TotallyInformation commented 3 years ago

Many thanks for this. I will test as soon as I can.

windkh commented 3 years ago

There is still one todo inside the code. There is a fallback implemented for Mardown when large messages are split into smaller chunks. MarkdownV2 messages that are longer than 4096 characters will have probably still a problem