Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bad argument in arithmetic expression #38

Open
slashdotdash opened this issue Oct 5, 2018 · 3 comments
Open

Bad argument in arithmetic expression #38

slashdotdash opened this issue Oct 5, 2018 · 3 comments

Comments

@slashdotdash
Copy link

slashdotdash commented Oct 5, 2018

Environment

  • Elixir: v1.6.6
  • Operating system: Docker container running alpine-elixir

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
Copy link
Member

hpopp commented Oct 5, 2018

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
Copy link
Member

hpopp commented Oct 13, 2018

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

@slashdotdash
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants