search

BlakeWilliams / Elixir-Slack

Slack real time messaging and web API client in Elixir
MIT License
675 stars 182 forks source link

Swap rtm.start for rtm.connect #177

Closed acconrad closed 5 years ago

acconrad commented 5 years ago

According to the Slack docs on rtm.start:

Please use rtm.connect instead, especially when connecting on behalf of an Enterprise Grid customer.

The API is very similar albeit pared down from start. If you're not comfortable with this let me know and I can always append connect rather than replace, but I'd imagine given the recommendations this is a suitable option and we can always add start back in later.

BlakeWilliams commented 5 years ago

@acconrad Have you tried this with a bot that's being actively used?

It looks like rtm.connect won't pre-fill channels, users, etc. so I'm afraid it may break existing bots, or require more action on our end.

acconrad commented 5 years ago

@BlakeWilliams yeah I've used both with a bot I'm using, which prompted the update. To be fair, I didn't need all of the extra JSON response info from rtm.start so if you're concerned we can always provide an upgrade guide and/or just add in the function rather than replace it.

acconrad commented 5 years ago

@BlakeWilliams any chance we can get this merged in? Been a while, and I'm still actively using my own branch of this package

BlakeWilliams commented 5 years ago

Thanks for the patience, been a bit busy so this went under the radar for a bit. Looks good, thanks!

acconrad commented 5 years ago

@BlakeWilliams no worries! just so you know the test failed even though the code has been updated, does there need to be a new contract issued w/ the Slack API in order to have it know what functions it requires?

aquarhead commented 5 years ago

This is failing for me

** (EXIT from #PID<0.323.0>) shell process exited with reason: an exception was raised:
    ** (KeyError) key :bots not found in: %{ok: true, self: %{...}, team: %{...}, url: "..."}
        (slack) lib/slack/bot.ex:90: Slack.Bot.init/1
        (websocket_client) /.../deps/websocket_client/src/websocket_client.erl:164: :websocket_client.init/1
        (stdlib) gen_fsm.erl:351: :gen_fsm.init_it/6
        (stdlib) proc_lib.erl:249: :proc_lib.init_p_do_apply/3

Reverting to 4812cf8c77ceed74ef5fb93ffcc14efe0339464f works... Seems Slack.Bot.init/1 needs to be updated to not try to unpack extra information that rtm.connect does not return compared to rtm.start

binaryseed commented 5 years ago

Just saw the same error ^^