Twirp.Error's MetaMap should use string as keys

lib/twirp/client/http.ex

@@ -11,28 +11,28 @@ defmodule Twirp.Client.HTTP do
   end
 
   def call(mod, client, ctx, rpc) do
-    path            = "#{rpc.service_url}/#{rpc.method}"
-    content_type    = ctx.content_type
+    path = "#{rpc.service_url}/#{rpc.method}"
+    content_type = ctx.content_type
     encoded_payload = Encoder.encode(rpc.req, rpc.input_type, content_type)
 
     case mod.request(client, ctx, path, encoded_payload) do
       {:error, %{reason: :timeout}} ->
-        meta = %{error_type: "timeout"}
+        meta = %{"error_type" => "timeout"}
         msg = "Deadline to receive data from the service was exceeded"
         {:error, Error.deadline_exceeded(msg, meta)}
 
       {:error, %{reason: reason}} ->
-        meta = %{error_type: "#{reason}"}
+        meta = %{"error_type" => "#{reason}"}
         {:error, Error.unavailable("Service is down", meta)}
 
       {:error, e} ->
-        meta = %{error_type: "#{inspect e}"}
+        meta = %{"error_type" => "#{inspect(e)}"}
         {:error, Error.internal("Unhandled client error", meta)}
 
-      {:ok, %{status: status}=resp} when status != 200 ->
+      {:ok, %{status: status} = resp} when status != 200 ->
         {:error, build_error(resp, rpc)}
 
-      {:ok, %{status: 200}=resp} ->
+      {:ok, %{status: 200} = resp} ->
         handle_success(resp, rpc, content_type)
     end
   end
@@ -43,7 +43,10 @@ defmodule Twirp.Client.HTTP do
     if resp_content_type && String.starts_with?(resp_content_type, content_type) do
       Encoder.decode(resp.body, rpc.output_type, content_type)
     else
-      {:error, Error.internal(~s|Expected response Content-Type "#{content_type}" but found #{resp_content_type || "nil"}|)}
+      {:error,
+       Error.internal(
+         ~s|Expected response Content-Type "#{content_type}" but found #{resp_content_type || "nil"}|
+       )}
     end
   end
 
@@ -53,24 +56,29 @@ defmodule Twirp.Client.HTTP do
     cond do
       http_redirect?(status) ->
         location = resp_header(resp, "location")
+
         meta = %{
-          http_error_from_intermediary: "true",
-          not_a_twirp_error_because: "Redirects not allowed on Twirp requests",
-          status_code: Integer.to_string(status),
-          location: location,
+          "http_error_from_intermediary" => "true",
+          "not_a_twirp_error_because" => "Redirects not allowed on Twirp requests",
+          "status_code" => Integer.to_string(status),
+          "location" => location
         }
+
         msg = "Unexpected HTTP Redirect from location=#{location}"
         Error.internal(msg, meta)
 
       true ->
         case Encoder.decode_json(resp.body) do
-          {:ok, %{"code" => code, "msg" => msg}=error} ->
+          {:ok, %{"code" => code, "msg" => msg} = error} ->
             if Error.valid_code?(code) do
-                # Its safe to convert to an atom here since all the codes are already
-                # created and loaded. If we explode we explode.
+              # Its safe to convert to an atom here since all the codes are already
+              # created and loaded. If we explode we explode.
               Error.new(String.to_existing_atom(code), msg, error["meta"] || %{})
             else
-              Error.internal("Invalid Twirp error code: #{code}", invalid_code: code, body: resp.body)
+              Error.internal("Invalid Twirp error code: #{code}", %{
+                "invalid_code" => code,
+                "body" => resp.body
+              })
             end
 
           {:ok, _} ->
@@ -85,10 +93,10 @@ defmodule Twirp.Client.HTTP do
 
   defp intermediary_error(status, reason, body) do
     meta = %{
-      http_error_from_intermediary: "true",
-      not_a_twirp_error_because: reason,
-      status_code: Integer.to_string(status),
-      body: body
+      "http_error_from_intermediary" => "true",
+      "not_a_twirp_error_because" => reason,
+      "status_code" => Integer.to_string(status),
+      "body" => body
     }
 
     case status do

lib/twirp/error.ex

@@ -42,7 +42,7 @@ defmodule Twirp.Error do
   }
 
   for code <- @error_codes do
-    def unquote(code)(msg, meta \\ []) do
+    def unquote(code)(msg, meta \\ %{}) do
       new(unquote(code), msg, meta)
     end
   end
@@ -58,12 +58,12 @@ defmodule Twirp.Error do
     schema(%__MODULE__{
       code: spec(is_atom() and (& &1 in @error_codes)),
       msg: spec(is_binary()),
-      meta: map_of(spec(is_atom()), spec(is_binary())),
+      meta: map_of(spec(is_binary()), spec(is_binary())),
     })
   end
 
-  def new(code, msg, meta \\ []) do
-    conform!(%__MODULE__{code: code, msg: msg, meta: Enum.into(meta, %{})}, s())
+  def new(code, msg, meta \\ %{}) do
+    conform!(%__MODULE__{code: code, msg: msg, meta: meta}, s())
   end
 
   @impl true

lib/twirp/plug.ex

@@ -290,6 +290,6 @@ defmodule Twirp.Plug do
   end
 
   defp bad_route(msg, conn) do
-    Error.bad_route(msg, twirp_invalid_route: "#{conn.method} #{conn.request_path}")
+    Error.bad_route(msg, %{"twirp_invalid_route" => "#{conn.method} #{conn.request_path}"})
   end
 end

test/twirp/client/hackney_test.exs

@@ -107,9 +107,9 @@ defmodule Twirp.Client.HackneyTest do
     assert {:error, resp} = Client.echo(Req.new(msg: "test"))
     assert resp.code == :unavailable
     assert resp.msg == "unavailable"
-    assert resp.meta.http_error_from_intermediary == "true"
-    assert resp.meta.not_a_twirp_error_because == "Response is not JSON"
-    assert resp.meta.body == "plain text error"
+    assert resp.meta["http_error_from_intermediary"] == "true"
+    assert resp.meta["not_a_twirp_error_because"] == "Response is not JSON"
+    assert resp.meta["body"] == "plain text error"
   end
 
   test "error has no code", %{service: service} do
@@ -122,8 +122,8 @@ defmodule Twirp.Client.HackneyTest do
     assert {:error, resp} = Client.echo(Req.new(msg: "test"))
     assert resp.code == :unknown
     assert resp.msg == "unknown"
-    assert resp.meta.http_error_from_intermediary == "true"
-    assert resp.meta.not_a_twirp_error_because == "Response is JSON but it has no \"code\" attribute"
+    assert resp.meta["http_error_from_intermediary"] == "true"
+    assert resp.meta["not_a_twirp_error_because"] == "Response is JSON but it has no \"code\" attribute"
   end
 
   test "error has incorrect code", %{service: service} do
@@ -136,7 +136,7 @@ defmodule Twirp.Client.HackneyTest do
     assert {:error, resp} = Client.echo(Req.new(msg: "test"))
     assert resp.code == :internal
     assert resp.msg == "Invalid Twirp error code: keathley"
-    assert resp.meta.invalid_code == "keathley"
+    assert resp.meta["invalid_code"] == "keathley"
   end
 
   test "redirect errors", %{service: service} do
@@ -150,14 +150,14 @@ defmodule Twirp.Client.HackneyTest do
 
     assert {:error, resp} = Client.echo(Req.new(msg: "test"))
     assert match?(%Error{code: :internal}, resp)
-    assert resp.meta.http_error_from_intermediary == "true"
-    assert resp.meta.not_a_twirp_error_because == "Redirects not allowed on Twirp requests"
+    assert resp.meta["http_error_from_intermediary"] == "true"
+    assert resp.meta["not_a_twirp_error_because"] == "Redirects not allowed on Twirp requests"
   end
 
   test "connect timeouts", %{service: _service} do
     assert {:error, resp} = Client.echo(%{connect_deadline: 0}, Req.new(msg: "test"))
     assert resp.code == :deadline_exceeded
-    assert resp.meta.error_type == "timeout"
+    assert resp.meta["error_type"] == "timeout"
   end
 
   test "recv timeouts", %{service: service} do
@@ -168,7 +168,7 @@ defmodule Twirp.Client.HackneyTest do
 
     assert {:error, resp} = Client.echo(%{deadline: 1}, Req.new(msg: "test"))
     assert resp.code == :deadline_exceeded
-    assert resp.meta.error_type == "timeout"
+    assert resp.meta["error_type"] == "timeout"
   end
 
   test "service is down", %{service: service} do

test/twirp/client_test.exs

@@ -85,9 +85,9 @@ defmodule Twirp.ClientTest do
     assert {:error, resp} = Client.echo(Req.new(msg: "test"))
     assert resp.code == :unavailable
     assert resp.msg == "unavailable"
-    assert resp.meta.http_error_from_intermediary == "true"
-    assert resp.meta.not_a_twirp_error_because == "Response is not JSON"
-    assert resp.meta.body == "plain text error"
+    assert resp.meta["http_error_from_intermediary"] == "true"
+    assert resp.meta["not_a_twirp_error_because"] == "Response is not JSON"
+    assert resp.meta["body"] == "plain text error"
   end
 
   test "error has no code", %{service: service} do
@@ -100,8 +100,10 @@ defmodule Twirp.ClientTest do
     assert {:error, resp} = Client.echo(Req.new(msg: "test"))
     assert resp.code == :unknown
     assert resp.msg == "unknown"
-    assert resp.meta.http_error_from_intermediary == "true"
-    assert resp.meta.not_a_twirp_error_because == "Response is JSON but it has no \"code\" attribute"
+    assert resp.meta["http_error_from_intermediary"] == "true"
+
+    assert resp.meta["not_a_twirp_error_because"] ==
+             "Response is JSON but it has no \"code\" attribute"
   end
 
   test "error has incorrect code", %{service: service} do
@@ -114,7 +116,22 @@ defmodule Twirp.ClientTest do
     assert {:error, resp} = Client.echo(Req.new(msg: "test"))
     assert resp.code == :internal
     assert resp.msg == "Invalid Twirp error code: keathley"
-    assert resp.meta.invalid_code == "keathley"
+    assert resp.meta["invalid_code"] == "keathley"
+  end
+
+  test "error has meta", %{service: service} do
+    Bypass.expect(service, fn conn ->
+      resp =
+        ~s|{"code": "internal", "msg": "Internal Server Error", "meta": {"cause": "some exception"}}|
+      conn
+      |> Plug.Conn.put_resp_content_type("application/json")
+      |> Plug.Conn.send_resp(500, resp)
+    end)
+
+    assert {:error, resp} = Client.echo(Req.new(msg: "test"))
+    assert resp.code == :internal
+    assert resp.msg == "Internal Server Error"
+    assert resp.meta == %{"cause" => "some exception"}
   end
 
   test "redirect errors", %{service: service} do
@@ -128,8 +145,8 @@ defmodule Twirp.ClientTest do
 
     assert {:error, resp} = Client.echo(Req.new(msg: "test"))
     assert match?(%Error{code: :internal}, resp)
-    assert resp.meta.http_error_from_intermediary == "true"
-    assert resp.meta.not_a_twirp_error_because == "Redirects not allowed on Twirp requests"
+    assert resp.meta["http_error_from_intermediary"] == "true"
+    assert resp.meta["not_a_twirp_error_because"] == "Redirects not allowed on Twirp requests"
   end
 
   test "service is down", %{service: service} do
@@ -141,14 +158,20 @@ defmodule Twirp.ClientTest do
 
   @tag :skip
   test "clients are easy to stub" do
-    Twirp.Client.Stub.new(echo: fn %{msg: "foo"} ->
-      Error.unavailable("test")
-    end)
+    Twirp.Client.Stub.new(
+      echo: fn %{msg: "foo"} ->
+        Error.unavailable("test")
+      end
+    )
+
     assert {:error, %Error{code: :unavailable}} = Client.echo(Req.new(msg: "foo"))
 
-    Twirp.Client.Stub.new(echo: fn %{msg: "foo"} ->
-      Resp.new(msg: "foo")
-    end)
+    Twirp.Client.Stub.new(
+      echo: fn %{msg: "foo"} ->
+        Resp.new(msg: "foo")
+      end
+    )
+
     assert {:ok, %Resp{msg: "foo"}} = Client.echo(Req.new(msg: "foo"))
 
     assert_raise Twirp.Client.StubError, ~r/does not define/, fn ->
@@ -157,9 +180,12 @@ defmodule Twirp.ClientTest do
     end
 
     assert_raise Twirp.Client.StubError, ~r/expected to return/, fn ->
-      Twirp.Client.Stub.new(echo: fn _ ->
-        {:ok, Req.new(msg: "test")}
-      end)
+      Twirp.Client.Stub.new(
+        echo: fn _ ->
+          {:ok, Req.new(msg: "test")}
+        end
+      )
+
       Client.echo(Req.new(msg: "foo"))
     end
   end

test/twirp_test.exs

@@ -61,7 +61,7 @@ defmodule TwirpTest do
 
     assert {:error, resp} = Client.slow_echo(%{deadline: 5}, req)
     assert resp.code == :deadline_exceeded
-    assert resp.meta.error_type == "timeout"
+    assert resp.meta["error_type"] == "timeout"
   end
 
   test "clients allow interceptors" do