-
-
Notifications
You must be signed in to change notification settings - Fork 59
Catch :noproc errors in Xandra.Connection
#385
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
Catch :noproc errors in Xandra.Connection
#385
Conversation
…get sent any :DOWN messages
:noproc errors in Xandra.Connection
lib/xandra/connection.ex
Outdated
|
|
||
| %Xandra.Error{} = error -> | ||
| {{:error, error}, Map.put(metadata, :reason, error)} | ||
| try do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrapping the whole case into a try block. I think that's not right, as it would hide exits that could happen anywhere.
Instead, we should wrap just the :gen_statem.call/2 into try. That would also mean significantly more readable code, as this starts to be a really convoluted nested piece of code.
Basically, do this:
case gen_statem_call_trapping_noproc(conn_pid, {:checkout_state_for_next_request, req_alias}) do
# ...
# New clause:
{:error, :noproc} ->
# ...
end
# Then:
defp gen_statem_call_trapping_noproc(pid, call) do
:gen_statem.call(pid, call)
catch
:exit, {:noproc, _} ->
{:error, :noproc}
endDoes that make sense? It will also, incidentally, make the diff a lot slimmer 🙃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes a lot of sense
|
Fantastic work 💟 |
Closes #383
Since we're catching the
:noprocerrors onXandra.Connectionlevel, we don't need to catch them inXandra.Cluster.