search

codedge-llc / kadabra

HTTP/2 client for Elixir
https://hex.pm/packages/kadabra
MIT License
43 stars 23 forks source link

Bad argument in arithmetic expression #38

Open slashdotdash opened 6 years ago

slashdotdash commented 6 years ago

Environment

Current behavior

We've encountered the following exception numerous times in production error reports when using the pigeon library for sending large batches of FCM push notifications (1,000 notifications):

Elixir.ArithmeticError: bad argument in arithmetic expression
  File "lib/connection/processor.ex", line 131, in Kadabra.Connection.Processor.process/2
  File "lib/connection.ex", line 155, in Kadabra.Connection.handle_info/2
  File "gen_server.erl", line 616, in :gen_server.try_dispatch/4
  File "gen_server.erl", line 686, in :gen_server.handle_msg/6
  File "proc_lib.erl", line 247, in :proc_lib.init_p_do_apply/3
  Module "Elixir.Kadabra.Connection", in Kadabra.Connection.init/1

Unfortunately I haven't seen it happen locally, nor have I been able to reproduce it.

I've looked through Kadabra.Connection.Processor module and am wondering whether the issue is because the default max_concurrent_streams setting in the Kadabra.Connection.Settings module is :infinite but line 131 is assuming it will be a number:

to_ask =
  settings.max_concurrent_streams - flow.stream_set.active_stream_count

Could that be the culprit for the exception?

hpopp commented 6 years ago

Thats probably it exactly. If you notice in the def block directly above:

  # nil settings means use default
  def process(%Frame.Settings{ack: false, settings: nil}, state) do
    %{flow_control: flow, config: config} = state

    Egress.send_settings_ack(config.socket)

    case flow.stream_set.max_concurrent_streams do
      :infinite ->
        GenServer.cast(state.queue, {:ask, 2_000_000_000})

      max ->
        to_ask = max - flow.stream_set.active_stream_count
        GenServer.cast(state.queue, {:ask, to_ask})
    end

    {:ok, state}
  end

Weird that I didn't make the same validation. I'll have a patch up.

hpopp commented 6 years ago

Should be fixed with v0.4.4. Let me know if it crops up again.

slashdotdash commented 6 years ago

@hpopp Thanks for the rapid fix and release. I will upgrade to v0.4.4 today ready for deployment.