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

handle errors in client calls #470

Closed
wants to merge 3 commits into from

Conversation

v0idpwn
Copy link
Contributor

@v0idpwn v0idpwn commented Sep 3, 2021

brod_client safe_gen_call implementation leaked timeouts

Closes #468

brod_client safe_gen_call implementation leaked timeouts
@zmstone
Copy link
Contributor

zmstone commented Sep 3, 2021

thanks for the PR.
but the reason reported in #468 isn't {timeout, _} rather

[
  {{'localhost', 29092},
   {{{:kpro_req, #Reference<0.3351001794.2516844547.112162>, :api_versions, 0,
      false, []}, :timeout},
    [
      {:kpro_lib, :send_and_recv_raw, 4, [file: 'src/kpro_lib.erl', line: 70]},
      {:kpro_lib, :send_and_recv, 5, [file: 'src/kpro_lib.erl', line: 81]},
      {:kpro_connection, :query_api_versions, 4,
       [file: 'src/kpro_connection.erl', line: 246]},
      {:kpro_connection, :init_connection, 2,
       [file: 'src/kpro_connection.erl', line: 233]},
      {:kpro_connection, :init, 4, [file: 'src/kpro_connection.erl', line: 170]},
      {:proc_lib, :init_p_do_apply, 3, [file: 'proc_lib.erl', line: 226]}
    ]}}
]

which is the {error, Errors} where Errors is a list of per-host error, returned from kpro_brokers:connect_any https://github.com/kafka4beam/kafka_protocol/blob/9ee00f2efa97df23c13d12a48196bed11e183f41/src/kpro_brokers.erl#L294
which in turn caused this exit exception:

{error, Reason} -> erlang:exit(Reason)

@v0idpwn
Copy link
Contributor Author

v0idpwn commented Sep 4, 2021

Oh, I see! Thats a call with infinity timeout.

@v0idpwn v0idpwn changed the title handle timeouts in client calls handle errors in client calls Sep 11, 2021
@v0idpwn
Copy link
Contributor Author

v0idpwn commented Sep 11, 2021

Pushed a fix for that

all safe_gen_call calls have infinity timeout
Comment on lines +845 to +846
exit : {reason, _} ->
{error, reason}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
exit : {reason, _} ->
{error, reason}
exit : Reason ->
{error, {client_down, Reason}}

@zmstone
Copy link
Contributor

zmstone commented Sep 14, 2021

please help to update changelog.md (bump to 3.16.2)

@zmstone
Copy link
Contributor

zmstone commented Feb 5, 2022

#492

@zmstone zmstone closed this Feb 5, 2022
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

Successfully merging this pull request may close these issues.

produce_sync raises on timeout
2 participants