SlackRTM -> HObot -> Telegram fails to relay through

[pleasantone]

...but Telegram -> HObot -> Slack works just fine.

SlackRTM -> HO works. HO -> SlackRTM works. TG -> HO works. HO -> TG works.

Telegram is registering on event=hangups.ChatMessageEvent (_on_hangouts_message). Slack is registering an input handler on "allmessages".

Slack is using: coro_send_message(hangoutid, msg, context={'slack': True} to send messages to hangouts.

Telegram is using coro_send_message(hangoutid, msg) to send messages to hangouts.

I suspect that we need to add a context otherwise it won't get relayed through.

[pleasantone]

The bug is in slackrtm.

When it sends a message out that is a "relay message" it should be attaching a reprocessor (similar to the legacy slack's _slack_repeater_cleaner()) which sets a "do not rebroadcast this to myself" flag that does something vaguely like this:

def _slack_repeater_cleaner(bot, event, id):
    event_tokens = event.text.split(":", maxsplit=1)
    event_text = event_tokens[1].strip()
    if event_text.lower().startswith(tuple([cmd.lower() for cmd in bot._handlers.bot_command])):
        event_text = bot._handlers.bot_command[0] + " [REDACTED]"
    event.text = event_text
    event.from_bot = False
    event._slack_no_repeat = True
    event._external_source = event_tokens[0].strip() + "@slack"

I'll take this on. The magic is that a message that is RELAYED should have from_bot = False because it's not really from the bot (kinda sorta).

@nylonee @endofline Is there an "official" way to do this in the relaying framework that we never used?

[endofline]

quick comment: there isn't an official way yet since the reprocessor was only used in slack legacy and API. now would be a good time to figure out a pattern that we can reuse in any future bridges and some standard documentation to go with it. perhaps we can maintain a per-event checklist that keeps a tracking record of which plugin-unique_room_or_conv_id has already relayed it.

p.s. for those just joining us, reprocessor was unbroken in v3. as far as i can tell its permanently broken on v2 due to a protocol change, so plugins backported to v2 will probably experience message duplication or (in worse case scenarios) endless looping

[pleasantone]

Yeah, I tried a few other ways other than the reprocessor (which really is overkill) and failed. I think there are possibly two things we want to try to solve, and they may not have the same solution:

1. Has a message already been sent out a particular output channel

2. How do we indicate the source of a message so it never gets relayed back out the same output channel. is_relayed is a hack/kludge, and I suspect what we really want is to pass something in in the context parameter. My big problem was that context itself is lost in a rebroadcast, hence resorting to the reprocessor which comes along after the event has actually been generated.

On Mar 10, 2017, at 4:45 PM, Innocent Bystander notifications@github.com wrote:

quick comment: there isn't an official way yet since the reprocessor was only used in slack legacy and API. now would be a good time to figure out a pattern that we can reuse in any future bridges and some standard documentation to go with it. perhaps we can maintain a per-event checklist that keeps a tracking record of which plugin-unique_room_or_conv_id has already relayed it.

p.s. for those just joining us, reprocessor was unbroken in v3. as far as i can tell its permanently broken on v2 due to a protocol change, so plugins backported to v2 will probably experience message duplication or (in worse case scenarios) endless looping

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/hangoutsbot/hangoutsbot/issues/781#issuecomment-285823754, or mute the thread https://github.com/notifications/unsubscribe-auth/AFF6kfmUOEbPswRDdY_g5NvU6TbfdcnYks5rke6wgaJpZM4MZoYa.

[endofline]

the context parameter itself is a kludge. the protobuf entity that we use for relaying the reprocessor id only seems to accept scalar values, in this case the reprocessor id string which is remapped to some other function or handler. as far as i can tell that's the only way you can make metadata survive a trip through google servers at the moment.

therefore if you actually need the send_message context retained, we could map an automated reprocessor to re-attach the context values back at the handler level prior to entry into the command, chat handlers

[thannemann]

I have seen recent work on #636 (discord sync plugin) so I am going to reference it here so it can be verified it works properly with the reprocessor in v3.

[endofline]

further comment: what if you could relay before the broadcast? we do have a sending handler which catches outgoing message first before the bot sends it. it would more suitable to perform the relay here, and would simplify the type of context that needs to survive the broadcast: in this scenario, we only need to specify a generic no-relay flag. all relay plugins would register a dedicated sending handler (something like syncrooms but without modifying the broadcast list) to filter which conversations need to be relayed

[endofline]

types of messages:

• user messages in a hangout
  • sending handler: NA, sent by other clients
  • allmessages handler: no metadata / standard non-bot event
  • relay: always
  • summary: message relayed to all bridge plugins post-send (i.e. when received by hangout)
• user messages in third-party chat
  • sending handler: context = "no-relay" + channelid + pluginname (optionally +"bridge")
  • relay: not by same pluginame, channelid
  • allmessages handler: context = "no-relay"
  • relay: never
  • summary: message relayed to all bridge plugins except source (plugin, channel) during pre-send
• bot messages in a hangout (as result of user command)
  • sending handler: context = pluginname (or none, for old plugins)
  • relay: always
  • allmessages handler: no metadata / standard bot event
  • relay: never
  • summary: message relayed to all bridge plugins during pre-send

[pleasantone]

Consider the case where we want to syncrooms slack channel/team to slack channel/team.. it's not no-relay, it's no relay back to source x

On Mar 10, 2017, at 6:20 PM, Innocent Bystander notifications@github.com wrote:

types of messages:

user messages in a hangout sending handler: NA, sent by other clients message handler: no metadata / standard non-bot event relay: always user messages in third-party chat sending handler: context = "no-relay" + "bridge" + pluginname relay: not by same pluginame message handler: context = "no-relay" relay: never bot messages in a hangout (as result of user command) sending handler: context = pluginname (or none, for old plugins) relay: always message handler: no metadata / standard bot event relay: never — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

[endofline]

that would be a filtering issue resolved at the sending handler, I've updated the "story"

[endofline]

modified the story to reflect use of "allmessages" handler rather than "messages", as demonstrated

[endofline]

~consider adding sourcebot as well to the context, as running this on upgraded multi-bot relays may cause message looping/duplication since upgraded bots can see each other's context~ no longer an issue with passthru and context implementations since it relies on client-side storage

[endofline]

after some testing with context-aware syncrooms, we may have to work on a unified sending handler pattern that can call a standard shared function registered through all chat bridges to format incoming identities and messages. we can consider using call_shared; or passing the function directly via passthru. the code seems a lot easier to build tho (sending handler _broadcast, allmessages handler _repeat), and may have some bugs right now, I'm committing it as a guide, feel free to modify it further to test interaction with other chat bridges.