gen_statem badrecord,trans_opts

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

gen_statem badrecord,trans_opts

getonga
Thanks, I recheck my source code, and I found the error reason. Thanks for your time.

_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: gen_statem badrecord,trans_opts

Torben Hoffmann
Hi,
Would it be possible to add some text to the gen_statem docs that this message usually means you have messed up one of the actions?
It wasn't obvious to me before I googled it and then it was a matter of chasing down all my actions.
Somehow I find the error messages of gen_statem harder to work with than gen_fsm, which is a pity since gen_statem is so much easier to work with in terms of design flexibility and easy to raed code.

Cheers,
Torben


On Tue, May 15, 2018 at 8:23 AM getonga <[hidden email]> wrote:
Thanks, I recheck my source code, and I found the error reason. Thanks for your time.
_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
--

_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: gen_statem badrecord,trans_opts

Raimo Niskanen-2
On Tue, May 15, 2018 at 10:59:50PM +0200, Torben Hoffmann wrote:

> Hi,
> Would it be possible to add some text to the gen_statem docs that this
> message usually means you have messed up one of the actions?
> It wasn't obvious to me before I googled it and then it was a matter of
> chasing down all my actions.
> Somehow I find the error messages of gen_statem harder to work with than
> gen_fsm, which is a pity since gen_statem is so much easier to work with in
> terms of design flexibility and easy to raed code.
>
> Cheers,
> Torben

The message in this case:
  Context:    child_terminated Reason:
    {{badrecord,trans_opts},
     [{gen_statem,loop_event_actions_list,10,
       [{file,"gen_statem.erl"},{line,1210}]},
      {proc_lib,init_p_do_apply,3,
       [{file,"proc_lib.erl"},{line,247}]}]}

I classify this as a bug, but not severe enough to merit a patch on the 20.3
release.  What the code was trying to do but crashed instead was to
terminate with error:{bad_action_from_state_function,Action} in a regular
Error Report.

The intention is that gen_statem should pinpoint bad actions fairly
precisely.  That crash is fixed in 20.0-rc1 and later so it will terminate
as intended.

I agree that the OP:s crash is not a good error message for end users, but
it was a good one for me since it was precise for me to fix the crash.

Do you have any suggestions about improving correct error messages from
gen_statem, or can you exemplify some that you find hard to understand?
In comparison with gen_fsm, if you like.

Best Regards
/ Raimo


>
>
> On Tue, May 15, 2018 at 8:23 AM getonga <[hidden email]> wrote:
>
> > Thanks, I recheck my source code, and I found the error reason. Thanks for
> > your time.
> > _______________________________________________
> > erlang-questions mailing list
> > [hidden email]
> > http://erlang.org/mailman/listinfo/erlang-questions
> >
> --
> https://www.linkedin.com/in/torbenhoffmann/

> _______________________________________________
> erlang-questions mailing list
> [hidden email]
> http://erlang.org/mailman/listinfo/erlang-questions


--

/ Raimo Niskanen, Erlang/OTP, Ericsson AB
_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: gen_statem badrecord,trans_opts

Torben Hoffmann
Hi Raimo,

I'm looking forward to that particular error being more understandable in a future release. It is too small to patch for.

The other day I was trying to capture a somewhat elaborate state machine in a gen_statem and I hadn't read the manual well enough, so I thought that I could do some nifty checking in the enter clause and decide if another state was more suited to do the job. But with state_enter you cannot change state in the enter clause. It is clearly written din the manual, but... you know... blush.

However, the error message doesn't point you to this fundamental rule:

** Last event = {internal,init_state}
** When server state  = {starting,happy}
** Reason for termination = error:{bad_return_from_state_function,
                                      {next_state,creating,happy}}
** Callback mode = [state_functions,state_enter]


It would be nice if it said something about not changing state when entering.

Same kind of issue when adding next_event in a state enter:

** Reason for termination = error:{bad_action_from_state_
function,
                              
        {next_event,internal,jump}}

Maybe the error should be {dont_add_next_event_in_enter_read_the_manual_dude, ...} or something to that effect!

And when you misspell next_state you get this:

** Reason for termination = error:{bad_return_from_state_function,
                                   {next_stete,running,happy,
                                    {next_event,internal,jump}}}


It would be nice if it said what it could not like.

I have not looked in the gen_statem code, but I am pretty sure that the checks would allow more details in the error messages. The information must be at hand when it errors out.

Cheers,
Torben


On Wed, May 16, 2018 at 8:52 AM Raimo Niskanen <[hidden email]> wrote:
On Tue, May 15, 2018 at 10:59:50PM +0200, Torben Hoffmann wrote:
> Hi,
> Would it be possible to add some text to the gen_statem docs that this
> message usually means you have messed up one of the actions?
> It wasn't obvious to me before I googled it and then it was a matter of
> chasing down all my actions.
> Somehow I find the error messages of gen_statem harder to work with than
> gen_fsm, which is a pity since gen_statem is so much easier to work with in
> terms of design flexibility and easy to raed code.
>
> Cheers,
> Torben

The message in this case:
  Context:    child_terminated Reason:
    {{badrecord,trans_opts},
     [{gen_statem,loop_event_actions_list,10,
       [{file,"gen_statem.erl"},{line,1210}]},
      {proc_lib,init_p_do_apply,3,
       [{file,"proc_lib.erl"},{line,247}]}]}

I classify this as a bug, but not severe enough to merit a patch on the 20.3
release.  What the code was trying to do but crashed instead was to
terminate with error:{bad_action_from_state_function,Action} in a regular
Error Report.

The intention is that gen_statem should pinpoint bad actions fairly
precisely.  That crash is fixed in 20.0-rc1 and later so it will terminate
as intended.

I agree that the OP:s crash is not a good error message for end users, but
it was a good one for me since it was precise for me to fix the crash.

Do you have any suggestions about improving correct error messages from
gen_statem, or can you exemplify some that you find hard to understand?
In comparison with gen_fsm, if you like.

Best Regards
/ Raimo


>
>
> On Tue, May 15, 2018 at 8:23 AM getonga <[hidden email]> wrote:
>
> > Thanks, I recheck my source code, and I found the error reason. Thanks for
> > your time.
> > _______________________________________________
> > erlang-questions mailing list
> > [hidden email]
> > http://erlang.org/mailman/listinfo/erlang-questions
> >
> --
> https://www.linkedin.com/in/torbenhoffmann/

> _______________________________________________
> erlang-questions mailing list
> [hidden email]
> http://erlang.org/mailman/listinfo/erlang-questions


--

/ Raimo Niskanen, Erlang/OTP, Ericsson AB
_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
--

_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: gen_statem badrecord,trans_opts

Raimo Niskanen-2
On Wed, May 16, 2018 at 10:39:36AM +0200, Torben Hoffmann wrote:

> Hi Raimo,
>
> I'm looking forward to that particular error being more understandable in a
> future release. It is too small to patch for.
>
> The other day I was trying to capture a somewhat elaborate state machine in
> a gen_statem and I hadn't read the manual well enough, so I thought that I
> could do some nifty checking in the enter clause and decide if another
> state was more suited to do the job. But with state_enter you cannot change
> state in the enter clause. It is clearly written din the manual, but... you
> know... blush.
>
> However, the error message doesn't point you to this fundamental rule:
>
> ** Last event = {internal,init_state}
> ** When server state  = {starting,happy}
> ** Reason for termination = error:{bad_return_from_state_function,
>                                       {next_state,creating,happy}}
> ** Callback mode = [state_functions,state_enter]
>
> It would be nice if it said something about not changing state when
> entering.

That one was changed to
  error:{bad_state_enter_return_from_state_function,
         {next_state,creating,happy}}
in commit ea1963553ffb06eb1bda636f328718f91136ed9c on 'master'

I hope that is not too subtle - I wanted to stick to the format.
At least it indicates state_enter and state function.


>
> Same kind of issue when adding next_event in a state enter:
>
> ** Reason for termination = error:{bad_action_from_state_function,
>                                       {next_event,internal,jump}}
>
> Maybe the error should be
> {dont_add_next_event_in_enter_read_the_manual_dude, ...} or something to
> that effect!
>

That one was changed to
  error:{bad_state_enter_action_from_state_function,
         {next_event,internal,jump}}
in the same commit.


> And when you misspell next_state you get this:
>
> ** Reason for termination = error:{bad_return_from_state_function,
>                                    {next_stete,running,happy,
>                                     {next_event,internal,jump}}}
>
> It would be nice if it said what it could not like.

In this case you return a tuple with a tag that is not recognized, and it
would take some artificial intelligence to figure out that it most likely
should be next_state that the user attempted.

The tuple does not match any of the allowed patterns, simply, so it is
a bad return value.  Difficult to do anything smarter.


Best regards
/ Raimo


>
> I have not looked in the gen_statem code, but I am pretty sure that the
> checks would allow more details in the error messages. The information must
> be at hand when it errors out.
>
> Cheers,
> Torben
>
>
> On Wed, May 16, 2018 at 8:52 AM Raimo Niskanen <
> [hidden email]> wrote:
>
> > On Tue, May 15, 2018 at 10:59:50PM +0200, Torben Hoffmann wrote:
> > > Hi,
> > > Would it be possible to add some text to the gen_statem docs that this
> > > message usually means you have messed up one of the actions?
> > > It wasn't obvious to me before I googled it and then it was a matter of
> > > chasing down all my actions.
> > > Somehow I find the error messages of gen_statem harder to work with than
> > > gen_fsm, which is a pity since gen_statem is so much easier to work with
> > in
> > > terms of design flexibility and easy to raed code.
> > >
> > > Cheers,
> > > Torben
> >
> > The message in this case:
> >   Context:    child_terminated Reason:
> >     {{badrecord,trans_opts},
> >      [{gen_statem,loop_event_actions_list,10,
> >        [{file,"gen_statem.erl"},{line,1210}]},
> >       {proc_lib,init_p_do_apply,3,
> >        [{file,"proc_lib.erl"},{line,247}]}]}
> >
> > I classify this as a bug, but not severe enough to merit a patch on the
> > 20.3
> > release.  What the code was trying to do but crashed instead was to
> > terminate with error:{bad_action_from_state_function,Action} in a regular
> > Error Report.
> >
> > The intention is that gen_statem should pinpoint bad actions fairly
> > precisely.  That crash is fixed in 20.0-rc1 and later so it will terminate
> > as intended.
> >
> > I agree that the OP:s crash is not a good error message for end users, but
> > it was a good one for me since it was precise for me to fix the crash.
> >
> > Do you have any suggestions about improving correct error messages from
> > gen_statem, or can you exemplify some that you find hard to understand?
> > In comparison with gen_fsm, if you like.
> >
> > Best Regards
> > / Raimo
> >
> >
> > >
> > >
> > > On Tue, May 15, 2018 at 8:23 AM getonga <[hidden email]> wrote:
> > >
> > > > Thanks, I recheck my source code, and I found the error reason. Thanks
> > for
> > > > your time.
> > > --
> > > https://www.linkedin.com/in/torbenhoffmann/
> >
> > --
> >
> > / Raimo Niskanen, Erlang/OTP, Ericsson AB
> --
> https://www.linkedin.com/in/torbenhoffmann/


--

/ Raimo Niskanen, Erlang/OTP, Ericsson AB
_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
Reply | Threaded
Open this post in threaded view
|

Re: gen_statem badrecord,trans_opts

Torben Hoffmann
Those improvements are really good.

I can live without AI in the OTP libs ;-)

Cheers,
Torben

On Wed, May 16, 2018 at 12:13 PM Raimo Niskanen <[hidden email]> wrote:
On Wed, May 16, 2018 at 10:39:36AM +0200, Torben Hoffmann wrote:
> Hi Raimo,
>
> I'm looking forward to that particular error being more understandable in a
> future release. It is too small to patch for.
>
> The other day I was trying to capture a somewhat elaborate state machine in
> a gen_statem and I hadn't read the manual well enough, so I thought that I
> could do some nifty checking in the enter clause and decide if another
> state was more suited to do the job. But with state_enter you cannot change
> state in the enter clause. It is clearly written din the manual, but... you
> know... blush.
>
> However, the error message doesn't point you to this fundamental rule:
>
> ** Last event = {internal,init_state}
> ** When server state  = {starting,happy}
> ** Reason for termination = error:{bad_return_from_state_function,
>                                       {next_state,creating,happy}}
> ** Callback mode = [state_functions,state_enter]
>
> It would be nice if it said something about not changing state when
> entering.

That one was changed to
  error:{bad_state_enter_return_from_state_function,
         {next_state,creating,happy}}
in commit ea1963553ffb06eb1bda636f328718f91136ed9c on 'master'

I hope that is not too subtle - I wanted to stick to the format.
At least it indicates state_enter and state function.


>
> Same kind of issue when adding next_event in a state enter:
>
> ** Reason for termination = error:{bad_action_from_state_function,
>                                       {next_event,internal,jump}}
>
> Maybe the error should be
> {dont_add_next_event_in_enter_read_the_manual_dude, ...} or something to
> that effect!
>

That one was changed to
  error:{bad_state_enter_action_from_state_function,
         {next_event,internal,jump}}
in the same commit.


> And when you misspell next_state you get this:
>
> ** Reason for termination = error:{bad_return_from_state_function,
>                                    {next_stete,running,happy,
>                                     {next_event,internal,jump}}}
>
> It would be nice if it said what it could not like.

In this case you return a tuple with a tag that is not recognized, and it
would take some artificial intelligence to figure out that it most likely
should be next_state that the user attempted.

The tuple does not match any of the allowed patterns, simply, so it is
a bad return value.  Difficult to do anything smarter.


Best regards
/ Raimo


>
> I have not looked in the gen_statem code, but I am pretty sure that the
> checks would allow more details in the error messages. The information must
> be at hand when it errors out.
>
> Cheers,
> Torben
>
>
> On Wed, May 16, 2018 at 8:52 AM Raimo Niskanen <
> [hidden email]> wrote:
>
> > On Tue, May 15, 2018 at 10:59:50PM +0200, Torben Hoffmann wrote:
> > > Hi,
> > > Would it be possible to add some text to the gen_statem docs that this
> > > message usually means you have messed up one of the actions?
> > > It wasn't obvious to me before I googled it and then it was a matter of
> > > chasing down all my actions.
> > > Somehow I find the error messages of gen_statem harder to work with than
> > > gen_fsm, which is a pity since gen_statem is so much easier to work with
> > in
> > > terms of design flexibility and easy to raed code.
> > >
> > > Cheers,
> > > Torben
> >
> > The message in this case:
> >   Context:    child_terminated Reason:
> >     {{badrecord,trans_opts},
> >      [{gen_statem,loop_event_actions_list,10,
> >        [{file,"gen_statem.erl"},{line,1210}]},
> >       {proc_lib,init_p_do_apply,3,
> >        [{file,"proc_lib.erl"},{line,247}]}]}
> >
> > I classify this as a bug, but not severe enough to merit a patch on the
> > 20.3
> > release.  What the code was trying to do but crashed instead was to
> > terminate with error:{bad_action_from_state_function,Action} in a regular
> > Error Report.
> >
> > The intention is that gen_statem should pinpoint bad actions fairly
> > precisely.  That crash is fixed in 20.0-rc1 and later so it will terminate
> > as intended.
> >
> > I agree that the OP:s crash is not a good error message for end users, but
> > it was a good one for me since it was precise for me to fix the crash.
> >
> > Do you have any suggestions about improving correct error messages from
> > gen_statem, or can you exemplify some that you find hard to understand?
> > In comparison with gen_fsm, if you like.
> >
> > Best Regards
> > / Raimo
> >
> >
> > >
> > >
> > > On Tue, May 15, 2018 at 8:23 AM getonga <[hidden email]> wrote:
> > >
> > > > Thanks, I recheck my source code, and I found the error reason. Thanks
> > for
> > > > your time.
> > > --
> > > https://www.linkedin.com/in/torbenhoffmann/
> >
> > --
> >
> > / Raimo Niskanen, Erlang/OTP, Ericsson AB
> --
> https://www.linkedin.com/in/torbenhoffmann/


--

/ Raimo Niskanen, Erlang/OTP, Ericsson AB
_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions
--

_______________________________________________
erlang-questions mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-questions