-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add GenServer.format_status/1 callback #13511
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
589ca18
1ae1ffe
00b109f
0e9bd2c
43cf9af
d496b3d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -759,22 +759,38 @@ defmodule GenServer do | |
when old_vsn: term | {:down, term} | ||
|
||
@doc """ | ||
Invoked in some cases to retrieve a formatted version of the `GenServer` status: | ||
This function is called by a `GenServer` process in the following situations: | ||
|
||
* 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` | ||
* [`: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. | ||
|
||
* the `GenServer` terminates abnormally and logs an error; in such cases, | ||
`reason` is `:terminate` | ||
This callback is used to limit the status of the process returned by | ||
[`:sys.get_status/1,2`](`:sys.get_status/1`) or sent to logger. | ||
|
||
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. | ||
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 | ||
|
||
`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 since: "1.17.0" | ||
@callback format_status(status :: :gen_server.format_status()) :: | ||
new_status :: :gen_server.format_status() | ||
|
||
@doc deprecated: "Use format_status/1 callback instead" | ||
@callback format_status(reason, pdict_and_state :: list) :: term | ||
when reason: :normal | :terminate | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should still document it. In any case, let's also add a TODO to remove it on 2.0? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, let's remove the docs completely. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's add the TODO and ship it! |
||
|
||
|
@@ -783,6 +799,7 @@ defmodule GenServer do | |
handle_info: 2, | ||
handle_cast: 2, | ||
handle_call: 3, | ||
format_status: 1, | ||
format_status: 2, | ||
handle_continue: 2 | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can reproduce this behavior with
format_status/1
, and this breaks a test - should I just keep this one around for now?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we can just remove these tests, or implement them to use
:mod
?If
:supervisor.get_callback_module
is used somewhere which matters, should erlang add a key to theformat_status
map?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sabiwara we should mirror whatever the Erlang
supervisor
is doing. This was most likely just borrowed from them.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK nice, seems they just removed it.
erlang/otp@67513dc#diff-030fc115adbcd287aecbd623b8355c9cf9aa1fd05ef07b4001b05ce927ec5e03
OK to remove the tests completely then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes!