WIP CoAP.SocketServer errors

config/config.exs

@@ -1,3 +1,3 @@
-use Mix.Config
+import Mix.Config
 
 import_config "#{Mix.env()}.exs"

config/test.exs

@@ -1,3 +1,3 @@
-use Mix.Config
+import Mix.Config
 
 config :logger, level: :info

lib/coap/block.ex

@@ -58,6 +58,10 @@ defmodule CoAP.Block do
   def decode(<<number::size(28), more::size(1), size_exponent::size(3)>>),
     do: decode(number, more, size_exponent)
 
+  def decode(number) do
+    decode(number, 0, 0)
+  end
+
   @spec decode(integer, 0 | 1, integer) :: t()
   def decode(number, more, size_exponent) do
     %__MODULE__{

lib/coap/socket_server.ex

@@ -91,23 +91,38 @@ defmodule CoAP.SocketServer do
   def handle_info({:udp, _socket, peer_ip, peer_port, data}, state) do
     debug("CoAP socket received raw data #{to_hex(data)} from #{inspect({peer_ip, peer_port})}")
 
-    message = Message.decode(data)
+    case decode_message(data) do
+      {:ok, message} ->
+        connection_id = {peer_ip, peer_port, message.token}
 
-    {connection, new_state} =
-      connection_for(message.request, {peer_ip, peer_port, message.token}, state)
+        {connection, new_state} = connection_for(message.request, connection_id, state)
+
+        if connection do
+          send(connection, {:receive, message})
+
+          {:noreply, new_state}
+        else
+          # If we can't find a connection, we can't deliver the message to the client
+
+          warn(
+            "CoAP socket received message for lost connection from " <>
+              "ip: #{inspect(peer_ip)}, port: #{inspect(peer_port)}.  Message: #{inspect(message)}"
+          )
+
+          {:stop, :normal, state}
+        end
+
+      {:error, reason} ->
+        # If we can't decode the message, we can't construct the connection_id which is necessary to lookup
+        # the connection process, which is required to return an error message to the client.
 
-    case connection do
-      nil ->
         warn(
-          "CoAP socket received message for lost connection from " <>
-            "ip: #{inspect(peer_ip)}, port: #{inspect(peer_port)}.  Message: #{inspect(message)}"
+          "CoAP socket failed to decode udp packets because #{inspect(reason)} from " <>
+            "ip: #{inspect(peer_ip)}, port: #{inspect(peer_port)}.  data: #{inspect(data, limit: :infinity, print_limit: :infinity)}"
         )
 
-      c ->
-        send(c, {:receive, message})
+        {:stop, :normal, state}
     end
-
-    {:noreply, new_state}
   end
 
   # Deliver messages to be sent to a peer
@@ -132,6 +147,7 @@ defmodule CoAP.SocketServer do
   # Handles message for completed connection
   # Removes complete connection from the registry and monitoring
   def handle_info({:DOWN, ref, :process, _from, reason}, state) do
+    IO.inspect("CoAP socket received DOWN:#{reason} from #{inspect(ref)}")
     {host, port, _} = Map.get(state.monitors, ref)
 
     :telemetry.execute(
@@ -143,14 +159,17 @@ defmodule CoAP.SocketServer do
     connection_complete(type(state), ref, reason, state)
   end
 
+  def terminate(_reason, state) do
+    :ok = :gen_udp.close(state.socket)
+    :ok
+  end
+
   defp connection_complete(:server, ref, reason, %{monitors: monitors} = state) do
     connection_id = Map.get(monitors, ref)
     connection = Map.get(state[:connections], connection_id)
 
     debug(
-      "CoAP socket SERVER received DOWN:#{reason} in CoAP.SocketServer from:#{
-        inspect(connection_id)
-      }:#{inspect(connection)}:#{inspect(ref)}"
+      "CoAP socket SERVER received DOWN:#{reason} in CoAP.SocketServer from:#{inspect(connection_id)}:#{inspect(connection)}:#{inspect(ref)}"
     )
 
     {:noreply,
@@ -166,9 +185,7 @@ defmodule CoAP.SocketServer do
     connection = Map.get(state[:connections], connection_id)
 
     debug(
-      "CoAP socket CLIENT received DOWN:#{reason} in CoAP.SocketServer from: #{
-        inspect(connection_id)
-      }:#{inspect(connection)}:#{inspect(ref)}"
+      "CoAP socket CLIENT received DOWN:#{reason} in CoAP.SocketServer from: #{inspect(connection_id)}:#{inspect(connection)}:#{inspect(ref)}"
     )
 
     {:stop, :normal, state}
@@ -239,4 +256,14 @@ defmodule CoAP.SocketServer do
 
   defp type(%{port: 0}), do: :client
   defp type(_), do: :server
+
+  defp decode_message(data) do
+    {:ok, Message.decode(data)}
+  rescue
+    e ->
+      {:error, e}
+  catch
+    e ->
+      {:error, e}
+  end
 end

test/coap/socket_server_test.exs

@@ -0,0 +1,76 @@
+defmodule CoAP.SocketServerTest do
+  use ExUnit.Case
+
+  import ExUnit.CaptureLog
+
+  alias CoAP.SocketServer
+  alias CoAP.Adapters.GenericServer
+
+  # @moduletag :capture_log
+
+  defmodule FakeEndpoint do
+  end
+
+  describe "CoAP.SocketServer decoding errors" do
+    test "non repeatable options" do
+      peer_ip = "127.0.0.1"
+      peer_port = 5830
+
+      {:ok, server} = SocketServer.start_link([{GenericServer, FakeEndpoint}, peer_port])
+
+      data =
+        <<100, 69, 0, 1, 252, 1, 46, 56, 68, 80, 231, 168, 249, 100, 69, 0, 1, 252, 1, 46, 56, 68,
+          80, 231, 168, 249>>
+
+      log = capture_log(fn ->
+        send(server, {:udp, :socket, peer_ip, peer_port, data})
+
+        response =
+          receive do
+            {:deliver, response, _peer} -> response
+            {:error, reason} -> {:error, reason}
+          after
+            10 -> {:error, {:timeout, :await_response}}
+          end
+
+        assert response == {:error, {:timeout, :await_response}}
+      end)
+
+      assert log =~ "CoAP socket failed to decode udp packets"
+      assert log =~ "is not repeatable"
+      assert log =~ "from ip: \"#{peer_ip}\""
+      assert log =~ "port: #{peer_port}"
+      assert log =~ "data: #{inspect(data, binaries: :as_binary)}"
+    end
+
+    test "CaseClauseError" do
+      peer_ip = "127.0.0.1"
+      peer_port = 5830
+
+      {:ok, server} = SocketServer.start_link([{GenericServer, FakeEndpoint}, peer_port])
+
+      data =
+        <<100, 69, 0, 1, 83, 61, 239, 152, 68, 244, 81, 253, 46, 100, 69, 0, 1, 83, 61, 239, 152, 68, 244, 81, 253, 46>>
+
+      log = capture_log(fn ->
+        send(server, {:udp, :socket, peer_ip, peer_port, data})
+
+        response =
+          receive do
+            {:deliver, response, _peer} -> response
+            {:error, reason} -> {:error, reason}
+          after
+            10 -> {:error, {:timeout, :await_response}}
+          end
+
+        assert response == {:error, {:timeout, :await_response}}
+      end)
+
+      assert log =~ "CoAP socket failed to decode udp packets"
+      assert log =~ "CaseClauseError"
+      assert log =~ "from ip: \"#{peer_ip}\""
+      assert log =~ "port: #{peer_port}"
+      assert log =~ "data: #{inspect(data, binaries: :as_binary)}"
+    end
+  end
+end