search

BlakeWilliams / Elixir-Slack

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

Crash upon start #181

Closed binaryseed closed 5 years ago

binaryseed commented 5 years ago

Hi there! https://github.com/BlakeWilliams/Elixir-Slack/pull/177 introduces a bug that causes the app to crash upon start...

See the https://github.com/BlakeWilliams/Elixir-Slack/pull/177#issuecomment-461811475

** (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

cc/ @acconrad @BlakeWilliams @aquarhead

axelson commented 5 years ago

I'm seeing this too. Using this as the slack dependency works for me:

{:slack, github: "BlakeWilliams/Elixir-Slack", ref: "4812cf8"},
BlakeWilliams commented 5 years ago

Interesting. I’ll look into this soon and revert and re-release or pull the existing release if necessary.

acconrad commented 5 years ago

@BlakeWilliams This was my release. rtm.start is deprecated and not recommended by Slack anymore:

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

https://api.slack.com/methods/rtm.start

As the documentation for rtm.connect mentions:

Unlike rtm.start, this method is focused only on connecting to the RTM API.

Use this method in conjunction with other Web API methods like channels.list, users.list, and team.info to build a full picture of the team or workspace you're connecting on behalf of.

bots, channels, groups, users, and ims should be changed from being mapped from rtm.start and should be called directly with methods like channels.list to get the state of the channels, users.list to get the list of users, etc.

I can get to a fix for this later tonight.

acconrad commented 5 years ago

@binaryseed @axelson @BlakeWilliams I've got a tentative fix open with #184 I just need some guidance to get the tests passing and then we should be good to go

perzanko commented 5 years ago

I have the same issue 😎

acconrad commented 5 years ago

@perzanko are you on the latest version? it should be resolved now via #190

axelson commented 5 years ago

But do note that there isn't a release with that change yet, so you'd have to use master directly. @BlakeWilliams is it possible to cut another release with the fix?

BlakeWilliams commented 5 years ago

@axelson 0.19.0 was released that includes that change I believe. I'll update the README and push the tag.

axelson commented 5 years ago

Ah, I see. Didn't notice that release I guess. Thanks for updating the readme and pushing the tag!

perzanko commented 5 years ago

@acconrad @axelson @BlakeWilliams Now it works - THANKS 👍