From 589ca18914c8d14e7ceccb56dd86a8d068be298d Mon Sep 17 00:00:00 2001 From: sabiwara Date: Tue, 23 Apr 2024 21:38:53 +0200 Subject: [PATCH 1/6] Add GenServer.format_status/1 callback --- lib/elixir/lib/gen_server.ex | 34 ++++++++++++++++++++++++ lib/elixir/pages/references/typespecs.md | 2 +- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/lib/elixir/lib/gen_server.ex b/lib/elixir/lib/gen_server.ex index 8bb29f8be9f..fb95a0491b2 100644 --- a/lib/elixir/lib/gen_server.ex +++ b/lib/elixir/lib/gen_server.ex @@ -758,6 +758,38 @@ defmodule GenServer do | {:error, reason :: term} when old_vsn: term | {:down, term} + @doc """ + This function is called by a `GenServer` process in the following situations: + + * `sys:get_status/1,2` is invoked to get the `GenServer` status. + * The `GenServer` process terminates abnormally and logs an error. + + This callback is used to limit the status of the process returned by + `sys:get_status/1,2` or sent to logger. + + The callback gets a map `status` describing the current status and shall return + a map `new_status` with the same keys, but it may transform some values. + + Two possible use cases for this callback is to remove sensitive information + from the state to prevent it from being printed in log files, or to compact + large irrelevant status items that would only clutter the logs. + + ## Example + + @impl GenServer + def format_status(status) do + Map.new(status, fn + {:state, state} -> {:state, Map.delete(state, :private_key)} + {:message, {:password, _} -> {:message, {:password, "redacted"}} + key_value -> key_value + end) + end + + """ + @doc since: "1.17.0" + @callback format_status(status :: :gen_server.format_status()) :: + new_status :: :gen_server.format_status() + @doc """ Invoked in some cases to retrieve a formatted version of the `GenServer` status: @@ -775,6 +807,7 @@ defmodule GenServer do list of `{key, value}` tuples representing the current process dictionary of the `GenServer` and `state` is the current state of the `GenServer`. """ + @doc deprecated: "Use format_status/1 callback instead" @callback format_status(reason, pdict_and_state :: list) :: term when reason: :normal | :terminate @@ -783,6 +816,7 @@ defmodule GenServer do handle_info: 2, handle_cast: 2, handle_call: 3, + format_status: 1, format_status: 2, handle_continue: 2 diff --git a/lib/elixir/pages/references/typespecs.md b/lib/elixir/pages/references/typespecs.md index e79fc66bc8e..a34bb09f55c 100644 --- a/lib/elixir/pages/references/typespecs.md +++ b/lib/elixir/pages/references/typespecs.md @@ -371,7 +371,7 @@ Optional callbacks can be defined through the `@optional_callbacks` module attri @optional_callbacks non_vital_fun: 0, non_vital_macro: 1 end -One example of optional callback in Elixir's standard library is `c:GenServer.format_status/2`. +One example of optional callback in Elixir's standard library is `c:GenServer.format_status/1`. ### Inspecting behaviours From 1ae1ffea35c2eaa752463b7f3e5b885d80ae6712 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Tue, 23 Apr 2024 21:39:39 +0200 Subject: [PATCH 2/6] Remove or comment existing implementations --- lib/elixir/lib/dynamic_supervisor.ex | 9 --------- lib/elixir/lib/gen_event.ex | 2 ++ 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/lib/elixir/lib/dynamic_supervisor.ex b/lib/elixir/lib/dynamic_supervisor.ex index 52e3ba7c1d8..c74e5af3f60 100644 --- a/lib/elixir/lib/dynamic_supervisor.ex +++ b/lib/elixir/lib/dynamic_supervisor.ex @@ -1136,15 +1136,6 @@ defmodule DynamicSupervisor do ] end - @impl true - def format_status(:terminate, [_pdict, state]) do - state - end - - def format_status(_, [_pdict, %{mod: mod} = state]) do - [data: [{~c"State", state}], supervisor: [{~c"Callback", mod}]] - end - ## Helpers @compile {:inline, call: 2} diff --git a/lib/elixir/lib/gen_event.ex b/lib/elixir/lib/gen_event.ex index e2ea969e1b8..38e655f43c7 100644 --- a/lib/elixir/lib/gen_event.ex +++ b/lib/elixir/lib/gen_event.ex @@ -507,6 +507,8 @@ defmodule GenEvent do {:ok, states, [name, handlers, hib]} end + # Keeping deprecated format_status/2 since the current implementation is not + # compatible with format_status/1 and GenEvent is deprecated anyway @doc false def format_status(opt, status_data) do [pdict, sys_state, parent, _debug, [name, handlers, _hib]] = status_data From 00b109fbe211086a49a8bda65953dfa4f8d36ddd Mon Sep 17 00:00:00 2001 From: sabiwara Date: Wed, 24 Apr 2024 16:49:06 +0200 Subject: [PATCH 3/6] Remove test for obsolete behavior --- lib/elixir/test/elixir/dynamic_supervisor_test.exs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/elixir/test/elixir/dynamic_supervisor_test.exs b/lib/elixir/test/elixir/dynamic_supervisor_test.exs index 32cf398eb44..5437352c281 100644 --- a/lib/elixir/test/elixir/dynamic_supervisor_test.exs +++ b/lib/elixir/test/elixir/dynamic_supervisor_test.exs @@ -140,14 +140,6 @@ defmodule DynamicSupervisorTest do {:ok, pid} = DynamicSupervisor.start_link(strategy: :one_for_one) assert :proc_lib.initial_call(pid) == {:supervisor, Supervisor.Default, [:Argument__1]} end - - test "returns the callback module" do - {:ok, pid} = Supervisor.start_link([], strategy: :one_for_one) - assert :supervisor.get_callback_module(pid) == Supervisor.Default - - {:ok, pid} = DynamicSupervisor.start_link(strategy: :one_for_one) - assert :supervisor.get_callback_module(pid) == Supervisor.Default - end end ## Code change From 0e9bd2c75f381bccc45d07794c13551d94e2f177 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Wed, 24 Apr 2024 16:49:35 +0200 Subject: [PATCH 4/6] Remove doc for deprecated callback --- lib/elixir/lib/gen_server.ex | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/lib/elixir/lib/gen_server.ex b/lib/elixir/lib/gen_server.ex index fb95a0491b2..a3cf71c6da9 100644 --- a/lib/elixir/lib/gen_server.ex +++ b/lib/elixir/lib/gen_server.ex @@ -790,23 +790,6 @@ defmodule GenServer do @callback format_status(status :: :gen_server.format_status()) :: new_status :: :gen_server.format_status() - @doc """ - Invoked in some cases to retrieve a formatted version of the `GenServer` status: - - * one of `:sys.get_status/1` or `:sys.get_status/2` is invoked to get the - status of the `GenServer`; in such cases, `reason` is `:normal` - - * the `GenServer` terminates abnormally and logs an error; in such cases, - `reason` is `:terminate` - - This callback can be useful to control the *appearance* of the status of the - `GenServer`. For example, it can be used to return a compact representation of - the `GenServer`'s state to avoid having large state terms printed. - - `pdict_and_state` is a two-elements list `[pdict, state]` where `pdict` is a - list of `{key, value}` tuples representing the current process dictionary of - the `GenServer` and `state` is the current state of the `GenServer`. - """ @doc deprecated: "Use format_status/1 callback instead" @callback format_status(reason, pdict_and_state :: list) :: term when reason: :normal | :terminate From 43cf9af6a75bec5e175e42086b8725a7ce35159b Mon Sep 17 00:00:00 2001 From: sabiwara Date: Wed, 24 Apr 2024 16:52:19 +0200 Subject: [PATCH 5/6] Fix docs --- lib/elixir/lib/gen_server.ex | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/elixir/lib/gen_server.ex b/lib/elixir/lib/gen_server.ex index a3cf71c6da9..ce8f72203ff 100644 --- a/lib/elixir/lib/gen_server.ex +++ b/lib/elixir/lib/gen_server.ex @@ -761,11 +761,11 @@ defmodule GenServer do @doc """ This function is called by a `GenServer` process in the following situations: - * `sys:get_status/1,2` is invoked to get the `GenServer` status. + * [`:sys.get_status/1,2`](`:sys.get_status/1`) is invoked to get the `GenServer` status. * The `GenServer` process terminates abnormally and logs an error. This callback is used to limit the status of the process returned by - `sys:get_status/1,2` or sent to logger. + [`:sys.get_status/1,2`](`:sys.get_status/1`) or sent to logger. The callback gets a map `status` describing the current status and shall return a map `new_status` with the same keys, but it may transform some values. @@ -780,7 +780,7 @@ defmodule GenServer do def format_status(status) do Map.new(status, fn {:state, state} -> {:state, Map.delete(state, :private_key)} - {:message, {:password, _} -> {:message, {:password, "redacted"}} + {:message, {:password, _}} -> {:message, {:password, "redacted"}} key_value -> key_value end) end From d496b3da7cf5228c39b0086bffe210ddde6b19eb Mon Sep 17 00:00:00 2001 From: sabiwara Date: Wed, 24 Apr 2024 17:08:00 +0200 Subject: [PATCH 6/6] Add TODO --- lib/elixir/lib/gen_server.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/elixir/lib/gen_server.ex b/lib/elixir/lib/gen_server.ex index ce8f72203ff..7f8a4be1186 100644 --- a/lib/elixir/lib/gen_server.ex +++ b/lib/elixir/lib/gen_server.ex @@ -790,6 +790,7 @@ defmodule GenServer do @callback format_status(status :: :gen_server.format_status()) :: new_status :: :gen_server.format_status() + # TODO: Remove this on v2.0 @doc deprecated: "Use format_status/1 callback instead" @callback format_status(reason, pdict_and_state :: list) :: term when reason: :normal | :terminate