mickel8

Membrane Core Team

How to correctly handle GenServer.call exits because of non-existing process

GenServer.call might fail when a process we try to execute a call on no longer exists. Are there any guidelines on how to handle such situation? Let’s assume the following scenario:

defmodule RoomService do
  def list_rooms() do
      RoomRegistry
      |> Registry.select([{{:"$1", :_, :_}, [], [:"$1"]}])
      |> Enum.map(&Room.get_state(&1))
      |> Enum.reject(&(&1 == nil))
  end
end

defmodule Room do
  def get_state(room_id) do
    registry_room_id = registry_id(room_id)
    GenServer.call(registry_room_id, :get_state)
  end
end

WIth the above API, RoomService will crash when any of the calls to the Room.get_state crashes which is unwanted behavior.

RoomService cannot wrap Room.get_state into try catch as it is not aware of get_state internal implementation.

We could use try catch inside get_state like:

  def get_state(room_id) do
    registry_room_id = registry_id(room_id)

    try do
      GenServer.call(registry_room_id, :get_state)
    catch
      :exit, {:noproc, {GenServer, :call, [^registry_room_id, :get_state, _timeout]}} ->
        Logger.warning(
          "Cannot get state of #{inspect(room_id)}, the room's process doesn't exist anymore"
        )

        nil
    end
  end

but this effectively means that every single GenServer.call should be wrapped into try catch.

I feel like I am missing something

19 comments

#genserver

4 2131 19

2023-09-15 13:01:56 UTC

First 10 of 19 Posts

LostKobrakai

That’s the idea. With a GenServer.call the caller is expected to need the response to the call. If a response is impossible to get it’s expected that the caller cannot/should not continue.

Post #1

mickel8

Membrane Core Team

How can the Room module know the way it is going to be used by other modules? Room module can’t know whether its caller should or shouldn’t continue when a call to the room crashes. This is caller specific behavior, especially when you are writing a library which exposes a GenServer. You can’t know how users of this library are going to use your API. Some of them might want to crash with GenServer.call crash, some other might not.

Post #2

LostKobrakai

Catch the exit of GenServer.call and give the caller of the function enough context to make their own decisions. Room is still the module doing the GenServer.call, so it can decide how it deals with the fact that this might cause an exit.

Post #3

mickel8

Membrane Core Team

So we end up with the solution where we wrap every single GenServer.call with a try catch, doesn’t we? Sounds like something we should take care of in the standard library? Something like GenServer.call and GenServer.call!

Post #4

LostKobrakai

Can’t do that without breaking BC and the fact that the elixir API mirrors what :gen_server.call does.

Post #5

lud

You can search github for safe_call and you will find a buch of results. There may be lots of other names for that, for instance in the brod library it’s called safe_gen_call.

As a general rule of thumb, in a library that deals with processes, you may want to do that try/catch directly in library code if the process not being alive is frequent and desirable. And in all other cases do not wrap the call because when you are calling a process in general it should be there.

A common use case that comes to mind is dealing with transient processes, or timeout-then-stop, because calling Process.alive? before doing a call has an inherent race condition. So you just do the call, and if it exits with :noproc you start the process and try again.

So no you should not default to wrap everything, if the use case arises it’s generally pretty obvious.

Post #6

mickel8

Membrane Core Team

The example with brod library is great, thanks!

As a general rule of thumb, in a library that deals with processes, you may want to do that try/catch directly in library code if the process not being alive is frequent and desirable. And in all other cases do not wrap the call because when you are calling a process in general it should be there.

I can imagine that you have some kind of a client, which is a process and might die when it disconnects from a remote server. In such a case, wrapping calls to the client into try/catch sounds good.

But shouldn’t we be prepared for every process to crash at any time, e.g. because there is a bug in a code or something unexpected happened? Aren’t they reasons why we always try to spawn a process under supervisor?

Post #7

LostKobrakai

Yes we should expect processes to crash. How we react on a process not being unavailable is the point of discussion though. The option choosen for GenServer.call and (probably way) earlier for :gen_server.call is to exit if the process is not available. I’d imagine the reasoning being that the calling process requires a response to the call and if that cannot be retrieved all code depending on the response likely shouldn’t run as well. Choosing how to handle (such) errors is a tradeoff for sure, but I don’t think this one is entirely unreasonable.

The idea of supervisors and OTPs fault tolerance is not that things always have fallbacks (I’d even say to the contrary), but rather that failures only affect the users, which run into a fault, but not others. For this case this means the unlucky user trying to call a crashed process will crash as well, but any user not needing to interact with that missing process is fine. Once the process is back up the system is healed and everything works again.

Another thing you can look into is the async alternative to doing a call (send_request, wait_response, …) introduced in OTP 23, which doesn’t fail if a process is not alive, but returns an error tuple on trying to receive the response. That one is not yet in mirrored in elixir though, but the erlang api should work just fine.

Post #8

mat-hek

Membrane Core Team

It’s a bit like saying that if a process opens a file and the file doesn’t exist, all the code dependent on the file shouldn’t run. But it doesn’t mean that the process has to crash, thus we have File.open and File.open!

Post #9

LostKobrakai

That might be a very reasonable thing to do.

I’m not argueing that having that option wouldn’t be useful - it very much would be. I don’t see a way to get there now though. The decision for :gen_server.call had been made ages ago and GenServer.call mirrored it. I don’t think GenServer.call_but_not_exit is a great option as well. Unless this discussion is aimed at elixir 2.0 I’m not sure what this topic is meant to get to.

Post #10

Load more posts