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/message.ex

@@ -4,7 +4,6 @@ defmodule CoAP.Message do
   alias CoAP.Multipart
 
   # import Logger, only: [debug: 1]
-  require Logger
 
   # @max_block_size 1024
 

lib/coap/message_option.ex

@@ -73,7 +73,6 @@ defmodule CoAP.MessageOption do
     use CoAP.MessageOption.UnsignedOptions
 
     def to_tuple(option_id, value) do
-      IO.inspect({option_id, value}, label: "Option")
       decode_option(option_id, value)
     end
 

lib/coap/message_options.ex

@@ -1,8 +1,6 @@
 defmodule CoAP.MessageOptions do
   # @payload_marker 0xFF
 
-  require Logger
-
   @doc """
 
   Examples
@@ -55,26 +53,10 @@ defmodule CoAP.MessageOptions do
       # key becomes the next delta_sum
       case tail do
         <<value::binary-size(length), rest::binary>> ->
-          option_list =
-            case CoAP.MessageOption.decode(key, value) do
-              {nil, _} = option ->
-                throw({:error, {:invalid_option, option}})
-                # IO.inspect(option, label: "Invalid option")
-                # option_list
-
-              option ->
-                append_option(option, option_list)
-            end
-
-          decode(rest, key, option_list)
+          decode(rest, key, append_option(CoAP.MessageOption.decode(key, value), option_list))
 
         <<>> ->
-          option = CoAP.MessageOption.decode(key, <<>>)
-          option_list = append_option(option, option_list)
-          decode(<<>>, key, option_list)
-
-        <<rest::binary>> ->
-          decode(rest, key, option_list)
+          decode(<<>>, key, append_option(CoAP.MessageOption.decode(key, <<>>), option_list))
       end
     end
 
@@ -119,7 +101,7 @@ defmodule CoAP.MessageOptions do
           [{key, values ++ [value]} | options]
 
         false ->
-          throw({:error, {:key_not_repeatable, key}})
+          throw({:error, "#{key} is not repeatable"})
       end
     end
 

lib/coap/socket_server.ex

@@ -91,30 +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}
-  catch
-    e ->
-      warn(
-        "CoAP socket failed to decode udp packets because #{inspect e} from " <>
-          "ip: #{inspect(peer_ip)}, port: #{inspect(peer_port)}.  data: #{inspect(data, limit: :infinity, print_limit: :infinity)}"
-      )
-      {:noreply, state}
   end
 
   # Deliver messages to be sent to a peer
@@ -139,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(
@@ -150,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,
@@ -173,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}
@@ -246,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/client_test.exs

@@ -118,43 +118,6 @@ defmodule CoAP.ClientTest do
     assert byte_size(response.payload) == 2048
   end
 
-
-  defmodule BadOptionFakeEndpoint do
-    def request(message) do
-      # path should have api in it
-      # params should be empty
-
-      payload = <<100, 69, 0, 1, 104, 112, 179, 19, 68, 244, 81, 253, 46, 100, 69, 0, 1, 104, 112, 179, 19, 68, 244, 81, 253, 46>>
-
-      %Message{
-        type: :con,
-        code_class: 2,
-        code_detail: 5,
-        message_id: message.message_id,
-        token: message.token,
-        payload: payload
-      }
-    end
-  end
-
-  @tag :wip
-  test "get a response with bad option" do
-    # pass a module that has a response
-    # endpoint = Task.new(fn)
-    # start a socket server
-    CoAP.SocketServer.start([{CoAP.Adapters.GenericServer, BadOptionFakeEndpoint}, @port + 5])
-    payload = <<100, 69, 0, 1, 104, 112, 179, 19, 68, 244, 81, 253, 46, 100, 69, 0, 1, 104, 112, 179, 19, 68, 244, 81, 253, 46>>
-
-
-    # make a request with the client
-    response = CoAP.Client.get("coap://localhost:#{@port + 5}/api", payload)
-
-    assert response.message_id > 0
-    assert response.code_class == 2
-    assert response.code_detail == 5
-    assert response.payload == "dE\0\x01hp\xB3\x13D\xF4Q\xFD.dE\0\x01hp\xB3\x13D\xF4Q\xFD."
-  end
-
   # test "get a timed out response" do
   #   # pass a module that has a response
   #   # endpoint = Task.new(fn)

test/coap/socket_server_test.exs

@@ -1,19 +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>>
 
-  test "handle_info/2" do
-    connection_id = {{10, 148, 21, 176}, 5683, <<252, 1, 46, 56>>}
-    state = %{connections: %{connection_id => self()}, monitors: %{}, endpoint: %{}, config: %{}}
+      log = capture_log(fn ->
+        send(server, {:udp, :socket, peer_ip, peer_port, data})
 
-    message =
-      {:udp, self(), {10, 148, 21, 176}, 5683,
-       <<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>>}
+        response =
+          receive do
+            {:deliver, response, _peer} -> response
+            {:error, reason} -> {:error, reason}
+          after
+            10 -> {:error, {:timeout, :await_response}}
+          end
 
-    resp = SocketServer.handle_info(message, state)
+        assert response == {:error, {:timeout, :await_response}}
+      end)
 
-    assert resp == {:noreply, state}
+      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