search

travisghansen / node-red-slack

A node-red module to post to Slack.com
MIT License
22 stars 29 forks source link

Slack Bot In node doesn't handle connection problem #15

Closed janvda closed 5 years ago

janvda commented 6 years ago

Hi,

First and foremost my appreciation for these excellent slack nodes.

I am using these nodes for quite some time and the thing I noticed is that after some time (even some days) my [Slack Bot In] node no longer receives any slack messages (in the slack - I also see under "Apps" that it is no longer connected to the "Slack Bot").

I think that the connection between the [Slack Bot In] node and slack is broken and that this connection is not automatically restored. So I need to restart or redeploy my node red application to get it working again.

Note also that no error message is logged in the node red log files when this connection drops and there is also no status indication when consulting the [Slack Bot In] node in the node red editor.

kr Jan

shanness commented 6 years ago

I'm having the same problem..

baze- commented 5 years ago

Same problem here. Also bots online status changes to "offline" in Slack. This seems to indicate some kind of connection problem.

shanness commented 5 years ago

I've coded up what I think will work around this for now. If it works, I'll put in a background task to do it automatically. For now it's a new node that you use like so :-

Inject timer (every 2 mins say) -> Slack Bot Keep Alive node

It outputs either true or false depending on if the connection was still alive, and if it's not, it reconnects it.

yayadrian commented 5 years ago

Thanks for the workaround. I'll take a look this week and might redesign the whole thing to use webhooks VS the current web sockets

janvda commented 5 years ago

@shanness I have tried the work around with the Slack Bot Keep Alive node but it is not working. Have you checked that this is working ?

E.g. this is what I see in my node-RED log file:

[Sat Dec 15 2018 17:43:18 GMT+0100 (CET)] ERROR Last pong is too old: 14.7
[Sat Dec 15 2018 17:43:18 GMT+0100 (CET)] INFO Reconnecting in 1000ms
[Sat Dec 15 2018 17:43:19 GMT+0100 (CET)] INFO Attempting reconnect
[Sat Dec 15 2018 17:43:19 GMT+0100 (CET)] INFO Connecting...
15 Dec 17:43:20 - [info] [Slack Bot In:stemija slackbot] in: Logged in:
15 Dec 17:44:00 - [info] [Slack Bot Keep Alive:stemija keep alive] KEEP ALIVE : Reconencting to Slack.
15 Dec 17:44:00 - [info] [Slack Bot Keep Alive:stemija keep alive] Slack disconnecting...
15 Dec 17:44:00 - [info] [Slack Bot Keep Alive:stemija keep alive] Slack already connected
15 Dec 17:45:00 - [info] [Slack Bot Keep Alive:stemija keep alive] Keep alive called before other bot nodes initialised
15 Dec 17:46:00 - [info] [Slack Bot Keep Alive:stemija keep alive] Keep alive called before other bot nodes initialised
15 Dec 17:47:00 - [info] [Slack Bot Keep Alive:stemija keep alive] Keep alive called before other bot nodes initialised
15 Dec 17:48:00 - [info] [Slack Bot Keep Alive:stemija keep alive] Keep alive called before other bot nodes initialised
15 Dec 17:49:00 - [info] [Slack Bot Keep Alive:stemija keep alive] Keep alive called before other bot nodes initialised
15 Dec 17:50:00 - [info] [Slack Bot Keep Alive:stemija keep alive] Keep alive called before other bot nodes initialised

One problem is that the slackReconnect() function always says that Slack already connected while this is most likely because the slackLogOut() function is deleting the ojbect slackBotGlobal[token] which is checked in line 29 of the slackLogin() function.

Moreover I am also doubting that fixing the slackReconnect() would make that after successful reconnect that the slack on message handler (line 82) is still registered.

shanness commented 5 years ago

Hey @janvda. I just got my first "Reconnecting" today actually (been doing lots of node-red dev, so hasn't triggered till today). I didn't write the slackReconnect, just call it as it was called in the out node. But agreed, it looks busted, seems to dump all the listeners and not reconnect them. I suspect it would just require making slackLogOut() return a copy of the listeners it removed, and re-setting them after slackLogIn() in the slackReconnect().

    function slackReconnect(token, node) {
        var handlers = slackLogOut(token, node);
        slackLogin(token, node);
        for ( int i = 0 ; i < handlers.length; i++ ) {
           slackBotGlobal[token].on(handler.event,handler.funtionRef)
       }
   }

Or actually, on second thoughts, perhaps just call slackLogin() instead of reconnect (like the error handler does), that would hopefully just re-login.. I'll do that now. I've removed the slackReconnect function (only had two calls, and one was mine, both now just call slackLogin()). Just leaving above in case it's needed.

Fingers crossed this will fix it. Hard to know until it triggers again.. e7e922e4d657ff86970e9a7a86c9cf2798474e33

shanness commented 5 years ago

Got it fixed @janvda. See https://github.com/shanness/node-red-slack/commit/e8557203a2cd9987c2f2c72c3c4696f48141737d

Tested and working (after resetting slackBotState[token].connecting = false before the slackLogin call

[Slack Bot Keep Alive:b516ad8b.50dc8] KEEP ALIVE : Reconencting to Slack. (false,true)
[Slack Bot Keep Alive:b516ad8b.50dc8] Slack not connected
INFO Connecting...
...1 second gap..
[Slack Bot In:cmd bot in] in: Logged in:

Figured out how to force disconnects (details in commit message). 

sudo netstat -pn | grep node
find the "tcp        0      0 10.0.0.7:48670          13.54.39.212:443 ESTABLISHED 29167/node-red  "
line (key is the :443), and run "sudo tcpkill port 48670" hitting Ctrl-C after a couple of
seconds.  This will force a reconnect, which is now working :)

So now sends me one connection lost message to slack (using the webhook node), and the $help shows it reconnected

cmd APP [4:35 AM] cmdbot lost connection, reconnecting : false !
craig [4:39 AM] $help
cmd APP [4:39 AM] Valid commands :-
$bounce [live|beta]

This implied it's could by run by a setInterval() function (with perhaps 30 second loops) without the need for the KeepAlive node :) I'll get onto that next, then this should be solid.

shanness commented 5 years ago

OK, this is all fixed. No longer needs the separate KeepAlive node, just does a 30 second interval timer per token. I'll leave it running for a while before squashing my commits into one and doing a PR. For anyone that wants to assist with testing, pull this and replace the html and js files in ~/.node-red/node_modules/node-red-contrib-slack with the ones from here. https://github.com/shanness/node-red-slack

janvda commented 5 years ago

@shanness I have integrated your changes. No connection drops so far. Thanks a lot.

janvda commented 5 years ago

@shanness : There is an issue with the changes (https://github.com/shanness/node-red-slack/commit/2ec2ba41aec7e0258968dee2dd88d0ffe3cd7512). It seems to reconnect but now every slack message is received multiple times by the Slack Bot In node. So today I am receiving every slack message 5 times !

FYI here some logs showing the reconnections:

20 Dec 00:05:56 - [info] [Slack Bot In:stemija slackbot] KEEP ALIVE : Reconencting to Slack. (false,true)
20 Dec 00:05:56 - [info] [Slack Bot In:stemija slackbot] Slack logging in ...
[Thu Dec 20 2018 00:05:56 GMT+0100 (CET)] INFO Connecting...
20 Dec 00:05:57 - [info] [Slack Bot In:stemija slackbot] in: Logged in:
20 Dec 00:41:57 - [info] [Slack Bot In:stemija slackbot] KEEP ALIVE : Reconencting to Slack. (false,true)
20 Dec 00:41:57 - [info] [Slack Bot In:stemija slackbot] Slack logging in ...
[Thu Dec 20 2018 00:41:57 GMT+0100 (CET)] INFO Connecting...
20 Dec 00:41:57 - [info] [Slack Bot In:stemija slackbot] in: Logged in:
20 Dec 00:43:27 - [info] [Slack Bot In:stemija slackbot] KEEP ALIVE : Reconencting to Slack. (false,true)
20 Dec 00:43:27 - [info] [Slack Bot In:stemija slackbot] Slack logging in ...
[Thu Dec 20 2018 00:43:27 GMT+0100 (CET)] INFO Connecting...
20 Dec 00:43:27 - [info] [Slack Bot In:stemija slackbot] in: Logged in:
20 Dec 00:49:57 - [info] [Slack Bot In:stemija slackbot] KEEP ALIVE : Reconencting to Slack. (false,true)
20 Dec 00:49:57 - [info] [Slack Bot In:stemija slackbot] Slack logging in ...
[Thu Dec 20 2018 00:49:57 GMT+0100 (CET)] INFO Connecting...
20 Dec 00:49:57 - [info] [Slack Bot In:stemija slackbot] in: Logged in:
20 Dec 00:57:27 - [info] [Slack Bot In:stemija slackbot] KEEP ALIVE : Reconencting to Slack. (false,true)
20 Dec 00:57:27 - [info] [Slack Bot In:stemija slackbot] Slack logging in ...
[Thu Dec 20 2018 00:57:27 GMT+0100 (CET)] INFO Connecting...
20 Dec 00:57:27 - [info] [Slack Bot In:stemija slackbot] in: Logged in:
yayadrian commented 5 years ago

@janvda sorry this is still an issue for you. Can you give me a rough idea of your use case?

janvda commented 5 years ago

@shanness

Maybe it is fixed now. I think I had another reconnect this morning (see below) without messages being received twice by the "Slack Bot In" node.

FYI I did 2 things:

  1. I set autoReconnect to false (var autoReconnect = false;) as the reconnection will be done by the slackKeepAlive() function.
  2. I have stopped another node-red instance that was also using a Slack Bot In node with same token (but I think it listened to a different slack channel).

22 Dec 06:31:14 - [info] [Slack Bot In:stemija slackbot] KEEP ALIVE : Reconencting to Slack. (false,true) 22 Dec 06:31:14 - [info] [Slack Bot In:stemija slackbot] Slack logging in ... [Sat Dec 22 2018 06:31:14 GMT+0100 (CET)] INFO Connecting... 22 Dec 06:31:15 - [info] [Slack Bot In:stemija slackbot] in: Logged in:

janvda commented 5 years ago

@yayadrian the use case is pretty simple. I have a node red flow that is handling any message that is posted on a channel of my slack work space that has been joined by my slack bot app. The Slack Bot In node is used to receive those messages. This flow is also posting a response on the same channel using the Slack Bot Out node.

janvda commented 5 years ago

@shanness The 2 changes proposed 2 comments ago didn't fix the issue regarding the slack bot in receiving every slack message multiple times. So today it received the slack bot in message 6 times (most likely due to several re-connections in the last days).

shanness commented 5 years ago

Hi @janvda, sorry for the delay, just back from holidays. Will have a look tomorrow to see if I can repeat your problem, unfortunately I restarted NR a few minutes before reading this, so will hopefully have a couple of reconnects by the morning.

shanness commented 5 years ago

Hey @janvda , I haven't been able to repeat this. Despite multiple reconnects, I'm only getting the messages once.

janvda commented 5 years ago

It happened again for me the 11th of January (I received the message 2 times) but on the 14th of January it is working again properly (message is only received once). I need to check log files (as I can't remember if I restarted node-red in that period).

Update: I only have the logs of the 16th Jan. So the relevant part is not available.

In the log of the 16th - I see following error but this seems not related to my problem regarding duplicate messages:

Trace
at Client.<anonymous> (/home/pi/.node-red/node_modules/node-red-contrib-slack/slackpost.js:125:21)
at emitOne (events.js:121:20)
at Client.emit (events.js:211:7)
at Client.onMessage (/home/pi/.node-red/node_modules/slack-client/src/client.js:481:21)
at WebSocket.<anonymous> (/home/pi/.node-red/node_modules/slack-client/src/client.js:149:24)
at emitTwo (events.js:126:13)
at WebSocket.emit (events.js:214:7)
at Receiver.ontext (/home/pi/.node-red/node_modules/slack-client/node_modules/ws/lib/WebSocket.js:816:10)
at /home/pi/.node-red/node_modules/slack-client/node_modules/ws/lib/Receiver.js:477:18
at Receiver.applyExtensions (/home/pi/.node-red/node_modules/slack-client/node_modules/ws/lib/Receiver.js:364:5)
at /home/pi/.node-red/node_modules/slack-client/node_modules/ws/lib/Receiver.js:466:14
at Receiver.flush (/home/pi/.node-red/node_modules/slack-client/node_modules/ws/lib/Receiver.js:340:3)
at Receiver.finish (/home/pi/.node-red/node_modules/slack-client/node_modules/ws/lib/Receiver.js:482:12)
at Receiver.expectHandler (/home/pi/.node-red/node_modules/slack-client/node_modules/ws/lib/Receiver.js:457:31)
at Receiver.add (/home/pi/.node-red/node_modules/slack-client/node_modules/ws/lib/Receiver.js:95:24)
at TLSSocket.realHandler (/home/pi/.node-red/node_modules/slack-client/node_modules/ws/lib/WebSocket.js:800:20)
at emitOne (events.js:116:13)
at TLSSocket.emit (events.js:211:7)
at addChunk (_stream_readable.js:263:12)
at readableAddChunk (_stream_readable.js:250:11)
at TLSSocket.Readable.push (_stream_readable.js:208:10)
at TLSWrap.onread (net.js:597:20)
16 Jan 21:26:40 - [error] [Slack Bot In:stemija slackbot] Error: [object Object]
Trace
at Client.<anonymous> (/home/pi/.node-red/node_modules/node-red-contrib-slack/slackpost.js:245:21)
at emitOne (events.js:121:20)
at Client.emit (events.js:211:7)
at Client.onMessage (/home/pi/.node-red/node_modules/slack-client/src/client.js:481:21)
at WebSocket.<anonymous> (/home/pi/.node-red/node_modules/slack-client/src/client.js:149:24)
at emitTwo (events.js:126:13)
at WebSocket.emit (events.js:214:7)
at Receiver.ontext (/home/pi/.node-red/node_modules/slack-client/node_modules/ws/lib/WebSocket.js:816:10)
at /home/pi/.node-red/node_modules/slack-client/node_modules/ws/lib/Receiver.js:477:18
at Receiver.applyExtensions (/home/pi/.node-red/node_modules/slack-client/node_modules/ws/lib/Receiver.js:364:5)
at /home/pi/.node-red/node_modules/slack-client/node_modules/ws/lib/Receiver.js:466:14
at Receiver.flush (/home/pi/.node-red/node_modules/slack-client/node_modules/ws/lib/Receiver.js:340:3)
at Receiver.finish (/home/pi/.node-red/node_modules/slack-client/node_modules/ws/lib/Receiver.js:482:12)
at Receiver.expectHandler (/home/pi/.node-red/node_modules/slack-client/node_modules/ws/lib/Receiver.js:457:31)
at Receiver.add (/home/pi/.node-red/node_modules/slack-client/node_modules/ws/lib/Receiver.js:95:24)
at TLSSocket.realHandler (/home/pi/.node-red/node_modules/slack-client/node_modules/ws/lib/WebSocket.js:800:20)
at emitOne (events.js:116:13)
at TLSSocket.emit (events.js:211:7)
at addChunk (_stream_readable.js:263:12)
at readableAddChunk (_stream_readable.js:250:11)
at TLSSocket.Readable.push (_stream_readable.js:208:10)
at TLSWrap.onread (net.js:597:20)
16 Jan 21:26:40 - [error] [Slack Bot Out:stemija slackbot response] Error sending to Slack: {"msg":"slow down, too many messages...","code":-1,"source":"gs-fra-ak8g"}
shanness commented 5 years ago

Hmm, strange. Not sure what to suggest, except to figure out if you somehow have multiple open connections to slack. I only have one

craig@home:~$ sudo netstat -pn | grep ':443.*node-red'
tcp        0      0 10.0.0.7:49304          13.54.39.212:443        ESTABLISHED 6066/node-red

Despite quite a few reconnects (oops, I spelt that wrong in the log message).

Jan 16 16:16:46 home Node-RED[783]: 16 Jan 16:16:46 - [info] [Slack Bot In:cmd bot in] KEEP ALIVE : Reconencting to Slack. (false,true)
Jan 19 05:22:52 home Node-RED[6066]: 19 Jan 05:22:52 - [info] [Slack Bot In:cmd bot in] KEEP ALIVE : Reconencting to Slack. (false,true)
Jan 20 16:49:35 home Node-RED[6066]: 20 Jan 16:49:35 - [info] [Slack Bot In:cmd bot in] KEEP ALIVE : Reconencting to Slack. (false,true)
Jan 21 16:15:22 home Node-RED[6066]: 21 Jan 16:15:22 - [info] [Slack Bot In:cmd bot in] KEEP ALIVE : Reconencting to Slack. (false,true)
Jan 22 16:17:29 home Node-RED[6066]: 22 Jan 16:17:29 - [info] [Slack Bot In:cmd bot in] KEEP ALIVE : Reconencting to Slack. (false,true)

I have little traffic on my inbound, due to it being a command channel, but checked the NR logs, and there are no dupes received. Next time you get one, try running the above netstat command and see if you somehow have multiple sockets connected.

@yayadrian I've squashed my stuff into one commit, would be good to have someone else test it to see if they have the issue janvda does. It's been working perfectly for me since this change.

travisghansen commented 5 years ago

@yayadrian I'd be happy to modernize some of the code base and clean up this along with several other issues (update to latest client, using credentials instead of normal settings, use proper configuration nodes, etc) if you'd be willing to accept an MR. It would break current setups though (particularly moving configuration to credentials and using proper config nodes)...any interest?

yayadrian commented 5 years ago

@travisghansen Yep I would be happy to take a look. I started on a new version but time was short and I haven't got far.

travisghansen commented 5 years ago

@yayadrian yeah, I took the liberty to rewrite everything and I have a really solid foundation now. inputs/outputs are quite different but I've implemented full usage of the 'Web' API (ie: you can invoke any method you want generically) and the 'RTM' listens to all events, not just incoming messages (ie: you can see reaction events, channel updates, etc, etc, etc).

I've added node statues so you can see when it's connected and loses connection and when activity is in place. Observing such, I've bee able to cut connectivity and watch the nodes go to 'disconnected' state and subsequently automatically return when connectivity was restored and go to a 'connected' state. I really like where it's headed.

Given the massive overhaul of config/inputs/outputs I'm still not sure whether you'd like to keep it in this repo. I'd love to keep it here as I feel this is pretty much the de-facto package but I'm happy to publish as something else altogether if you're not comfortable with it. If you want to proceed it's definitely a 'major' version bump for semver.

If we do move forward it will close nearly all the 'issues' in this repo (although I'm sure it'll add new ones). The code is however quite concise and has pretty small surface for error.

travisghansen commented 5 years ago

MR #22

travisghansen commented 5 years ago

@janvda or @shanness would either of you be willing to install the rewrite referenced above and let me know if you're still seeing connection issues? I've pretty much completed the functionality and have had it running for several days now so it should be stable. I've tested several reconnect scenarios and it's always reconnected perfectly.

shanness commented 5 years ago

Hey @travisghansen, sure, happy to test. I'm having a problem though.

 [node-red-contrib-slack/slack] Error: Cannot find module '@slack/client'

Because of this

  "dependencies": {
    "@slack/client": "^4.9.0"
  }

(which used to be "slack-client": "^1.4.0"

Pardon my ignorance about how to fix it, basically I've just changed the files in the installed package to symlinks to yours, which is breaking me restarting node-red.

craig@home:~/.node-red/node_modules/node-red-contrib-slack$ ltr
total 20
-rw-rw-r-- 1 craig craig 1105 Jan 24  2017 LICENSE
lrwxrwxrwx 1 craig craig   73 Mar  1 17:55 package.json -> /home/craig/apps/node-red-stuff/node-red-slack_travisghansen/package.json
lrwxrwxrwx 1 craig craig   70 Mar  1 17:56 README.md -> /home/craig/apps/node-red-stuff/node-red-slack_travisghansen/README.md
lrwxrwxrwx 1 craig craig   73 Mar  1 17:56 slackpost.js -> /home/craig/apps/node-red-stuff/node-red-slack_travisghansen/slackpost.js
lrwxrwxrwx 1 craig craig   75 Mar  1 17:56 slackpost.html -> /home/craig/apps/node-red-stuff/node-red-slack_travisghansen/slackpost.html
travisghansen commented 5 years ago

@shanness not sure if this will work with what you've done but we'll try it first. Just go into the new folder and run npm install which should install all the deps.

shanness commented 5 years ago

Hmm, when I did it from that directory, it created another node_modules dir in it (which is itself already in a node_modules dir). And that didn't fix it. Moving @slack from ~/.node-red/node_modules/node-red-contrib-slack/node_modules to ~/.node-red/node_modules/ didn't help either.

I then did the npm install from ~/.node-red, and it reset node-red-contrib-slack to the package version :(

Any idea how to install a non npm version of something to node red (i.e. from github like yours?)

i.e. this is what I got after the last npm install 

craig@home:~/.node-red/node_modules/node-red-contrib-slack$ ls -ltr
total 32
-rw-rw-r-- 1 craig craig 8650 Jan 24  2017 slackpost.js
-rw-rw-r-- 1 craig craig 6189 Jan 24  2017 slackpost.html
-rw-rw-r-- 1 craig craig 2461 Jan 24  2017 README.md
-rw-rw-r-- 1 craig craig 1105 Jan 24  2017 LICENSE

Sorry, I'm a coder, but not a JS coder, and don't know npm really.

shanness commented 5 years ago

Doh, should have read the man page for npm install it was just a matter of doing npm install https://github.com/travisghansen/node-red-slack from the ~/.node-red directory.

That seems to have gotten it working. It's not very backwards compatible though, channel isn't in the dialog (yet?), and the inbound messages aren't strings in msg.payload (instead it's an object) But that's probably a good thing, just gotta fix my nodes (it's a pretty breaking change though, requiring a lot of re-writing of my functions). Also, seems I need to set msg.topic = "message" and keep the msg.payload.channel (id) and set output text in msg.payload.text. It crashes with slack-rtm-out:b3de6698.41d428] TypeError: Cannot read property '0' of undefined if msg.topic isn't set, because of this line

        if (method[0] == "@" || method[0] == "#") {

And crashes with the same error if msg.payload.channel isn't set but msg.topic is set to message

Looking good though so far. Will update when I have it working properly, just wanted to let you know I've got it running.

shanness commented 5 years ago

OK, finally got it all working. Apart from the problems above, the I can't seem to send attachments like I used to. Used to do this :- image Now it just posts this image

I was using the outbound webhook for sending the former.

travisghansen commented 5 years ago

@shanness the interface has changed quite a bit (it would be easy to mimic previous behavior with a simple function node in front of the output node). In order to do complex messages you need to send the following to the slack-web-out (not the slack-rtm-out) node:

set msg.topic to chat.postMessage set msg.payload to the same structure as found here: https://api.slack.com/methods/chat.postMessage

ignore the token property and channel will intelligently handle @ or # channel names so you don't need to use the acutal channel id.

Let me know if that helps. Thanks for trying it out!

shanness commented 5 years ago

Cool, all working, thanks. For the record, the problem was that the old API used msg.attachments, and the new one uses msg.payload.attachments. I like it! Much nicer using the slack JS payload :)

I'll let you know how well it stays connected :) Thanks for your hard work! I think it's worth checking the topic (and channel) is set rather than giving the TypeError: Cannot read property '0' of undefined with no stack trace for users to try to debug. Then you can throw a specific error.

travisghansen commented 5 years ago

Yeah thanks for the feedback. I ran into that myself and mostly my thought has been pass through errors from slack directly...but that's clearly not even making it to slack so I'll rethink that a little.

travisghansen commented 5 years ago

@shanness just noticed you're earlier comment about backward compatibility etc. That's essentially what I was stating earlier in the thread is I don't really have any intention of making it backwards compatible. As it is it supports the fullness of slack's API which the other simply didn't do so it's much more generic (and powerful) now (which is why channel isn't an option at the node level as some API calls don't require it). It would be pretty easy for someone to front the output nodes with a simple function to replicate whatever the previous behavior was or do generally whatever custom stuff is wanted/needed. I have for example already made some nodes to handle sending /me messages from the bot (https://api.slack.com/methods/chat.meMessage). To do so I created a simple function node in front of the slack-web-out node to simplify the 'API' to send a /me message (ie: setting topic to chat.meMessage and building the appropriate msg.payload using the old topic as the channel property and the old payload as the text property).

I haven't updated the documentation yet but the general idea is:

  1. For the simplistic use-case, if topic starts with @ or # then it's assumed you're trying to send a message and the payload should be a string. The output node (web or rtm) automatically handles sending a simple message.
  2. For anything else, the topic is the slack rtm/web method/function and the payload is the arguments.
  3. The output for all of the above includes a msg.slackState object which has several properties containing the lists of users/channels/bot/team info/etc so downstream nodes can do 'lookups' without re-hitting the API (this data is currently refreshed in the background every 10 minutes)
  4. The msg.payload of the output side is the direct passthrough/response from the rtm/web invocation. However, to make things easier, before passing it through we traverse the response and set fooObject properties for channel/user/bot/etc (basically doing lookups from slackState for you) to make it easier to assert/switch/whatever based off of username/real name/etc instead of internal slack IDs.

As an example of that last point, the user_typing event (https://api.slack.com/events/user_typing) of the slack-rtm-in node produces:

{
  "type": "user_typing",
  "channel": "...",
  "user": "...",
  "channelObject": {
    "id": "...",
    "name": "...",
    "is_channel": true,
    "is_group": false,
    "is_im": false,
    "created": 1434735155,
    ...
  },
  "userObject": {
    "id": "...",
    "name": "...",
    "real_name": "...",
    ...
  }
}

instead of just:

{
  "type": "user_typing",
  "channel": "...",
  "user": "..."
}

All of the lookups are done dynamically/generically so regardless of what API response you get if the node finds a property that appears to be a user in some shape or form, it creates <property>Object attribute with the user lookup value. For example, in some of my messages I've seen bot_idObject getting created, but if it found a property called bot it would add botObject etc. Ultimately it's doing all the lookups from slackState so it could be done on your own but it's added to simplify.

Hopefully that helps clear things up a little bit and provides some insight into the design. I'll document all of it once everyone gives the metaphorical nod to the interface/decisions.

shanness commented 5 years ago

Hey @travisghansen , Yep, worked most of that out by looking at the code and output :) Nice work. Do you know if there is info about who is online? Would be useful, but the members in the slackState doesn't seem to have it. And yep, I agree not making it backwards compatible, would be messy and not the way forward.

travisghansen commented 5 years ago

@shanness yeah slack calls it presence. Depending on your needs there are 2 approaches.

  1. Use the web node to invoke https://api.slack.com/methods/users.getPresence
  2. Use the rtm node to invoke https://api.slack.com/events/presence_sub

The former would be a '1 shot' approach on an as-needed basis looking for a particular/known user.

The latter would subscribe you to the stream of events which would show up on all slack-rtm-in nodes. You won't receive the presence details on the output side of the request directly.

If you want to watch the whole team for example I would put a trigger node on some time interval of 10+ minutes in front of the slack-state node to emit the current state. Put a function node on the other side to iterate the users and build up the list of ids. Set msg.topic to presence_sub and set msg.payload to { ids: list_from_iteration } and push the result to the/a slack-rtm-out node. From then on your slack-rtm-in nodes will start receiving events with type: "presence_change".

I personally have a switch node right after the slack-rtm-in node to divide appropriately based on the type.

It may be a good idea for us to watch that event ourself and add the data to slack state. I wouldn't automatically subscribe but if the user/flow has subscribed then go ahead and start tracking it in the internal state so it's more accessible.

Edit: technically there is also https://api.slack.com/events/presence_query but it seems a waste of time in this context but it would be available.

travisghansen commented 5 years ago

@shanness ok, I've pushed another fairly large update. All state data has been updated to use hash maps so lookups can be done via key instead of traversing a list (array). The state node also added a second output that pushes events (currently only the ready event) which you can key on to trigger presence subscriptions after the state has been initialized on startup. The state also now tracks presence and dnd data. presence data is only tracked if the user/flow subscribes to presence events. I've also worked out all the events (I think) which will update state based on server-side events instead of needing to poll state every 10 minutes (which is still enabled).

Lastly, I've updated the general appearance of the nodes (updated color, icons, alignment, etc) to match guidelines and modernize.

Enjoy! Let me know how things look for you.

janvda commented 5 years ago

@travisghansen , Thanks for taking up this challenge. I am currently struggling with the new API that is not backwards compatible. I fully understand that you are breaking backwards compatibility, but it would be good to describe how the people can migrate there existing flows to the new API : I haven't got a clue if I need to use "slack web out" or "slack rtm out" instead of the former "slack out" node. At the moment my node-red application is not working and I am figuring out what I need to do.

It would be good to give some examples what the incoming payload could be for the different nodes. I have also no clue how I can receive messages posted on a certain slack channel and how I can best generate a response in the same channel.

.... after some struggling, I am gradually figuring out how to use the new API. One of the things I still need to figure out is how I can change the user (currently it is "bot") when posting a message with slack rtm out in a channel.

travisghansen commented 5 years ago

@janvda yeah thanks for your patience and taking the time to test it out! All of the documentation is the slack API documentation for requests/responses. For the slack-web-out node here: https://api.slack.com/methods and for the slack-rtm-in/out nodes here: https://api.slack.com/rtm

msg.topic is the function/method name, msg.payload is the params/args (for -out nodes). The slack-rtm-in node listens to all events and passes the event payload on directly as msg.payload (with some niceties added from msg.slackState added as <foo>Object properties. Click on any of the events from the rtm link above and you'll see the expected responses.

Using slack-rtm-out for sending messages should only be used for very basic stuff, just assume using the slack-web-out for anything beyond the simplest messaging use-case.

In order to mimic the previous outbound API try putting a function node like this in front of a slack-web-out node...it should effectively mimic previous behavior 100%, just point your old-style outputs to the function node:

// https://api.slack.com/methods/chat.postMessage
msg.topic = "chat.postMessage"
var payload = {
    text: msg.payload
};

if (msg.emojiIcon) {
    payload.icon_emoji = msg.emojiIcon;
}

if (msg.channel) {
    payload.channel = msg.channel
} else {
    payload.channel = "#your_default_channel_set_on_node_previously"
}

if (msg.username) {
    payload.username = msg.username;
    payload.as_user = false;
}

if (msg.attachments) {
    payload.attachments = msg.attachments;
}

msg.payload = payload;

return msg;

For inbound node try putting a function node like this after a slack-rtm-in node. From what I can tell it will mimic the old inbound node 100% as well:

// https://api.slack.com/events/message

if (msg.payload.type != "message") {
    return null;
}

// if you only want to watch a specific channel put name here
var channel = "";
if (channel && msg.payload.channelObject.name != channel.replace(/^@/, "").replace(/^#/, "")) {
    return null;
}

var payload = "";
if (msg.payload.text) {
    payload += msg.payload.text;
}

if (msg.payload.attachments) {
    if (payload) {
        payload += "\n";
    }

    msg.payload.attachments.forEach((attachment, index) => {
        if (index > 0) {
            payload += "\n";
        }
        payload += attachment.fallback;
    })
}

var slackObj = {
    id: msg.payload.client_msg_id,
    type: msg.payload.type,
    text: msg.payload.text,
    channelName: msg.payload.channelObject.name,
    channel: msg.payload.channelObject,
    fromUser: (msg.payload.userObject) ? msg.payload.userObject.name : "",
    attachments: msg.payload.attachments
};

msg = {
    payload: payload,
    slackObj: slackObj
}

return msg;

I don't have the old nodes installed anymore so I may have something off a little. If you still have them installed and want to send some examples over I can compare a little better.

Let me know how they work and I'll keep them updated and add them to the eventual documentation as transition examples.

travisghansen commented 5 years ago

@janvda I just noticed your comment about responding to messages in channels. Here's an example function node to put between a slack-rtm-in node and a slack-web-out node:

// ignore anything but messages
if (msg.payload.type != "message") {
    return null;
}

// ignore deleted messages
if (msg.payload.subtype == "message_deleted") {
    return null;
}

// ignore messages from bots
if (msg.payload.bot_id || msg.payload.userObject.is_bot) {
    return null;
}

// if you only want to watch a specific channel put name here
var channel = "";
if (channel && msg.payload.channelObject.name != channel.replace(/^@/, "").replace(/^#/, "")) {
    return null;
}

// only specific users
var username = "";
if (username && msq.payload.userObject.name.replace(/^@/, "") != username) {
    return null;
}

// check for keyword
// could use regex etc
if (!msg.payload.text.includes("keyword")) {
    return null;
}

// prepare outbound response
var topic = "chat.postMessage";
var payload = {
    channel: msg.payload.channel, // respond to same channel
    //text: '<@' + msg.payload.userObject.name + '>, thanks for chatting',
    text: '<@' + msg.payload.user + '>, thanks for chatting',
    //as_user: false,
    //username: "",
    //attachments: [],
    //icon_emoji: "",
}

msg = {
    topic: topic,
    payload: payload
}

return msg;
janvda commented 5 years ago

@travisghansen thanks a lot for sharing this. It will definitely help me. I will have a look at it later. I also think it would be good to enable "issues" on your node-red-slack repository (that is also the fork that I have cloned) so we can raise any issues there before they are integrated in this repository. One of the things I think you still need to do is changing the version from 1.2.0 to 2.0.0.

travisghansen commented 5 years ago

@janvda yeah no problem. Although the version is currently 0.1.2 I think it's probably best to bump to 2.0.0 when it's ready to publish like you mentioned. I've gone ahead and enabled issues there but I prefer to keep this work as 'upstream' as possible.

Regarding documentation, I think I can start working on that now, I feel like things have settled enough in the interfaces that it's probably about what it will be. I'll work on the README and the integrated documentation (as seen in the Node-RED interface) and start pushing those up.

yayadrian commented 5 years ago

All of my testings so far has been successful. There is clearly a big change required so we should differently bump to 2.0 when ready.

travisghansen commented 5 years ago

Given that the slackState object is now indexed do we think we should still be adding the <foo>Object properties to the msg.payload for convenience or should we strip them to be more concise?

yayadrian commented 5 years ago

@travisghansen From a users point of view, I think I would go with stripping them out to keep it concise. I'm not sure if there are performance benefits around doing this.

travisghansen commented 5 years ago

@yayadrian probably very little in the way of performance. It is handy but entirely possible to operate without them. For example, I haven't tested if a switch node let's you put something like slackState.members[msg.payload.user].name. I'll give it a whirl and see what happens.

UPDATE: yeah Node-RED switch nodes don't like that. Being able to switch on those properties is quite handy..

travisghansen commented 5 years ago

Of interest to the original issue: https://github.com/slackapi/node-slack-sdk/issues/710

travisghansen commented 5 years ago

I've pushed another commit with updated README.md. It includes details about how to use the nodes and how to migrate from the current version. I'm not really a technical writer so any feedback is welcome. I'm interested to know if it's understandable and flows well.

In addition, I've updated the reconnect settings per the results discussed in the issue linked above. Hopefully that fully addresses any ongoing connection issues.

janvda commented 5 years ago

Thanks @travisghansen for the great work - I have migrated my flows. Will keep you posted.

travisghansen commented 5 years ago

Has anyone here been using a slack user token (starts with xoxp-) vs a bot token (starts with xoxb-)? I've been using a bot token and want to make sure the new code at least semi works with a user token. Subsequently I want to document the required scopes for user tokens to work fully.

janvda commented 5 years ago

No, my token also starts with xoxb-

shanness commented 5 years ago

Thanks for the README update @travisghansen ! Nice work, explains it well. And yeah, I'm using a bot token for my stuff too.