Improve application error formatting

fishcakez · fishcakez · commit c2ceeb9ea4c4 · 2014-05-07T00:22:51.000+01:00
* Format (nested) supervision tree init failures
* Format :gen_server exit reasons
* Fixes :invalid_options text
* Fixes text when when application start returns {:error, reason}
diff --git a/lib/elixir/lib/application.ex b/lib/elixir/lib/application.ex
@@ -334,7 +334,8 @@ defmodule Application do
 
   # {:error, reason} return value
   defp impl_format_reason({reason, {mod, :start, args}}) do
-    Exception.format_exit({reason, {mod, :start, args}})
+    Exception.format_mfa(mod, :start, args) <> " returned an error: " <>
+      Exception.format_exit(reason)
   end
 
   # error or exit(reason) call, use exit reason as reason.
@@ -344,8 +345,8 @@ defmodule Application do
 
   # bad return value
   defp impl_format_reason({:bad_return, {{mod, :start, args}, return}}) do
-    Exception.format_mfa(mod, :start, args) <> " had bad return: " <>
-      inspect(return)
+    Exception.format_mfa(mod, :start, args) <>
+      " returned a bad value: " <> inspect(return)
   end
 
   defp impl_format_reason({:already_started, app}) when is_atom(app) do
@@ -377,7 +378,7 @@ defmodule Application do
   end
 
   defp impl_format_reason({:invalid_options, opts}) do
-    "invalid application name: #{inspect(opts)}"
+    "invalid application options: #{inspect(opts)}"
   end
 
   defp impl_format_reason({:badstartspec, spec}) do
diff --git a/lib/elixir/lib/exception.ex b/lib/elixir/lib/exception.ex
@@ -367,8 +367,24 @@ defmodule Exception do
     end
   end
 
-  # 2-Tuple could be an exit caused by mfa if second element is mfa,
-  defp format_exit({reason2, {mod, fun, args}} = reason, joiner) do
+  # :supervisor.start_link returns this error reason when it fails to init
+  # because a child's start_link raises.
+  defp format_exit({:shutdown,
+      {:failed_to_start_child, child, {:EXIT, reason}}}, joiner) do
+    format_start_child(child, reason, joiner)
+  end
+
+  # :supervisor.start_link returns this error reason when it fails to init
+  # because a child's start_link returns {:error, reason}.
+  defp format_exit({:shutdown, {:failed_to_start_child, child, reason}},
+      joiner) do
+    format_start_child(child, reason, joiner)
+  end
+
+  # 2-Tuple could be an exit caused by mfa if second element is mfa, args
+  # must be a list of arguments - max length 255 due to max arity.
+  defp format_exit({reason2, {mod, fun, args}} = reason, joiner)
+      when length(args) < 256 do
     try do
       format_mfa(mod, fun, args)
     else
@@ -407,8 +423,99 @@ defmodule Exception do
     "no connection to #{node_name}"
   end
 
+  # :gen_server exit reasons
+
+  defp format_exit_reason({:already_started, pid}) do
+    "already started: " <> inspect(pid)
+  end
+
+  defp format_exit_reason({:bad_return_value, value}) do
+    "bad return value: " <> inspect(value)
+  end
+
+  defp format_exit_reason({:bad_call, request}) do
+    "bad call: " <> inspect(request)
+  end
+
+  defp format_exit_reason({:bad_cast, request}) do
+    "bad cast: " <> inspect(request)
+  end
+
+  # :supervisor.start_link error reasons
+
+  # If value is a list will be be formatted by mfa exit in format_exit/1
+  defp format_exit_reason({:bad_return, {mod, :init, value}})
+      when is_atom(mod) do
+    format_mfa(mod, :init, 1) <> " returned a bad value: " <> inspect(value)
+  end
+
+  defp format_exit_reason({:bad_start_spec, start_spec}) do
+    "bad start spec: invalid children: " <> inspect(start_spec)
+  end
+
+  defp format_exit_reason({:start_spec, start_spec}) do
+    "bad start spec: " <> format_sup_spec(start_spec)
+  end
+
+  defp format_exit_reason({:supervisor_data, data}) do
+    "bad supervisor data: " <> format_sup_data(data)
+  end
+
   defp format_exit_reason(reason), do: inspect(reason)
 
+  defp format_start_child(child, reason, joiner) do
+    "shutdown: failed to start child: " <> inspect(child) <> joiner <>
+      "** (EXIT) " <> format_exit(reason, joiner <> <<"    ">>)
+  end
+
+  defp format_sup_data({:invalid_type, type}) do
+    "invalid type: " <> inspect(type)
+  end
+
+  defp format_sup_data({:invalid_strategy, strategy}) do
+    "invalid strategy: " <> inspect(strategy)
+  end
+
+  defp format_sup_data({:invalid_intensity, intensity}) do
+    "invalid intensity: " <> inspect(intensity)
+  end
+
+  defp format_sup_data({:invalid_period, period}) do
+    "invalid period: " <> inspect(period)
+  end
+
+  defp format_sup_data(other), do: inspect(other)
+
+  defp format_sup_spec({:invalid_child_spec, child_spec}) do
+   "invalid child spec: " <> inspect(child_spec)
+  end
+
+  defp format_sup_spec({:invalid_child_type, type}) do
+    "invalid child type: " <> inspect(type)
+  end
+
+  defp format_sup_spec({:invalid_mfa, mfa}) do
+    "invalid mfa: " <> inspect(mfa)
+  end
+
+  defp format_sup_spec({:invalid_restart_type, restart}) do
+    "invalid restart type: " <> inspect(restart)
+  end
+
+  defp format_sup_spec({:invalid_shutdown, shutdown}) do
+    "invalid shutdown: " <> inspect(shutdown)
+  end
+
+  defp format_sup_spec({:invalid_module, mod}) do
+    "invalid module: " <> inspect(mod)
+  end
+
+  defp format_sup_spec({:invalid_modules, modules}) do
+    "invalid modules: " <> inspect(modules)
+  end
+
+  defp format_sup_spec(other), do: inspect(other)
+
   @doc """
   Receives a stacktrace entry and formats it into a string.
   """
diff --git a/lib/elixir/test/elixir/exception_test.exs b/lib/elixir/test/elixir/exception_test.exs
@@ -74,6 +74,106 @@ defmodule Kernel.ExceptionTest do
     assert Exception.format_exit({:shutdown, :bye}) == "shutdown: :bye"
     assert Exception.format_exit({:badarg,[{:not_a_real_module, :function, 0, []}]}) ==
            "an exception was raised:\n    ** (ArgumentError) argument error\n        :not_a_real_module.function/0"
+    assert Exception.format_exit({:bad_call, :request}) == "bad call: :request"
+    assert Exception.format_exit({:bad_cast, :request}) == "bad cast: :request"
+    assert Exception.format_exit({:start_spec, :unexpected}) ==
+           "bad start spec: :unexpected"
+    assert Exception.format_exit({:supervisor_data, :unexpected}) ==
+           "bad supervisor data: :unexpected"
+  end
+
+  defmodule Sup do
+    def start_link(fun), do: :supervisor.start_link(__MODULE__, fun)
+
+    def init(fun), do: fun.()
+  end
+
+  test "format_exit with supervisor errors" do
+    trap = Process.flag(:trap_exit, true)
+
+    {:error, reason} = __MODULE__.Sup.start_link(fn() -> :foo end)
+    assert Exception.format_exit(reason) ==
+           "#{inspect(__MODULE__.Sup)}.init/1 returned a bad value: :foo"
+
+    return = {:ok, {:foo, []}}
+    {:error, reason} = __MODULE__.Sup.start_link(fn() -> return end)
+    assert Exception.format_exit(reason) ==
+           "bad supervisor data: invalid type: :foo"
+
+    return = {:ok, {{:foo, 1, 1}, []}}
+    {:error, reason} = __MODULE__.Sup.start_link(fn() -> return end)
+    assert Exception.format_exit(reason) ==
+           "bad supervisor data: invalid strategy: :foo"
+
+    return = {:ok, {{:one_for_one, :foo, 1}, []}}
+    {:error, reason} = __MODULE__.Sup.start_link(fn() -> return end)
+    assert Exception.format_exit(reason) ==
+           "bad supervisor data: invalid intensity: :foo"
+
+    return = {:ok, {{:one_for_one, 1, :foo}, []}}
+    {:error, reason} = __MODULE__.Sup.start_link(fn() -> return end)
+    assert Exception.format_exit(reason) ==
+           "bad supervisor data: invalid period: :foo"
+
+    return = {:ok, {{:simple_one_for_one, 1, 1}, :foo}}
+    {:error, reason} = __MODULE__.Sup.start_link(fn() -> return end)
+    assert Exception.format_exit(reason) ==
+           "bad start spec: invalid children: :foo"
+
+    return = {:ok, {{:one_for_one, 1, 1}, [:foo]}}
+    {:error, reason} = __MODULE__.Sup.start_link(fn() -> return end)
+    assert Exception.format_exit(reason) ==
+           "bad start spec: invalid child spec: :foo"
+
+    return = {:ok, {{:one_for_one, 1, 1},
+        [{:child, :foo, :temporary, 1, :worker, []}]}}
+    {:error, reason} = __MODULE__.Sup.start_link(fn() -> return end)
+    assert Exception.format_exit(reason) ==
+           "bad start spec: invalid mfa: :foo"
+
+    return = {:ok, {{:one_for_one, 1, 1},
+        [{:child, {:m, :f, []}, :foo, 1, :worker, []}]}}
+    {:error, reason} = __MODULE__.Sup.start_link(fn() -> return end)
+    assert Exception.format_exit(reason) ==
+           "bad start spec: invalid restart type: :foo"
+
+    return = {:ok, {{:one_for_one, 1, 1},
+        [{:child, {:m, :f, []}, :temporary, :foo, :worker, []}]}}
+    {:error, reason} = __MODULE__.Sup.start_link(fn() -> return end)
+    assert Exception.format_exit(reason) ==
+           "bad start spec: invalid shutdown: :foo"
+
+    return = {:ok, {{:one_for_one, 1, 1},
+        [{:child, {:m, :f, []}, :temporary, 1, :foo, []}]}}
+    {:error, reason} = __MODULE__.Sup.start_link(fn() -> return end)
+    assert Exception.format_exit(reason) ==
+           "bad start spec: invalid child type: :foo"
+
+    return = {:ok, {{:one_for_one, 1, 1},
+        [{:child, {:m, :f, []}, :temporary, 1, :worker, :foo}]}}
+    {:error, reason} = __MODULE__.Sup.start_link(fn() -> return end)
+    assert Exception.format_exit(reason) ==
+           "bad start spec: invalid modules: :foo"
+
+    return = {:ok, {{:one_for_one, 1, 1},
+        [{:child, {:m, :f, []}, :temporary, 1, :worker, [{:foo}]}]}}
+    {:error, reason} = __MODULE__.Sup.start_link(fn() -> return end)
+    assert Exception.format_exit(reason) ==
+           "bad start spec: invalid module: {:foo}"
+
+    return = {:ok, {{:one_for_one, 1, 1},
+        [{:child, {Kernel, :exit, [:foo]}, :temporary, 1, :worker, []}]}}
+    {:error, reason} = __MODULE__.Sup.start_link(fn() -> return end)
+    assert Exception.format_exit(reason) ==
+           "shutdown: failed to start child: :child\n    ** (EXIT) :foo"
+
+    return = {:ok, {{:one_for_one, 1, 1},
+        [{:child, {Kernel, :apply, [fn() -> {:error, :foo} end, []]}, :temporary, 1, :worker, []}]}}
+    {:error, reason} = __MODULE__.Sup.start_link(fn() -> return end)
+    assert Exception.format_exit(reason) ==
+           "shutdown: failed to start child: :child\n    ** (EXIT) :foo"
+
+    Process.flag(:trap_exit, trap)
   end
 
   test "format_exit with call" do
diff --git a/lib/mix/test/mix/tasks/app.start_test.exs b/lib/mix/test/mix/tasks/app.start_test.exs
@@ -124,9 +124,7 @@ defmodule Mix.Tasks.App.StartTest do
   defmodule ReturnApp do
     use Application.Behaviour
 
-    def start(_type, return) do
-      return # return
-    end
+    def start(_type, return), do: return
   end
 
   @tag app: :return_sample
@@ -136,8 +134,8 @@ defmodule Mix.Tasks.App.StartTest do
       Process.put(:application_definition, mod: {ReturnApp, {:error, :bye}})
       Mix.Tasks.Compile.run []
 
-      assert_raise Mix.Error, "Could not start application return_sample: exited in: Mix.Tasks.App.StartTest.ReturnApp.start(:normal, {:error, :bye})\n" <>
-      "    ** (EXIT) :bye", fn ->
+      assert_raise Mix.Error, "Could not start application return_sample: Mix.Tasks.App.StartTest.ReturnApp.start(:normal, {:error, :bye}) returned an error: :bye",
+      fn ->
         Mix.Tasks.App.Start.start(:return_sample)
       end
     end
@@ -151,43 +149,70 @@ defmodule Mix.Tasks.App.StartTest do
           {:badarg, [{ReturnApp, :start, 2, []}] }}})
       Mix.Tasks.Compile.run []
 
-      assert_raise Mix.Error, "Could not start application return_sample: exited in: Mix.Tasks.App.StartTest.ReturnApp.start(:normal, {:error, {:badarg, [{Mix.Tasks.App.StartTest.ReturnApp, :start, 2, []}]}})\n" <>
-        "    ** (EXIT) an exception was raised:\n" <>
-        "        ** (ArgumentError) argument error\n" <>
-        "            Mix.Tasks.App.StartTest.ReturnApp.start/2", fn ->
+      assert_raise Mix.Error, "Could not start application return_sample: Mix.Tasks.App.StartTest.ReturnApp.start(:normal, {:error, {:badarg, [{Mix.Tasks.App.StartTest.ReturnApp, :start, 2, []}]}}) returned an error: an exception was raised:\n" <>
+        "    ** (ArgumentError) argument error\n" <>
+        "        Mix.Tasks.App.StartTest.ReturnApp.start/2", fn ->
       Mix.Tasks.App.Start.start(:return_sample)
       end
     end
   end
 
   @tag app: :return_sample
-  test "start points to report on exit" do
+  test "start points to report on bad return" do
     Mix.Project.push ReturnSample
     in_fixture "no_mixfile", fn ->
-      Process.put(:application_definition, mod: {ReturnApp, {:EXIT, :bye}})
+      Process.put(:application_definition, mod: {ReturnApp, :bad})
       Mix.Tasks.Compile.run []
 
-      assert_raise Mix.Error, "Could not start application return_sample: exited in: Mix.Tasks.App.StartTest.ReturnApp.start(:normal, {:EXIT, :bye})\n" <>
+      assert_raise Mix.Error, "Could not start application return_sample: Mix.Tasks.App.StartTest.ReturnApp.start(:normal, :bad) returned a bad value: :bad",
+      fn ->
+        Mix.Tasks.App.Start.start(:return_sample)
+      end
+    end
+  end
+
+  defmodule ExitSample do
+    def project do
+      [app: :exit_sample, version: "0.1.0"]
+    end
+
+    def application do
+      Process.get(:application_definition)
+    end
+  end
+
+  defmodule ExitApp do
+    use Application.Behaviour
+
+    def start(_type, reason), do: exit(reason)
+  end
+
+  @tag app: :exit_sample
+  test "start points to report on exit" do
+    Mix.Project.push ExitSample
+    in_fixture "no_mixfile", fn ->
+      Process.put(:application_definition, mod: {ExitApp, :bye})
+      Mix.Tasks.Compile.run []
+
+      assert_raise Mix.Error, "Could not start application exit_sample: exited in: Mix.Tasks.App.StartTest.ExitApp.start(:normal, :bye)\n" <>
       "    ** (EXIT) :bye",
          fn ->
-        Mix.Tasks.App.Start.start(:return_sample)
+        Mix.Tasks.App.Start.start(:exit_sample)
       end
     end
   end
 
-  @tag app: :return_sample
-  test "start points to report on exception exit" do
-    Mix.Project.push ReturnSample
+  @tag app: :exit_sample
+  test "start points to report on normal exit" do
+    Mix.Project.push ExitSample
     in_fixture "no_mixfile", fn ->
-      Process.put(:application_definition, mod: {ReturnApp, {:EXIT,
-          {:badarg, [{ReturnApp, :start, 2, []}] }}})
+      Process.put(:application_definition, mod: {ExitApp, :normal})
       Mix.Tasks.Compile.run []
 
-      assert_raise Mix.Error, "Could not start application return_sample: exited in: Mix.Tasks.App.StartTest.ReturnApp.start(:normal, {:EXIT, {:badarg, [{Mix.Tasks.App.StartTest.ReturnApp, :start, 2, []}]}})\n" <>
-        "    ** (EXIT) an exception was raised:\n" <>
-        "        ** (ArgumentError) argument error\n" <>
-        "            Mix.Tasks.App.StartTest.ReturnApp.start/2", fn ->
-      Mix.Tasks.App.Start.start(:return_sample)
+      assert_raise Mix.Error, "Could not start application exit_sample: exited in: Mix.Tasks.App.StartTest.ExitApp.start(:normal, :normal)\n" <>
+      "    ** (EXIT) normal",
+         fn ->
+        Mix.Tasks.App.Start.start(:exit_sample)
       end
     end
   end
