Fix logger crash when :gen_statem format_status/2 returns non-tuple #13684
Conversation
sabiwara
force-pushed
the
fix_gen_statem_logger_crash
branch
from
902068e to
a7c7aeb
Compare
sabiwara
changed the title
sabiwara
force-pushed
the
fix_gen_statem_logger_crash
branch
2 times, most recently
from
5ac3e7b to
9efe6b8
Compare
sabiwara
changed the title
lib/logger/lib/logger/translator.ex
Outdated
| @@ -324,12 +324,19 @@ defmodule Logger.Translator do | |||
| ["\nPostponed: #{inspect(postponed, inspect_opts)}"] | |||
|
|
|||
| if min_level == :debug do | |||
| state_info = | |||
| case state do | |||
| {state, data} -> | |||
There was a problem hiding this comment.
Couldn't this be a user defined tuple then? That we think is State+Data, but it is just something else? How does Erlang/OTP report those? Is there another key we could look at instead?
There was a problem hiding this comment.
Good call, I should really always ask myself "what would Erlang do?" 😄
Indeed, it seems there is no such thing as State-Data unpacking, just State: https://github.com/erlang/otp/blob/OTP-27.0/lib/stdlib/src/gen_statem.erl#L4554-L4616. Will fix accordingly.
sabiwara
force-pushed
the
fix_gen_statem_logger_crash
branch
from
9efe6b8 to
dadb797
Compare
| @@ -552,6 +570,34 @@ defmodule Logger.TranslatorTest do | |||
| assert_receive {:event, {:string, ["Process " | _]}, _process_metadata} | |||
| end | |||
|
|
|||
| test "translates :gen_statem crashes when format_status/2 does not return a tuple" do | |||
There was a problem hiding this comment.
Should we keep this test just in case to prevent similar regressions?
It feels like a bit artificial now that there is no code assuming a tuple in the first place.
No strong opinion.
There was a problem hiding this comment.
Sorry, to be clear, we should have tests for both styles of gen_statem, but perhaps we should change its name.
| Queue: .* | ||
| Postponed: \[\] | ||
| State: :ok | ||
| Callback mode: .*, state_enter: false |
There was a problem hiding this comment.
Note: I can't check for callback mode because of erlang/otp#8605
Closes #13682
Will backport after merge.