Repository: elixir-grpc/grpc
Pull request #266 (2022-08-22 01:48:28)

Replace tuple style supervisor child_spec with map on cowboy adapter
PR for #265  @polvalente 

- Changing child_spec format/type from Supervisor.Spec.spec() (a tuple) to Supervisor.child_spec() (a map)
- Updating test case


prihandi: I see CI pipeline failed on dialyzer check because current `:ranch.child_spec` will never return a map, 

Should I suppress dialyzer warning for this?
polvalente: > I see CI pipeline failed on dialyzer check because current `:ranch.child_spec` will never return a map, 
> 
> Should I suppress dialyzer warning for this?

Does the code allow for returning a map, though?
prihandi: Yes, for newer version of `:ranch` 

polvalente: Looks like this PR should also update Ranch

Patch:
diff --git a/lib/grpc/server/adapters/cowboy.ex b/lib/grpc/server/adapters/cowboy.ex
index 4a80c33..51caa7f 100644
--- a/lib/grpc/server/adapters/cowboy.ex
+++ b/lib/grpc/server/adapters/cowboy.ex
@@ -7,6 +7,9 @@ defmodule GRPC.Server.Adapters.Cowboy do
 
   @behaviour GRPC.Server.Adapter
 
+  # ignore a specific warning generated by the case :ranch.child_spec call
+  @dialyzer {:no_match, child_spec: 4}
+
   require Logger
   alias GRPC.Server.Adapters.Cowboy.Handler
 
@@ -30,7 +33,7 @@ defmodule GRPC.Server.Adapters.Cowboy do
   end
 
   @spec child_spec(atom(), %{String.t() => [module()]}, non_neg_integer(), Keyword.t()) ::
-          Supervisor.Spec.spec()
+          Supervisor.child_spec()
   def child_spec(endpoint, servers, port, opts) do
     [ref, trans_opts, proto_opts] = cowboy_start_args(endpoint, servers, port, opts)
     trans_opts = Map.put(trans_opts, :connection_type, :supervisor)
@@ -42,13 +45,26 @@ defmodule GRPC.Server.Adapters.Cowboy do
         {:ranch_tcp, :cowboy_clear}
       end
 
-    {ref, mfa, type, timeout, kind, modules} =
-      :ranch.child_spec(ref, transport, trans_opts, protocol, proto_opts)
+    # Ideally, we would just update Ranch, but compatibility issues with cowboy hold us back on this
+    # So we just support both child spec versions here instead
+    case :ranch.child_spec(ref, transport, trans_opts, protocol, proto_opts) do
+      {ref, mfa, type, timeout, kind, modules} ->
+        scheme = if opts[:cred], do: :https, else: :http
+        # Wrap real mfa to print starting log
+        wrapped_mfa = {__MODULE__, :start_link, [scheme, endpoint, servers, mfa]}
 
-    scheme = if opts[:cred], do: :https, else: :http
-    # Wrap real mfa to print starting log
-    mfa = {__MODULE__, :start_link, [scheme, endpoint, servers, mfa]}
-    {ref, mfa, type, timeout, kind, modules}
+        %{
+          id: ref,
+          start: wrapped_mfa,
+          restart: type,
+          shutdown: timeout,
+          type: kind,
+          modules: modules
+        }
+
+      child_spec when is_map(child_spec) ->
+        child_spec
+    end
   end
 
   # spec: :supervisor.mfargs doesn't work


Test patch:
diff --git a/test/grpc/server/supervisor_test.exs b/test/grpc/server/supervisor_test.exs
index e69de01..495b6b0 100644
--- a/test/grpc/server/supervisor_test.exs
+++ b/test/grpc/server/supervisor_test.exs
@@ -33,8 +33,11 @@ defmodule GRPC.Server.SupervisorTest do
       assert {:ok,
               {%{strategy: :one_for_one},
                [
-                 {{:ranch_listener_sup, ^endpoint_str}, _, :permanent, :infinity, :supervisor,
-                  [:ranch_listener_sup]}
+                 %{
+                   id: {:ranch_listener_sup, ^endpoint_str},
+                   start: _,
+                   type: :supervisor
+                 }
                ]}} = Supervisor.init(endpoint: MockEndpoint, port: 1234, start_server: true)
     end
   end