Api.get_guild_members/2 results to a GenServer Ratelimiter error

ndac-todoroki commented 7 years ago

Using the method, it crashes with this output:

15:13:43.092 [error] GenServer Ratelimiter terminating
** (Poison.SyntaxError) Unexpected token at position 0: <
    (poison) lib/poison/parser.ex:57: Poison.Parser.parse!/2
    (poison) lib/poison.ex:83: Poison.decode!/2
    (nostrum) lib/nostrum/api/ratelimiter.ex:110: Nostrum.Api.Ratelimiter.format_response/1
    (nostrum) lib/nostrum/api/ratelimiter.ex:33: Nostrum.Api.Ratelimiter.handle_call/3
    (stdlib) gen_server.erl:636: :gen_server.try_handle_call/4
    (stdlib) gen_server.erl:665: :gen_server.handle_msg/6
    (stdlib) proc_lib.erl:247: :proc_lib.init_p_do_apply/3
Last message (from #PID<0.233.0>): {:queue, %{body: %{limit: 1..100}, headers: [{"content-type", "application/json"}], method: :get, options: [], route: "/guilds/260291356562423809/members/"}, nil}
State: []
Client #PID<0.233.0> is alive
    (stdlib) gen.erl:169: :gen.do_call/4
    (elixir) lib/gen_server.ex:734: GenServer.call/3
    (nostrum) lib/nostrum/api.ex:907: Nostrum.Api.get_guild_members/2
    (test_discord_bot) lib/test_discord_bot.ex:54: ExampleConsumer.handle_event/2
    (nostrum) lib/nostrum/consumer.ex:176: Nostrum.Consumer.do_event/3
    (nostrum) lib/nostrum/consumer.ex:170: Nostrum.Consumer.handle_events/3
    (gen_stage) lib/gen_stage.ex:2543: GenStage.consumer_dispatch/7
    (gen_stage) lib/gen_stage.ex:2083: GenStage.handle_info/2

HTTPoison is crashing trying to parse this:

<!DOCTYPE html>
<html lang=en>
  <meta charset=utf-8>
  <meta name=viewport content="initial-scale=1, minimum-scale=1, width=device-width">
  <title>Error 400 (Bad Request)!!1</title>
  <style>
    *{margin:0;padding:0}html,code{font:15px/22px arial,sans-serif}html{background:#fff;color:#222;padding:15px}body{margin:7% auto 0;max-width:390px;min-height:180px;padding:30px 0 15px}* > body{background:url(//www.google.com/images/errors/robot.png) 100% 5px no-repeat;padding-right:205px}p{margin:11px 0 22px;overflow:hidden}ins{color:#777;text-decoration:none}a img{border:0}@media screen and (max-width:772px){body{background:none;margin-top:0;max-width:none;padding-right:0}}#logo{background:url(//www.google.com/images/branding/googlelogo/1x/googlelogo_color_150x54dp.png) no-repeat;margin-left:-5px}@media only screen and (min-resolution:192dpi){#logo{background:url(//www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png) no-repeat 0% 0%/100% 100%;-moz-border-image:url(//www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png) 0}}@media only screen and (-webkit-min-device-pixel-ratio:2){#logo{background:url(//www.google.com/images/branding/googlelogo/2x/googlelogo_color_150x54dp.png) no-repeat;-webkit-background-size:100% 100%}}#logo{display:inline-block;height:54px;width:150px}
  </style>
  <a href=//www.google.com/><span id=logo aria-label=Google></span></a>
  <p><b>400.</b> <ins>That’s an error.</ins>
  <p>Your client has issued a malformed or illegal request.  <ins>That’s all we know.</ins>

and I can't understand why Google is returning the error.

Kraigie commented 7 years ago

For the body it looks like you're using a range for the limit. Instead try using just an integer. I'm not sure how httposion/hackney handles sending a range, but I would guess it doesn't (since I don't believe Erlang does at all). I can understand where the confusion lies based on the @spec. Apologies if it's not clear! :^)

I'll look into this more tomorrow if this doesn't resolve the issue.

ndac-todoroki commented 7 years ago

@Kraigie , thanks, and sorry for that. Soon after that I tried it with an integer, and still did not work.

After several investigations, it seems that passing API parameters as request bodies result to this crash. Changing this line to

case request(:get, Constants.guild_members(guild_id), "", params: options) do

solved the problem, with also changing constants.ex#L26 from

def guild_members(guild_id),                                  do: "/guilds/#{guild_id}/members/"

to

def guild_members(guild_id),                                  do: "/guilds/#{guild_id}/members"

(just removed the tailing slash. It'll be a 404 with it)

I'm not checking many other functions, so don't know if any other functions are affected by this problem. Also, while I referenced the master branch, I am using the stable one (the git one didn't get correct WS connections)

Kraigie commented 7 years ago

Ahh, yep. That makes sense. Any route that takes query string parameters should have that form.

By stable do you mean the Hex version? I'm interested in the inability to establish WS connections. There are known issues with OTP 20. Could you provide any errors? At this point the Hex version is horribly out of date, but I'm hesitant to publish an update with users running into this issue.

Thanks for taking the time to look into this!

ndac-todoroki commented 7 years ago

OK, I'll open a new issue for that!

Kraigie commented 7 years ago

Fixed with 150b5725fdd0a9f740a9eb06e3b9917c237d41e2.

Also added a warning to the README for the second issue.

Thanks!

Kraigie / nostrum

Api.get_guild_members/2 results to a GenServer Ratelimiter error #20