aaronpk / Slack-IRC-Gateway

Bridge Slack rooms to IRC channels
Other
34 stars 6 forks source link

Connection/user model poorly fits IRC #3

Closed AbstractBeliefs closed 7 years ago

AbstractBeliefs commented 8 years ago

This issue kind of covers two areas:

  1. Spawning 1 connection per Slack user will cause moderately busy Slack rooms to bust IRC network spam triggers by generating too many connections. With default settings, and connecting to freenode, this is hit at about 10 unique users per rolling 5 min window.
  2. Spawning a new connection for each Slack user causes delays and out-of-order message delivery.

Part 2 could be somewhat resolved by building a pool of connections on startup, and having these connections change nick and join/part channel as required.

This additionally could solve part 1 to a degree, if the pool size is configurable so hosters can set the pool to a size lower than the limit for the network they are connecting to. However, once the number of active users exceeds the pool size, the channel will become very noisy from connections changing nick, optionally also parting and rejoining.

The traditional approach for bridge bots like this is to only have a single connection (for example called SlackBridgeBot) which handles all messages, prepending the Slack username before the content. I'd look at writing a PR to implement this approach if you indicate that you'd consider accepting it once written.

aaronpk commented 8 years ago

@AbstractBeliefs have you actually encountered these issues yet? In my experience it's been fine so far, although granted there have been maybe 3-4 simultaneous IRC connections at the most. The out of order messages are not ideal, but also not the end of the world, since IRC doesn't even guarantee message order delivery either.

I am actually strongly opposed to the "traditional" model of having a single bot proxy all Slack users to IRC. This leads to a poor user experience on the IRC side, since the messages look "ugly" (prepending a username to the IRC line, and having the nick be "SlackBridgeBot" or whatever). The solution implemented here is the only place I've seen it done where each Slack user appears as a native IRC user, making it almost transparent to IRC users.

AbstractBeliefs commented 8 years ago

Have you actually encountered these issues yet?

Yes - the multiple, fast, short lived connections have triggered the IDS at the institution we host behind, as the connection pattern matched that of a botnet using IRC to coordinate command and control.

This actually happened after the report above was filed, but in any case I feel it's worthwhile mentioning.

re. out of order messages and latency: this is hugely an issue, right now we're seeing spikes of lag of up to 6 minutes in the extreme case, but on average 2m between a new user talking, and their messages being forwarded to the IRC channel, though once caught up, the forwarded messages are nearly instantaneous. This has little to do with your implementation, but the architecture: making new connections is slow compared to using a shared connection and simply sending new messages, simply because IRC servers make us wait a long time to connect some times.

Aside from this, the main issue for us is that we often have 5/6 different users on Slack jumping into the discussion at critical junctures to make points. In this case, discussion on Slack continues as normal, but on IRC, we suddenly see people talking at no one, and then some time later we get barrages of 6/7 lines all at once, out of order, and out of context. This often happens 3/4 times as different people come and go. I understand that IRC doesn't guarantee delivery order, but it's failure mode tends to be less challenging to accept than this, and certainly less common.

Overall, while I can understand why this would be a UX improvement in a fairly long-running conversation with few people coming and going, it's much less workable in larger more bustle-y channels, where we make an effort to make people feel welcome to jump in and out of discussion as they desire. For those who want the shiny UX, I recommend to simply use Slack. Those who use IRC seem more happy to take a bit of a hit in UX to get access to what would otherwise be many Slack teams, and additionally, the severity and frequency of out of order message delivery with fresh connections is sufficiently high that I would say that there's a net UX penalty.

AbstractBeliefs commented 8 years ago

As an example, here's a pic of some slack messages being late, out of order, and breaking context: out of order messages

Lachlan/@duck made a point, it was responded to, but by the time it reached IRC it was all back to front and way late.

There have been many other instances, but none have been quite so clear (nor quite so publically postable) as this.

As mentioned, this bot is your baby and if this is a design choice that you want to close as wontfix, feel free to do so rather than just leaving it dangling, but I still contend that this is a step backwards for most IRC users, and even though IRC doesn't guarantee message ordering, this isn't license to make it a common occurrence.

aaronpk commented 8 years ago

Thanks for the bump.

That's a really odd example since both Slack users weren't yet connected to IRC, and yet the one that spoke later connected to IRC first.

I'm surprised that you're seeing 2m as an average before you see the first message. My experience (with Freenode) is about a 20-30 second delay when someone first joins. I'm tempted to increase the timeout from 10 minutes to something much longer, like 1-2 hours, to avoid the Slack users being disconnected from IRC as often, which would help eliminate this lag. Do you think that would help your situation as well, where the Slack users would be nearly continuously connected to IRC in that case?

AbstractBeliefs commented 8 years ago

While in my heart of hearts I still feel that the model being used doesn't well fit with IRC, I wouldn't mind trialing this as a compromise - it would likely fix outright the issue we had with triggering the uni IDS, and it would probably reduce the issues with out of order messages and joins.

I think that while we'll just have to agree to disagree on the philosophy of UX on IRC, I think a longer timeout might be a pragmatic fix for the symptoms we're seeing.

aaronpk commented 8 years ago

I forgot that I already added that timeout as a config option. Try increasing it in your config.json. https://github.com/aaronpk/Slack-IRC-Gateway/blob/master/config.example.json#L7

I definitely do not want to change the fundamental model of how this gateway works to make it use a single IRC user. There are already other Slack bridges that do it that way, and I couldn't find one that connected to IRC as individual users, which is why I wrote this in the first place.

aaronpk commented 7 years ago

Thanks again for the report, but I'm going to close this as wontfix. We've continued to use the gateway over the past year and have not encountered these long delays, and it has continued to be a useful tool. I'm standing by the design decision of one IRC connection per Slack user, since it provides the best experience on the IRC side, often IRC users don't even realize Slack users aren't actually on IRC. I've continued to make some additional unrelated improvements: handling image uploads, converting slack emoji to native emoji and adding support for multiple channels.

AbstractBeliefs commented 7 years ago

I don't necessarily agree, but thank you for considering and bringing a conclusion to this issue.

Congrats on the new features and keep up the good work.