Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 38 additions & 7 deletions lib/elixir/lib/dynamic_supervisor.ex
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,20 @@ defmodule DynamicSupervisor do
end

def handle_info(msg, state) do
:error_logger.error_msg('DynamicSupervisor received unexpected message: ~p~n', [msg])
:logger.error(
%{
label: {DynamicSupervisor, :unexpected_msg},
report: %{
msg: msg
}
},
%{
domain: [:otp, :elixir],
error_logger: %{tag: :error_msg},
report_cb: &__MODULE__.format_report/1
}
)

{:noreply, state}
end

Expand Down Expand Up @@ -984,12 +997,22 @@ defmodule DynamicSupervisor do
end

defp report_error(error, reason, pid, child, %{name: name, extra_arguments: extra}) do
:error_logger.error_report(
:supervisor_report,
supervisor: name,
errorContext: error,
reason: reason,
offender: extract_child(pid, child, extra)
:logger.error(
%{
label: {:supervisor, error},
report: [
{:supervisor, name},
{:errorContext, error},
{:reason, reason},
{:offender, extract_child(pid, child, extra)}
]
},
%{
domain: [:otp, :sasl],
report_cb: &:logger.format_otp_report/1,
logger_formatter: %{title: "SUPERVISOR REPORT"},
error_logger: %{tag: :error_report, type: :supervisor_report}
}
)
end

Expand Down Expand Up @@ -1020,4 +1043,12 @@ defmodule DynamicSupervisor do
defp call(supervisor, req) do
GenServer.call(supervisor, req, :infinity)
end

@doc false
def format_report(%{
label: {__MODULE__, :unexpected_msg},
report: %{msg: msg}
}) do
{'DynamicSupervisor received unexpected message: ~p~n', [msg]}
end
end
26 changes: 24 additions & 2 deletions lib/elixir/lib/gen_server.ex
Original file line number Diff line number Diff line change
Expand Up @@ -785,8 +785,22 @@ defmodule GenServer do
{_, name} -> name
end

pattern = '~p ~p received unexpected message in handle_info/2: ~p~n'
:error_logger.error_msg(pattern, [__MODULE__, proc, msg])
:logger.error(
%{
label: {GenServer, :no_handle_info},
report: %{
module: __MODULE__,
message: msg,
name: proc
}
},
%{
domain: [:otp, :elixir],
error_logger: %{tag: :error_msg},
report_cb: &GenServer.format_report/1
}
)

{:noreply, state}
end

Expand Down Expand Up @@ -1205,4 +1219,12 @@ defmodule GenServer do
def whereis({name, node} = server) when is_atom(name) and is_atom(node) do
server
end

@doc false
def format_report(%{
label: {GenServer, :no_handle_info},
report: %{module: mod, message: msg, name: proc}
}) do
{'~p ~p received unexpected message in handle_info/2: ~p~n', [mod, proc, msg]}
end
end
37 changes: 32 additions & 5 deletions lib/elixir/lib/task/supervised.ex
Original file line number Diff line number Diff line change
Expand Up @@ -96,24 +96,51 @@ defmodule Task.Supervised do
:erlang.raise(:exit, value, __STACKTRACE__)

kind, value ->
log(owner, mfa, {log_value(kind, value), __STACKTRACE__})
{fun, args} = get_running(mfa)

:logger.error(
%{
label: {Task.Supervisor, :terminating},
report: %{
name: get_from(owner),
starter: self(),
function: fun,
args: args,
reason: {log_value(kind, value), __STACKTRACE__}
}
},
%{
domain: [:otp, :elixir],
error_logger: %{tag: :error_msg},
report_cb: &__MODULE__.format_report/1
}
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic work!


:erlang.raise(kind, value, __STACKTRACE__)
end
end

defp log_value(:throw, value), do: {:nocatch, value}
defp log_value(_, value), do: value

defp log(owner, mfa, reason) do
{fun, args} = get_running(mfa)

@doc false
def format_report(%{
label: {Task.Supervisor, :terminating},
report: %{
name: name,
starter: starter,
function: fun,
args: args,
reason: reason
}
}) do
message =
'** Task ~p terminating~n' ++
'** Started from ~p~n' ++
'** When function == ~p~n' ++
'** arguments == ~p~n' ++ '** Reason for termination == ~n' ++ '** ~p~n'

:error_logger.format(message, [self(), get_from(owner), fun, args, get_reason(reason)])
{message, [starter, name, fun, args, get_reason(reason)]}
end

defp get_from({node, pid_or_name, _pid}) when node == node(), do: pid_or_name
Expand Down
34 changes: 23 additions & 11 deletions lib/logger/lib/logger/translator.ex
Original file line number Diff line number Diff line change
Expand Up @@ -79,22 +79,34 @@ defmodule Logger.Translator do
{:ok, ["Application ", Atom.to_string(app), " exited: " | Application.format_error(reason)]}
end

def translate(_min_level, :error, :format, message) do
def translate(
_min_level,
:error,
:report,
{{Task.Supervisor, :terminating},
%{
name: name,
starter: starter,
function: function,
args: args,
reason: reason
}}
) do
opts = Application.get_env(:logger, :translator_inspect_opts)

case message do
# TODO: Remove this once tasks are migrated to Logger
{'** Task ' ++ _, [name, starter, function, args, reason]} ->
{formatted, reason} = format_reason(reason)
metadata = [crash_reason: reason] ++ registered_name(name)
{formatted, reason} = format_reason(reason)
metadata = [crash_reason: reason] ++ registered_name(name)

msg =
["Task #{inspect(name)} started from #{inspect(starter)} terminating"] ++
[formatted, "\nFunction: #{inspect(function, opts)}"] ++
["\n Args: #{inspect(args, opts)}"]
msg =
["Task #{inspect(name)} started from #{inspect(starter)} terminating"] ++
[formatted, "\nFunction: #{inspect(function, opts)}"] ++
["\n Args: #{inspect(args, opts)}"]

{:ok, msg, metadata}
{:ok, msg, metadata}
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have ported it as is, but is there any reason why do that in translator instead of using report_cb directly? I think it would make more sense to have uniform error reports.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that Is a separate discussion. Unifying reports would be a good option but we would definitely have to push some things uostream before. For example, we include more information on debug level.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@josevalim you mean in inspect? This is what 2nd argument in report_cb is for, to pass additional informations to formatters.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean we show more information in general. Compare both reports in debug mode for a crashed GenServer and you will see the differences. :)


def translate(_min_level, :error, :format, message) do
case message do
{'Error in process ' ++ _, [pid, node, {reason, stack}]} ->
reason = Exception.normalize(:error, reason, stack)

Expand Down