skoczen / will

Will is a simple, beautiful-to-code bot for slack, hipchat, and a whole lot more.
https://heywill.io
MIT License
406 stars 171 forks source link

Repeating responses on restart #27

Closed dpoirier closed 10 years ago

dpoirier commented 10 years ago

I'm experimenting with will right now so I restart it a lot, and every time it seems to respond again to messages that it had already responded to previously. I'm not sure why it would do that. If there's anything about the config or other information that would be helpful in understanding this, please let me know.

skoczen commented 10 years ago

Darn. I was afraid this would come up someday.

What's happening is that XMPP sends the last ~30 hipchat messages when you connect (as will does every restart.) To get around this, I just had will ignore any messages he hears in the first 3 seconds. (You can see the ugly hack in listener.py, line 97.) In your case, this isn't enough time.

Out of curiosity, what are the machine and internet connection specs for the machine you're running will on?

Thanks for the report! -Steven

dpoirier commented 10 years ago

It shouldn't be a particularly slow system - fairly recent laptop, business-class cable connection to the internet.

I'll try increasing the 3 seconds to 10.

skoczen commented 10 years ago

Very odd. Let me know how that goes!

dpoirier commented 10 years ago

10 seconds seems to be enough in my case.

Looking for a better solution, I was dumping the messages that come in at startup. They appear to have a "stamp" attribute with the original message timestamp. Maybe Will could ignore messages with a timestamp before the time the current invocation of Will started?

skoczen commented 10 years ago

Love love this idea. Are you up for implementing and submitting a PR?

dpoirier commented 10 years ago

We're still evaluating whether we're going to stick with HipChat, but if we do, I'll see if this works and submit a PR if so.

Dan Poirier dpoirier@caktusgroup.com

On Thu, May 15, 2014 at 1:23 PM, Steven Skoczen notifications@github.comwrote:

Love love this idea. Are you up for implementing and submitting a PR?

— Reply to this email directly or view it on GitHubhttps://github.com/greenkahuna/will/issues/27#issuecomment-43239280 .

skoczen commented 10 years ago

Sounds good - if not, let me know, and I'll add it to my list - it's a good idea either way!

Also, if you do go with another chat framework, my long-term goal is to make will's backends pluggable (there's a issue open for campfire support), so please let me know which one, and if you're interested, making will work with it is definitely possible.

dpoirier commented 10 years ago

Here's a first pass at a fix: https://github.com/dpoirier/will/compare/ignore_old_messages?expand=1

This seems to work, but just grubs around in the XML of the messages. There might be a cleaner way to get the information.

skoczen commented 10 years ago

It's possible there's a cleaner way, but I'm also of the "the perfect is the enemy of the good" mindset in cases like this. That code reads cleanly to me, and looks performant. If you're happy with it, send the PR over, and I'll get it merged in!

dpoirier commented 10 years ago

Now I'm not sure this is enough - I'm still seeing an occasional repeat as I've been stopping and starting the bot. I'll have to look into it some more, I guess; maybe that delay element isn't always included?

Also, I was just looking at http://help.hipchat.com/knowledgebase/articles/64377-xmpp-jabber-support-details and saw this:

    Room history is automatically sent when joining a room unless your JID resource is “bot”.

Could that be helpful? I don't know what a JID is.

skoczen commented 10 years ago

Oh, that's very interesting. Digging in!

skoczen commented 10 years ago

That. Is. Genius.

Looks to be working, here, mind checking it out and testing on your side? https://github.com/greenkahuna/will/pull/30

skoczen commented 10 years ago

Also, just for completeness, the JID is the jabber ID - in this case, it's what's being set in the WILL_USERNAME environment variable.

skoczen commented 10 years ago

Fixed in 0.4.8, closing!