Proposal: Introduce `use GenServer, strict: true`

[Alexei Sholik]

Hi,

We all love the ability to `use GenServer` in order to provide default stubs for callback functions, saving us time and effort of writing the same boilerplate over and over again.

However, I'm sure many of you have also been bitten by this very feature where you made a typo in the signature of init/1 or handle_info/2 and was left scratching your head wondering why the gen server's state looks so weird or whether it receives any messages sent to it.

I'd like to propose an addition that will make the default stubs stricter in terms of handling unexpected input. The idea is basically adding the `strict` option to the `use GenServer` call. When that option is supplied with value `true`, the following changes will take effect:

  1. There will be no default implementation of init/1. If the user forgets to implement it or implements it incorrectly by supplying a different number of arguments, the compiler will warn about the missing init/1 implementation.

  2. The default implementation of handle_info/1 will exit on any incoming message, in the same way handle_cast/2 and handle_call/3 already do.

The change will be backwards-compatible if we make `strict: false` the default. However, we should mention in the docs that calling `use GenServer, strict: true` is the recommended way for any serious purpose and that `use GenServer` should be reserved for playground or throw-away code.

Here's a proof of concept – https://github.com/elixir-lang/elixir/compare/master...alco:alco/strict-gen-server

Best regards,

Alex

[OvermindDL1]

There is a library that simplifies the process of making GenServers:  https://hex.pm/packages/exactor

It in fact has such a 'strict' mode similar to this, for showing prior design.

You could also do it yourself as a user, don't use GenServer, instead 'implement' GenServer, I.E.

```elixir

@behaviour GenServer

```

Then at least Dialyzer will yell at you if not everything is implemented.

I, personally, would like a strict mode, however I'm unsure of just how it should be done.  Your changes are good but I'd probably also add an after_compile hook to walk the module AST (not hard to do, but it is erlang AST at that point, not elixir) and verify the functions exist in addition to your changes.  Make a `defmodule_GenServer` macro would be easier though, and that could be done easily as a library.

[José Valim]

  2. The default implementation of handle_info/1 will exit on any incoming message, in the same way handle_cast/2 and handle_call/3 already do.

I believe this is not a good default because your processes may receive regular messages from other processes and you don't want to crash because of them. This is much more unlikely to happen with cast and call though, so we can afford to raise in cast and calls.

I am wondering if the best way to solve this problem would be to have a @before_compile callback that checks if you defined any of the callbacks with a different arity while, at the same time, did not define one with the proper arity.

[Paul Schoenfelder]

I am wondering if the best way to solve this problem would be to have a @before_compile callback that checks if you defined any of the callbacks with a different arity while, at the same time, did not define one with the proper arity.

This seems like a good compromise to me - it prevents common mistakes, but doesn't require adding options to GenServer. In my opinion, imlementing the :gen_server behaviour directly is how one rolls "strict" mode; adding `strict: true` as an option (or even just adding options to `use GenServer` to begin with) is too likely to cause confusion about what callbacks are implemented by default, and how they behave.

Paul

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-core+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4JqC6rzBUYJHd9ij%3DQRH9-mPNDG1qGk1hTMZ6r331iZKA%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

[Dmitry Belyaev]

It won't help if a function name is misspelled.

[Peter Hamilton]

The other route to go is to add source generators. This is pretty common in Erlang to have a template that gets copied whenever you make a new gen_server.

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.

To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/A32068A7-2B9E-492C-AFD7-B311A70B06EE%40gmail.com.

[James Fish]

When I saw this thread I thought we should be logging in the default `handle_info/2` implementation, regardless of the changes proposed here, because we are silently ignoring an unhandled message. This could certainly hide bugs as described by Alexei, we got this one wrong and need to change the default behaviour. However if we are logging `handle_info/2` should we avoid crashing and do similarly for `handle_call/3` and `handle_cast/2`?

For `handle_call/3`, if we log and reply then the caller may handle the error and if we log and noreply then the caller blocks for the timeout, which could be infinity. Therefore even though we are logging a message it requires handling by a human. A well designed supervision tree (and a poorly designed one can too) will limit the fault to the minimal error kernel and recover to a (hopefully) good state as soon as possible after a crash. In many cases the error will happen either nearly all the time and be caught early in the development cycle or will happen very rarely and can be fixed when priorities allow. This could be somewhere from immediately to never such is the beauty of OTP's "let it crash" philosophy.

With this in mind it makes sense to crash on unexpected calls/casts/other messages so that we can use the supervision tree to recover the system to a good state. These can occur for three reasons:

1) The message is intended for the GenServer but the server is in a bad state

2) The message is intended for the GenServer but the sender is in a bad state

2) The message is not intended for the GenServer and the sender is in a bad state

We can not recover from a bad state to a good state by logging alone.

Currently `handle_call/3` and `handle_cast/2` have different behaviour to `handle_info/2` because calls and cast should only be triggered by function calls to the callback module from client processes. This means an unhandled call or cast is definitely bad state somewhere. Whereas `handle_info/2` is more likely to be a result of a function call made by the GenServer. Common examples would starting async tasks, monitors, links, ports and sockets. In many situations some messages can be safely ignored, such as for async tasks where the :DOWN can be ignored after receiving the result message. Therefore it is a common pattern to have a catch all clause as the last function clause for `handle_info/2` but not `handle_call/3` and `handle_cast/2`.

When struggling with these problems I often look at the OTP source to see how it is handled there. The supervisor is the corner stone of OTP and it logs unexpected messages: https://github.com/erlang/otp/blob/41d1444bb13317f78fdf600a7102ff9f7cb12d13/lib/stdlib/src/supervisor.erl#L630. However a supervisor can run user code, in the `init/1` or `start` function. Both of these can be called and have exceptions caught and the supervisor will continue to run. These cause unhandled messages, one example would be catching a `GenServer.call/3` timeout, which can lead to an unexpected response message arriving later. This type of handling is not normal though.

If `handle_info/2` is not implemented then miscellaneous messages are not expected and perhaps should crash in the default implementation. Clearly something has gone wrong and there is a bug. When there is a bug the default behaviour in OTP is to crash and allow the supervision tree to do its work. Once the bug is understood then explicit handling is added, such as logging the unhandled message. I think we should crash in default implementation `handle_info/2` as with `handle_call/3` and `handle_cast/2`.

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.

To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-core+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4JqC6rzBUYJHd9ij%3DQRH9-mPNDG1qGk1hTMZ6r331iZKA%40mail.gmail.com.

[James Fish]

Going from silently ignoring to crashing might be too much of a step. It would certainly be possible for there to be unhandled messages people don't know about. Should we log for at least the next release cycle?

[José Valim]

Yes, let's please add logging to handle_info throughout Elixir (including GenStage). :)

José Valim

www.plataformatec.com.br

Skype: jv.ptec

Founder and Director of R&D

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CA%2BibZ98WrNhF0Xo_mbVn-ST2b9ysqryOCPoLWNf-jM7baisH2g%40mail.gmail.com.

[Louis Pop]

Hi

Will there be a way to opt out of logging and noop-ing? I would prefer
processes to crash. :)

Cheers,
Louis

On 29 September 2016 at 09:58, José Valim

>>>> an email to elixir-lang-co...@googlegroups.com.

>>>> To view this discussion on the web visit
>>>> https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4JqC6rzBUYJHd9ij%3DQRH9-mPNDG1qGk1hTMZ6r331iZKA%40mail.gmail.com.
>>>>
>>>> For more options, visit https://groups.google.com/d/optout.
>>>
>>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "elixir-lang-core" group.
>> To unsubscribe from this group and stop receiving emails from it, send an

>> email to elixir-lang-co...@googlegroups.com.

>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/elixir-lang-core/CA%2BibZ98WrNhF0Xo_mbVn-ST2b9ysqryOCPoLWNf-jM7baisH2g%40mail.gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>
>

> --
> You received this message because you are subscribed to the Google Groups
> "elixir-lang-core" group.
> To unsubscribe from this group and stop receiving emails from it, send an

> email to elixir-lang-co...@googlegroups.com.

> To view this discussion on the web visit

> https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4Kj2j7q9CFjdZ_E0q8cWPxN5i4xu3NSBa%3DYgwUZRWoX7w%40mail.gmail.com.

[Michał Muskała]

> On 30 Sep 2016, at 11:39, Louis Pop <louisp...@gmail.com> wrote:
>
> Hi
>
> Will there be a way to opt out of logging and noop-ing? I would prefer
> processes to crash. :)
>
> Cheers,
> Louis

You can always override the default definition.

Michał.