alexandre-normand/slackscot

Spamming "I don't understand"

Dasio opened this issue · 15 comments

Dasio commented

Describe the bug

Bot is reacting to his own messages

Version

Slackscot version: v1.37.0

To Reproduce

Steps to reproduce the behavior:

  1. DM to bot something he can't handle
  2. Bot responds " I don't understand. Ask me for "help" to get a list of things I do"
  3. He starts responding to his previous messages in infinity loop

Expected behavior

Bot should ignore his own messages

Anything you'd like to add

I found that s.selfID differs from m.BotID, any idea why?

// Ignore messages_replied and messages send by "us"
	if m.User == s.selfID || m.BotID == s.selfID {
		s.log.Debugf("Ignoring message from user [%s] because that's \"us\" [%s]", m.User, s.selfID)
		return responses
	}

One more question:

  • Can I send more than one message to one action? I need to split up messages but by my own splitting (split daily menu by restaurant)
  • Probably I can do it by returning nil in answerer and sending messages via SlackClient and PostMessage
Dasio commented

I tried https://api.slack.com/methods/bots.info/test, as bot I used m.BotID
In response there is id and user_id
id == m.BotID
user_id == s.selfID

But in https://api.slack.com/methods/rtm.connect/test (which I think slackscot is using)
I will get bot user_id and not id

Maybe I installed app wrongly?
In slack workspace customize, I see my app with 1 configuration where is 1 bot user. It's private distribution. I'm using Bot User OAuth Access Token.

Hey @Dasio!

That's interesting. I don't have a quick and definitive answer but hopefully I can provide a few pointers so we can figure this out.

Before we get to the problem you're seeing, let me just answer your other quick questions first:

Can I send more than one message to one action? I need to split up messages but by my own splitting (split daily menu by restaurant)

The built-in reaction triggering just has a simple 1 reaction per triggering message. I don't know if it makes sense in your case but you could consider having a message with sections using block kit which is supported via an Answer with ContentBlocks.

Probably I can do it by returning nil in answerer and sending messages via SlackClient and PostMessage

That would work but you'd lose the ability for slackscot to manage threaded replying/reaction updates/deletions. It might not be a big deal in your case but I just wanted to point it out the tradeoff.

Back to your main problem, it might be useful to run with debug enabled (by adding "debug": true to your configuration) and see what's the self-id from the s.log.Debugf("Caching self id [%s], self name [%s] and self prefix [%s]\n", s.selfID, s.selfName, s.selfUserPrefix) log statement and compare that to the content of the slack message for the DMs.

Regarding the way the selfID is determined, slackscot is using https://github.com/nlopes/slack/blob/master/websocket.go#L81 which returns what slack includes on the rtm.connect response (as opposed to the bots.info.

You seem to be installing your app correctly and the Bot User Oauth Access Token is the correct one to use.

Let me know what you find out. I'd like to make sure slackscot handles this correctly if something's missing.

Dasio commented

Caching self id [US42F34DS], self name [flowbot] and self prefix [<@US42F34DS> ]
This is my message from my personal slack:
Self: US42F34DS, User: U19JCMCGP, Bot:
This is message from bot:
Self: US42F34DS, User: , Bot: BS4FWCRH6

rtm.connects return US42F34DS
bot.info as bot = BS4FWCRH6 returns

{
    "ok": true,
    "bot": {
        "id": "BS4FWCRH6",
        "deleted": false,
        "name": "FlowBot",
        "updated": 1577640269,
        "app_id": "ARRN7RSF4",
        "user_id": "US42F34DS",
        "icons": {
        }
    }
}

What about this https://github.com/alexandre-normand/slackscot/blob/master/slackscot.go#L707 ?
This should force to send message as requested id, which is selfID and it doesn't work?

But based on https://api.slack.com/methods/chat.postMessage
When as_user is true:
Slack App bot user token
inherits Slack App's icon and app's bot username

Which seems working as intended?

Thanks for the details, @Dasio! This is very useful.

Do you mind doing a quick test so I can confirm my understanding of the as_user and username options on sendMessage? I'd like to see what happens (and how it compares to the details you included in your last output) in your case if you removed slack.MsgOptionAsUser(true) from line 707 so that it reads:

options := []slack.MsgOption{slack.MsgOptionText(o.OutgoingMessage.Text, false), slack.MsgOptionUser(s.selfID)}

If you're using go modules, you can use gohack and do gohack get github.com/alexandre-normand/slackscot to get a copy of slackscot you can change and build with.

There's definitely something not right in how I was setting up the send message options as as_user and username aren't to be used together (the doc for username says Set your bot's user name. Must be used in conjunction with as_user set to false, otherwise ignored. See authorship below.).

Dasio commented

Nothings changed
Caching self id [US42F34DS], self name [flowbot] and self prefix [<@US42F34DS> ]
Self: US42F34DS, User: , Bot: BS4FWCRH6

Also I tried just with slack.MsgOptionAsUser(true) and also without both. The same result.

Anyway thanks for gohack tip.

Also I tried just with slack.MsgOptionAsUser(true) and also without both. The same result.

Did you try with only slack.MsgOptionUser(s.selfID)? That's the one I'd be the most interested in. I have a PR here to make that change since having both options used together never really made sense.

Dasio commented

Yes, nothings changed. Still there are messages in thread (replies to my message) with botID BS4FWCRH6

func MsgOptionUser(userID string) MsgOption {
	return func(config *sendConfig) error {
		config.values.Set("user", userID)
		return nil
	}
}

I can't see any user in slack documentation

Anyway it's weird that only I have this issue?
What about implementing this solution? https://github.com/nlopes/slack/issues/361

You're right that this user option doesn't seem to exist in the slack documentation. I might just go back and remove it and bring back the AsUser(true). The solution to get the user info and find the bot ID seems valuable. It's going to require a few tests to change but it shouldn't be too bad.

That said, I am also curious as to what's different in your app configuration. Do you mind comparing your scope and settings to the ones I'm sharing below for one of my test apps?
Screenshot 2019-12-30 15 00 19

Screenshot 2019-12-30 15 00 35

Dasio commented

I got only bot
I tried to set everything as you, I thought it would be chat:write:bot. Reinstalled app, but nothings changed :(

Edit: Wait, somethings changed
"reply_users":["US42F34DS"],"replies":[{"user":"US42F34DS","ts":"1577748207.027600"}] user is as selfID but still non-stop spam, because in message BotID is BS4FWCRH6 and not US42F34DS and user is empty.

Dasio commented

If I set in config
"threadedReplies": false,
It works

Thanks for all the debug info. I'll have to sit down and review this but I have a hunch there might be a bug caused by the handling and sub messages when dealing with threading. In the meantime, you might want to go back to your original threadedReplies configuration and try it out with the code that fetches the bot ID from the user profile and adds filtering on that:
I'm not done updating the tests so the PR isn't open yet but you can do go get github.com/alexandre-normand/slackscot@ fa9a310cb9e3f0781b1661c0bfb62a255c298a0b and try it out as I'd be interested to see if that makes the detection of self more robust.

Dasio commented

Thanks, really appreciate it, it works.

I got some 2 minor issues:
When I return nil in answerer, bot responded "I don't understand. Ask me for "help" to get a list of things I do" which may be intended behaviour?

Before, I was using https://github.com/abourget/slick
I set 7 reactions to message, with slick it was nearly instant. With slackscot, I can see how each one is added individually (maybe 1-2s) - not a big issue, just something I run into. But slackscot and slick are using the same API, so it's weird, maybe message processing is "slower"? Bot add reaction, there is a new incoming event "reaction_added" and after he adds another reaction? Idk.

for _, react := range slackMsg.Reactions {
    err := p.EmojiReactor.AddReaction(react, slack.ItemRef{
        Channel: m.Channel,
        Timestamp: timeStamp,
    })
    if err != nil {
        log.Println(err)
    }
}

Thanks for the feedback, @Dasio.

When I return nil in answerer, bot responded "I don't understand. Ask me for "help" to get a list of things I do" which may be intended behavior?

This should only be the case if you're in a channel and tagging the bot user @flowbot do something or if you're in a direct conversation with the bot. Otherwise, all plugins returning nil answers will just have the bot remain silent. This is sort of by design as it was assumed that someone directly messaging the bot (either via @user message or in a direct conversation) was trying to trigger some interaction.

I set 7 reactions to message, with slick it was nearly instant. With slackscot, I can see how each one is added individually (maybe 1-2s) - not a big issue, just something I run into. But slackscot and slick are using the same API, so it's weird, maybe message processing is "slower"? Bot add reaction, there is a new incoming event "reaction_added" and after he adds another reaction? Idk.

That one is interesting. The slack API only allows adding one emoji reaction per call so I'd assume that slick would be limited in the same fashion. The slackscot EmojiReactor interface is just an interface to allow testability without extra processing but it's actually directly implemented by the nlopes/slack.Client so there's no real overhead. It also looks like slick is using nlopes/slack with the same function.

FYI: I released v1.37.1 just now with the fix to use the bot ID to filter self messages.

Dasio commented

Thanks, it's working.