Don't close candidate on eperm

lib/ex_ice/priv/checklist.ex

@@ -29,9 +29,6 @@ defmodule ExICE.Priv.Checklist do
   def get_valid_pair(checklist) do
     checklist
     |> Stream.map(fn {_id, pair} -> pair end)
-    # pair might have been marked as failed if the associated
-    # local candidate has been closed
-    |> Stream.filter(fn pair -> pair.state == :succeeded end)
     |> Stream.filter(fn pair -> pair.valid? end)
     |> Enum.sort_by(fn pair -> pair.priority end, :desc)
     |> Enum.at(0)
@@ -89,7 +86,7 @@ defmodule ExICE.Priv.Checklist do
   def close_candidate(checklist, local_cand) do
     Enum.reduce(checklist, {[], checklist}, fn {pair_id, pair}, {failed_pair_ids, checklist} ->
       if pair.local_cand_id == local_cand.base.id and pair.state != :failed do
-        checklist = Map.put(checklist, pair_id, %{pair | state: :failed})
+        checklist = Map.put(checklist, pair_id, %{pair | state: :failed, valid?: false})
         {[pair_id | failed_pair_ids], checklist}
       else
         {failed_pair_ids, checklist}

lib/ex_ice/priv/ice_agent.ex

@@ -3013,7 +3013,8 @@ defmodule ExICE.Priv.ICEAgent do
         pair = %CandidatePair{
           pair
           | packets_discarded_on_send: pair.packets_discarded_on_send + 1,
-            bytes_discarded_on_send: pair.bytes_discarded_on_send + byte_size(raw_req)
+            bytes_discarded_on_send: pair.bytes_discarded_on_send + byte_size(raw_req),
+            state: :failed
         }
 
         put_in(ice_agent.checklist[pair.id], pair)
@@ -3085,7 +3086,7 @@ defmodule ExICE.Priv.ICEAgent do
           """)
 
           ice_agent = put_in(ice_agent.local_cands[local_cand.base.id], local_cand)
-          ice_agent = close_candidate(ice_agent, local_cand)
+
           {:error, ice_agent}
         end
     end

test/priv/ice_agent_test.exs

@@ -1245,6 +1245,8 @@ defmodule ExICE.Priv.ICEAgentTest do
     end
   end
 
+  @conn_check_byte_size 92
+
   describe "connectivity check" do
     setup do
       ice_agent =
@@ -1563,6 +1565,50 @@ defmodule ExICE.Priv.ICEAgentTest do
 
       assert ice_agent.state == :completed
     end
+
+    test "candidate fails to send conn check" do
+      # 1. replace candidate with the mock one that always fails to send data
+      # 2. assert that after unsuccessful conn check sending, ice_agent move conn pair to the failed state
+
+      ice_agent =
+        ICEAgent.new(
+          controlling_process: self(),
+          role: :controlling,
+          if_discovery_module: IfDiscovery.Mock,
+          transport_module: Transport.Mock
+        )
+        |> ICEAgent.set_remote_credentials("someufrag", "somepwd")
+        |> ICEAgent.gather_candidates()
+        |> ICEAgent.add_remote_candidate(@remote_cand)
+        |> ICEAgent.end_of_candidates()
+
+      assert ice_agent.gathering_state == :complete
+
+      # replace candidate with the mock one
+      [local_cand] = Map.values(ice_agent.local_cands)
+      mock_cand = %Candidate.Mock{base: local_cand.base}
+      ice_agent = %{ice_agent | local_cands: %{mock_cand.base.id => mock_cand}}
+
+      # try to send conn check
+      ice_agent = ICEAgent.handle_ta_timeout(ice_agent)
+
+      assert ice_agent.state == :checking
+
+      # assert that the candidate pair has moved to a failed state
+      # and that the state was updated after the packet was discarded
+      assert [
+               %{
+                 state: :failed,
+                 valid?: false,
+                 packets_discarded_on_send: 1,
+                 bytes_discarded_on_send: @conn_check_byte_size
+               }
+             ] = Map.values(ice_agent.checklist)
+
+      ice_agent = ICEAgent.handle_ta_timeout(ice_agent)
+
+      assert ice_agent.state == :failed
+    end
   end
 
   describe "connectivity check with aggressive nomination" do
@@ -2492,8 +2538,8 @@ defmodule ExICE.Priv.ICEAgentTest do
   test "candidate fails to send data when ice is connected" do
     # 1. make ice agent connected
     # 2. replace candidate with the mock one that always fails to send  data
-    # 3. assert that after unsuccessful data sending, ice_agent moves to the failed state
-    # as there are no other pairs
+    # 3. assert that after unsuccessful data sending, ice_agent doesn't move to the failed state
+    # even when there is only one pair
 
     ice_agent =
       ICEAgent.new(
@@ -2519,10 +2565,18 @@ defmodule ExICE.Priv.ICEAgentTest do
     # try to send some data
     ice_agent = ICEAgent.send_data(ice_agent, "somedata")
 
-    # assert that local cand has been closed and the agent moved to the failed state
-    assert [%{base: %{closed?: true}}] = Map.values(ice_agent.local_cands)
-    assert ice_agent.state == :failed
-    assert [%{state: :failed}] = Map.values(ice_agent.checklist)
+    # assert that the local candidate hasn't been closed and that the agent hasn't moved to a failed state
+    assert [%{base: %{closed?: false}}] = Map.values(ice_agent.local_cands)
+    assert ice_agent.state == :connected
+
+    # assert that the local candidate's state was updated after the packet was discarded
+    assert [
+             %{
+               state: :succeeded,
+               packets_discarded_on_send: 1,
+               bytes_discarded_on_send: 8
+             }
+           ] = Map.values(ice_agent.checklist)
   end
 
   test "relay connection" do