Skip to content
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

Handle :undefined in Exception.format_arity/1 #3288

Merged
merged 2 commits into from Jun 7, 2015

Conversation

alco
Copy link
Member

@alco alco commented Apr 23, 2015

We get :undefined when a temporary worker of a simple_one_for_one supervisor crashes.

This PR fixes the following error.

18:19:15.088 [error] GenEvent handler Logger.ErrorHandler installed at :error_logger
** (exit) an exception was raised:
    ** (FunctionClauseError) no function clause matching in Exception.format_arity/1
        (elixir) lib/exception.ex:431: Exception.format_arity(:undefined)
        (elixir) lib/exception.ex:427: Exception.format_mfa/3
        (logger) lib/logger/translator.ex:165: Logger.Translator.child_info/2
        (logger) lib/logger/translator.ex:90: Logger.Translator.translate_supervisor/2
        (logger) lib/logger/error_handler.ex:116: Logger.ErrorHandler.translate/6
        (logger) lib/logger/error_handler.ex:62: Logger.ErrorHandler.log_event/5
        (logger) lib/logger/error_handler.ex:27: Logger.ErrorHandler.handle_event/2
        (stdlib) gen_event.erl:525: :gen_event.server_update/4

To reproduce the error, create a new application with mix new --sup arity, add the following code to it:

defmodule Worker do
  use GenServer
  def start_link, do: GenServer.start_link(__MODULE__, [], [])
  def init([]), do: 1 / 0
end

defmodule Arity do
  use Application

  def start(_type, _args) do
    import Supervisor.Spec
    children = [worker(Worker, [], restart: :temporary)]
    opts = [strategy: :simple_one_for_one, name: __MODULE__]
    Supervisor.start_link(children, opts)
  end

  def start_child do
    Supervisor.start_child(__MODULE__, [])
    receive do end
  end
end

And configure :logger as follows:

config :logger, [
  handle_sasl_reports: true,
]

Finally, run

$ mix run -e Arity.start_child

@alco
Copy link
Member Author

alco commented Apr 23, 2015

Thanks to @fishcakez for helping diagnose this.

@fishcakez
Copy link
Member

Should fix this in Logger.Translator and not Exception. Can you add a test too?

@alco
Copy link
Member Author

alco commented Apr 23, 2015

Sure, I'll push fixes tomorrow.

@alco
Copy link
Member Author

alco commented Apr 26, 2015

@fishcakez what is your argument for catching :undefined in the translator? In this particular case the error stems from this function but I am not sure that :undefined does not also appear here or here.

@fishcakez
Copy link
Member

I don't think it makes sense to handle :undefined arity in Exception because it is not a valid arity. lt would be ok for the translator to have its own format_mfa.

@alco
Copy link
Member Author

alco commented May 24, 2015

Sorry, I don't really understand how to best implement a custom format_mfa in Logger.Translator without just copying the implementation from Exception.

@fishcakez
Copy link
Member

The custom one can call Exception, it just needs to handle the :undefined args/arity before doing so. Something like:

defp format_mfa(mod, fun, :undefined), do: [inspect(mod), ?. | inspect(fun)]
defp format_mfa(mod, fun, args), do: Exception.format_mfa(mod, fun, args)

We get :undefined when a temporary worker of a simple_one_for_one
supervisor crashes.

Thanks to @fishcakez for feedback
@alco
Copy link
Member Author

alco commented Jun 7, 2015

Pushed updated implementation and added a test case.

defp format_arity(:undefined) do
"/?"
end

Copy link
Member

Choose a reason for hiding this comment

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

@alco perfect except you forgot to remove this, otherwise 👍.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the heads-up.

fishcakez added a commit that referenced this pull request Jun 7, 2015
Handle :undefined in Exception.format_arity/1
@fishcakez fishcakez merged commit d4eb690 into elixir-lang:master Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants