Confusing crash report when gen_statem:start_link is called with invalid arguments

[rlipscombe]

Describe the bug

I wrote the following:

start_link() -> gen_statem:start_link({local, ?MODULE}, ?MODULE, []).

(I forgot about the options parameter). This called gen_statem:start_link/3. Thus far: my fault.

The problem is that the error message is just ... not good:

2023-09-26T16:25:12.827154+01:00 error: crasher: initial call: my_gen_statem:init/1, pid: <0.182.0>, registered_name: [], error: {badarg,[{erlang,function_exported,[{local,my_gen_statem},format_status,1],[{error_info,#{module => erl_erts_errors}}]},{gen,format_status,4,[{file,"gen.erl"},{line,764}]},{gen_statem,error_info,7,[{file,"gen_statem.erl"},{line,2602}]},{gen_statem,init_it,6,[{file,"gen_statem.erl"},{line,978}]},{proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,241}]}]}, ancestors: [myapp_sup,<0.130.0>], message_queue_len: 0, messages: [], links: [<0.181.0>], dictionary: [], trap_exit: false, status: running, heap_size: 610, stack_size: 28, reductions: 208; neighbours:
2023-09-26T16:25:12.827327+01:00 error: Supervisor: {local,myapp_sup}. Context: start_error. Reason: {badarg,[{erlang,function_exported,[{local,my_gen_statem},format_status,1],[{error_info,#{module => erl_erts_errors}}]},{gen,format_status,4,[{file,"gen.erl"},{line,764}]},{gen_statem,error_info,7,[{file,"gen_statem.erl"},{line,2602}]},{gen_statem,init_it,6,[{file,"gen_statem.erl"},{line,978}]},{proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,241}]}]}. Offender: id=my_statem,pid=undefined.
2023-09-26T16:25:12.827414+01:00 error: crasher: initial call: application_master:init/4, pid: <0.129.0>, registered_name: [], exit: {{{shutdown,{failed_to_start_child,my_statem,{badarg,[{erlang,function_exported,[{local,my_gen_statem},format_status,1],[{error_info,#{module => erl_erts_errors}}]},{gen,format_status,4,[{file,"gen.erl"},{line,764}]},{gen_statem,error_info,7,[{file,"gen_statem.erl"},{line,2602}]},{gen_statem,init_it,6,[{file,"gen_statem.erl"},{line,978}]},{proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,241}]}]}}},{myapp_app,start,[normal,[]]}},[{application_master,init,4,[{file,"application_master.erl"},{line,142}]},{proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,241}]}]}, ancestors: [<0.128.0>], message_queue_len: 1, messages: [{'EXIT',<0.130.0>,normal}], links: [<0.128.0>,<0.44.0>], dictionary: [], trap_exit: true, status: running, heap_size: 610, stack_size: 28, reductions: 183; neighbours:
2023-09-26T16:25:12.827588+01:00 notice: Application: myapp. Exited: {{shutdown,{failed_to_start_child,my_statem,{badarg,[{erlang,function_exported,[{local,my_gen_statem},format_status,1],[{error_info,#{module => erl_erts_errors}}]},{gen,format_status,4,[{file,"gen.erl"},{line,764}]},{gen_statem,error_info,7,[{file,"gen_statem.erl"},{line,2602}]},{gen_statem,init_it,6,[{file,"gen_statem.erl"},{line,978}]},{proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,241}]}]}}},{myapp_app,start,[normal,[]]}}. Type: temporary.

Rather than something actionable, such as a function_clause error, the error talks about a missing format_status function. This could lead someone to try to implement one, rather than to realise that they've merely called the wrong gen_statem:start_link() function.

To Reproduce

See above.

Expected behavior

An error message that clearly points to the misuse of gen_statem:start_link.

Affected versions

OTP-26.1

[RaimoNiskanen]

I didn't see that one coming. Interesting...

Most API functions in gen_statem (and gen_server) simply pass on the arguments to the corresponding internal gen function, and this particular argument gets as far as to being used when formatting the error that there was no init/1 function in the bogus module {local,?MODULE}. So the error formatting crashes when trying to check if bogus module has got a formatting function...

I guess it would be reasonable to check the Module argument in all API functions, at least the start functions, so we do not get a bogus module over the doorstep. Then we would get a function clause when calling the API function with a bogus module.
If Module should be an atom but not a module, calling the module local formatting function would be skipped instead of causing an internal crash.

gen_server probably does not suffer from this particular quirk, since it does not try to do any formatting on the error when there is no Module:init/1 function. It just exits.

[rlipscombe]

So the error formatting crashes when trying to check if bogus module has got a formatting function...

Would it be acceptable to check that it's not bogus at this point? Would pretending it doesn't have a formatting function give a reasonable result?

I ask because I'm uneasy about the compat implications of adding guards to a public, well-used API. Of course, validating the types as early as possible is more intellectually pure.

[RaimoNiskanen]

It would work to check that Module is an atom() before calling erlang:function_exported/3 - formatting would be done without trying the callback Modules formatting function. But it is annoying to have an incorrect value type in Module to begin with. Who knows where it will blow up next, after some innocent rewrite in the future.

Since added guards in this case would not be stricter than the -spec for the function, it is likely that Dialyzer already would have caught this problem. And, early error detection in general facilitates fault analysis...

[RaimoNiskanen]

Fixed on 'maint' in 780c5b3 to be released in OTP-26.2