From c19a271efc63c0d1e41fa4e8c466fc3a7776297e Mon Sep 17 00:00:00 2001 From: Daniel Urumov Date: Fri, 11 Sep 2026 20:41:41 +0300 Subject: [PATCH 1/9] chore(deps): refresh the hackney chain and dev tooling hackney 4.7.2 -> 4.7.4, h2 0.11.0 -> 0.12.0, webtransport 0.4.4 -> 0.4.5 and quic 1.8.0 -> 1.8.2 are the HTTP stack behind tracker announces and BEP 19 web seeds, so the bump is validated by the full local CI gate, not just a compile. dialyxir and ex_doc are dev-only. Co-authored-by: Cursor --- mix.lock | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/mix.lock b/mix.lock index 769ca8a..e1a37d5 100644 --- a/mix.lock +++ b/mix.lock @@ -3,14 +3,14 @@ "bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"}, "certifi": {:hex, :certifi, "2.17.0", "835748414307e15e05b17d0e518190228ce648b08d569a5cc93a85a40f3e5c9b", [:rebar3], [], "hexpm", "8122798a17f0293c80daada25d0f81c7f4d708c73fef782c7c9b1950e26e4d21"}, "credo": {:hex, :credo, "1.7.19", "cc52129665fc7c15143d47838fda0f9cd6dac9ceced7bf4da6f85fcbfe64b12a", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "2d8bc95d5a7bb99dd2613621d4f08c6a3575c3fd4b62e6a2b48a100352a557b8"}, - "dialyxir": {:hex, :dialyxir, "1.4.7", "dda948fcee52962e4b6c5b4b16b2d8fa7d50d8645bbae8b8685c3f9ecb7f5f4d", [:mix], [{:erlex, ">= 0.2.8", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "b34527202e6eb8cee198efec110996c25c5898f43a4094df157f8d28f27d9efe"}, + "dialyxir": {:hex, :dialyxir, "1.4.8", "7ef671a8aff9948b091d8c30f09467fbb16e77305cda451bce48109a0f5e021c", [:mix], [{:erlex, ">= 0.2.8", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "cbd5a851571e5dfeb32aaf2e840bfa98b7864cb3071bf2ef5d95d1276b12e072"}, "earmark_parser": {:hex, :earmark_parser, "1.4.46", "67607a0532e810c6f630a515c548d0b24949643f168cc556303bee4cf96105c7", [:mix], [], "hexpm", "9c44636e8a1c68c62f526b2dcd85d941dbbcee7ab82cf64ba06ce28bef8e89f5"}, "erlex": {:hex, :erlex, "0.2.9", "7debbbaa9f4f368b8cd648983e0f1d7963028508e9c59e9d4ed504e94ef52a55", [:mix], [], "hexpm", "8cfffc0ec7159e6d73de2ab28a588064de80f88b2798d5cbe4482cbbc200178b"}, "ex_doc": {:hex, :ex_doc, "0.40.4", "66f2e42bf588594d5a8aab31cad87f2ddad09d0da1b1a2f379340ec2c2e497cb", [:mix], [{:earmark_parser, "~> 1.4.46", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "6222b9e423d76584ee34df2c82a5ed72c2d53dc153f7f483ad28b378694186cc"}, "excoveralls": {:hex, :excoveralls, "0.18.5", "e229d0a65982613332ec30f07940038fe451a2e5b29bce2a5022165f0c9b157e", [:mix], [{:castore, "~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "523fe8a15603f86d64852aab2abe8ddbd78e68579c8525ae765facc5eae01562"}, "file_system": {:hex, :file_system, "1.1.1", "31864f4685b0148f25bd3fbef2b1228457c0c89024ad67f7a81a3ffbc0bbad3a", [:mix], [], "hexpm", "7a15ff97dfe526aeefb090a7a9d3d03aa907e100e262a0f8f7746b78f8f87a5d"}, - "h2": {:hex, :h2, "0.11.0", "b8c9f7c86f400582c5cbed59f79c0efd6cf49ad6478c210bede91d2d15019b14", [:rebar3], [], "hexpm", "c61f24361d0dc76375eaf35c2bf0e01235d81b7685538df571db036467fac358"}, - "hackney": {:hex, :hackney, "4.7.2", "8f712c94d90220cc21ea8049172c2e760ffc41737060667603aa1aef52ab9a4d", [:rebar3], [{:certifi, "~> 2.17.0", [hex: :certifi, repo: "hexpm", optional: false]}, {:h2, "~> 0.11.0", [hex: :h2, repo: "hexpm", optional: false]}, {:idna, "~> 7.1.0", [hex: :idna, repo: "hexpm", optional: false]}, {:mimerl, "~> 1.4", [hex: :mimerl, repo: "hexpm", optional: false]}, {:parse_trans, "3.4.2", [hex: :parse_trans, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "~> 1.1.0", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}, {:webtransport, "~> 0.4.3", [hex: :webtransport, repo: "hexpm", optional: false]}], "hexpm", "fad03e527a6b1811ef3362ebe9d6d826d988927c18afc71358c35bf2098110e4"}, + "h2": {:hex, :h2, "0.12.0", "f393539ee2728f8118fb2024b6d5f3e2c45e40ceb31b18b4e9bf5e50d028f80f", [:rebar3], [], "hexpm", "beaafc93c54cdc5d623247334d3970cdf4bc66b6b8b296b74ba1d7c7513c3dfc"}, + "hackney": {:hex, :hackney, "4.7.4", "8fe2ddaa3ca27de99d68e682d72b66d07d2331da680f77c8000580a0122c69e6", [:rebar3], [{:certifi, "~> 2.17.0", [hex: :certifi, repo: "hexpm", optional: false]}, {:h2, "~> 0.12.0", [hex: :h2, repo: "hexpm", optional: false]}, {:idna, "~> 7.1.0", [hex: :idna, repo: "hexpm", optional: false]}, {:mimerl, "~> 1.5", [hex: :mimerl, repo: "hexpm", optional: false]}, {:parse_trans, "3.4.2", [hex: :parse_trans, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "~> 1.1.0", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}, {:webtransport, "~> 0.4.5", [hex: :webtransport, repo: "hexpm", optional: false]}], "hexpm", "d07d7e1358353ab6cc75132f058c155287f3e013d43f709fbb79d79eeab98195"}, "httpoison": {:hex, :httpoison, "3.0.0", "8566a933bb9175236d1ec335978445b67cd1f5b5d3ead6ca4b80be469d41f5d9", [:mix], [{:hackney, "~> 4.0", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm", "9130197b7658901c493d6fcfb842fb9676300fa8a6c8ed058c8889cf1a77f3c2"}, "idna": {:hex, :idna, "7.1.0", "1067a13043538129602d2f2ce6899d8713125c7d19734aa557ce2e3ea55bd4f1", [:rebar3], [], "hexpm", "6ae959a025bf36df61a8cab8508d9654891b5426a84c44d82deaffd6ddf8c71f"}, "jason": {:hex, :jason, "1.4.5", "2e3a008590b0b8d7388c20293e9dcc9cf3e5d642fd2a114e4cbbb52e595d940a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0 or ~> 3.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "b0c823996102bcd0239b3c2444eb00409b72f6a140c1950bc8b457d836b30684"}, @@ -25,10 +25,10 @@ "parse_trans": {:hex, :parse_trans, "3.4.2", "c352ddc1a0d5e54f9b1654d45f9c432eef76f9cea371c55ddff769ef688fdb74", [:rebar3], [], "hexpm", "4c25347de3b7c35732d32e69ab43d1ceee0beae3f3b3ade1b59cbd3dd224d9ca"}, "propcheck": {:hex, :propcheck, "1.5.0", "df6f554a97eba4f92c3e4bc7758c8480e8bc401188af32cfe09641e81d8a55ba", [:mix], [{:libgraph, "~> 0.13", [hex: :libgraph, repo: "hexpm", optional: false]}, {:proper, "~> 1.5", [hex: :proper, repo: "hexpm", optional: false]}], "hexpm", "3388aa7909277faed166dfced39fad6ecae3c3be064352d37646be459fc80e09"}, "proper": {:git, "https://github.com/proper-testing/proper.git", "eaa5eee5319478954e80bc7c16614d507f7842b4", [ref: "eaa5eee5319478954e80bc7c16614d507f7842b4"]}, - "quic": {:hex, :quic, "1.8.0", "7b074fc6f8a13b4a81e4e11254bf061db61c5c07b4366d568a79a38c5e8bb5f3", [:rebar3], [], "hexpm", "6b1f8f08a45412ac503bc6771efb8edc696bb14079bc48d2ca960b92933da8a4"}, + "quic": {:hex, :quic, "1.8.2", "c315176d2c4fad0725e52f2a8033b96d01c4fecfc7e6a7333615ff10041142a5", [:rebar3], [], "hexpm", "274d2f41ee9c00d8d6415248df9fb5637381fe69b5387771dc37e91038a65479"}, "recon": {:hex, :recon, "2.5.6", "9052588e83bfedfd9b72e1034532aee2a5369d9d9343b61aeb7fbce761010741", [:mix, :rebar3], [], "hexpm", "96c6799792d735cc0f0fd0f86267e9d351e63339cbe03df9d162010cefc26bb0"}, "sobelow": {:hex, :sobelow, "0.15.0", "b067d7f8522a9d758fa89cb2bfcbab7ad72c45a0993cb958c989c6fd956fdd56", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "24a800e2d7fa8c3bd21561b6ad8ad4745ed726a09fd606598981d9048708da98"}, "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.7", "354c321cf377240c7b8716899e182ce4890c5938111a1296add3ec74cf1715df", [:make, :mix, :rebar3], [], "hexpm", "fe4c190e8f37401d30167c8c405eda19469f34577987c76dde613e838bbc67f8"}, "stream_data": {:hex, :stream_data, "1.4.0", "026f929db613aabea6208012ae9b8970d3fd5f88b3bdf26831bc536f98c42036", [:mix], [], "hexpm", "2b0ee3a340dcce1c8cf6302a763ee757d1e01c54d6e16d9069062509d68b1dc9"}, - "webtransport": {:hex, :webtransport, "0.4.4", "714180b340fbd30a7d666f3f593a612186ff1ba444566b3e8c9ce37fb2950638", [:rebar3], [{:h2, "~> 0.11.0", [hex: :h2, repo: "hexpm", optional: false]}, {:quic, "~> 1.8.0", [hex: :quic, repo: "hexpm", optional: false]}], "hexpm", "80a6641e0f6628938ceee4c9d7a4f6d48a6089890da1ae3288ca8dab13a6005e"}, + "webtransport": {:hex, :webtransport, "0.4.5", "0e387202bbe707389fe81373ef8c56faa9d5aa321bb4800fa4765ee7c1399785", [:rebar3], [{:h2, "~> 0.12", [hex: :h2, repo: "hexpm", optional: false]}, {:quic, "~> 1.8.0", [hex: :quic, repo: "hexpm", optional: false]}], "hexpm", "bcb512239e48e551d5bd5c667312a9a7de4f29b89d84b1d33c5af44e1f3f730d"}, } From 4c7ac00db4a02ec3e8e680ec03c2284cc8efac63 Mon Sep 17 00:00:00 2001 From: Daniel Urumov Date: Fri, 11 Sep 2026 20:41:41 +0300 Subject: [PATCH 2/9] fix(dial): stop recording post-handshake local failures as unreachable :add_peer_failed is emitted by add_peer/6 only after TCP connect and the full BEP 3 handshake have completed -- we already hold the peer's id and reserved bytes -- and means Swarm.add/4 could not start the peer supervisor. That is the same class as :socket_handoff_failed, which was moved out of the reachability signals in an earlier pass; the sibling one step earlier on the same path was missed. As a result it counted as a DialStats family failure, depressing the measured v4/v6 yield that drives the per-family dial throttle, and wrote a DialBackoff row toward @hard_fail_threshold. Live, one torrent held 61 of its 62 known endpoints sticky-blocked with 4 peers connected. The catch-all also hid the outcome that actually produced it: peer supervisors register under {peer_id, hash}, so a fake-peer farm handing one id out from many IPs loses the race at start_child. That is :already_connected, which every stage downstream already understood. Co-authored-by: Cursor --- .../acceptor/connection/handshakes.ex | 24 ++++++++++++++----- lib/elixir_torrent/peer/dial_backoff.ex | 16 +++++++++---- test/handshakes_coverage_batch_test.exs | 4 ++-- test/handshakes_test.exs | 4 ++++ 4 files changed, 36 insertions(+), 12 deletions(-) diff --git a/lib/elixir_torrent/acceptor/connection/handshakes.ex b/lib/elixir_torrent/acceptor/connection/handshakes.ex index ccfe305..d86de1c 100644 --- a/lib/elixir_torrent/acceptor/connection/handshakes.ex +++ b/lib/elixir_torrent/acceptor/connection/handshakes.ex @@ -378,14 +378,17 @@ defmodule Acceptor.Connection.Handshakes do defp increment_failure(failures, reason), do: Map.update(failures, reason, 1, &(&1 + 1)) - # Dial outcome → Peer.DialStats family bump. :socket_handoff_failed is emitted - # only after TCP/uTP connect and the BitTorrent handshake succeeded; the peer - # proved reachable and local OTP churn (register/handoff/activate) failed. - # Count that as a family :ok so v4/v6 yield isn't poisoned by our own process - # wiring. :already_connected / :not_connectable are neutral (:skip). + # Dial outcome → Peer.DialStats family bump. :socket_handoff_failed and + # :add_peer_failed are both emitted only after TCP/uTP connect and the + # BitTorrent handshake succeeded; the peer proved reachable and local OTP + # churn (supervisor start / register / handoff / activate) failed. Count them + # as a family :ok so v4/v6 yield isn't poisoned by our own process wiring. + # :already_connected / :not_connectable are neutral (:skip). @doc false @spec dial_reachability_outcome(term()) :: :ok | :skip | :fail - def dial_reachability_outcome(:socket_handoff_failed), do: :ok + def dial_reachability_outcome(reason) + when reason in [:socket_handoff_failed, :add_peer_failed], + do: :ok def dial_reachability_outcome(reason) when reason in [:already_connected, :not_connectable], do: :skip @@ -946,6 +949,15 @@ defmodule Acceptor.Connection.Handshakes do safe_close(socket) {:error, :max_peers} + # Peer supervisors are registered under {peer_id, hash}, so a second + # endpoint presenting a peer id we already hold loses the race here. + # `already_connected?/2` screens the batch, but up to @batch dials run + # concurrently and a fake-peer farm hands the same id out from many IPs, + # so the duplicate is only observable at registration. + {:error, {:already_started, _pid}} -> + safe_close(socket) + {:error, :already_connected} + _ -> safe_close(socket) {:error, :add_peer_failed} diff --git a/lib/elixir_torrent/peer/dial_backoff.ex b/lib/elixir_torrent/peer/dial_backoff.ex index bd81c13..838cf59 100644 --- a/lib/elixir_torrent/peer/dial_backoff.ex +++ b/lib/elixir_torrent/peer/dial_backoff.ex @@ -61,10 +61,18 @@ defmodule Peer.DialBackoff do @sticky_reasons [:churn | @hard_failures] # These outcomes don't reflect endpoint reachability — don't count them toward - # the fail threshold and don't write a block row. :socket_handoff_failed means - # connect+handshake succeeded and only local handoff failed; Endpoints already - # records :churn when registration happened, so DialBackoff must not double-block. - @non_reachability_reasons [:already_connected, :not_connectable, :socket_handoff_failed] + # the fail threshold and don't write a block row. :socket_handoff_failed and + # :add_peer_failed both mean connect+handshake succeeded and only a local step + # failed (supervisor start, then handoff); Endpoints already records :churn + # when registration happened, so DialBackoff must not double-block. Blocking on + # them wrote off endpoints we had just spoken BEP 3 to: live, one torrent held + # 61 of 62 endpoints sticky-blocked with 4 peers connected. + @non_reachability_reasons [ + :already_connected, + :not_connectable, + :socket_handoff_failed, + :add_peer_failed + ] @spec child_spec(term()) :: Supervisor.child_spec() def child_spec(_) do diff --git a/test/handshakes_coverage_batch_test.exs b/test/handshakes_coverage_batch_test.exs index 80d74d9..5cf3035 100644 --- a/test/handshakes_coverage_batch_test.exs +++ b/test/handshakes_coverage_batch_test.exs @@ -394,8 +394,8 @@ defmodule HandshakesCoverageBatchTest do assert {ok_count, failures, failed} = Handshakes.dial_peers([peer, peer], hash) assert ok_count == 1 - assert Map.get(failures, :add_peer_failed) == 1 - assert Enum.any?(failed, fn {^peer, :add_peer_failed} -> true end) + assert Map.get(failures, :already_connected) == 1 + assert Enum.any?(failed, fn {^peer, :already_connected} -> true end) assert_receive :dial_server_done, @timeout send(server.pid, close_gate) :gen_tcp.close(listen) diff --git a/test/handshakes_test.exs b/test/handshakes_test.exs index 120de8a..a17b08e 100644 --- a/test/handshakes_test.exs +++ b/test/handshakes_test.exs @@ -109,6 +109,10 @@ defmodule HandshakesTest do assert Handshakes.dial_reachability_outcome(:socket_handoff_failed) == :ok end + test "add_peer_failed counts as family ok (endpoint completed BT handshake)" do + assert Handshakes.dial_reachability_outcome(:add_peer_failed) == :ok + end + test "already_connected and not_connectable are neutral skips" do assert Handshakes.dial_reachability_outcome(:already_connected) == :skip assert Handshakes.dial_reachability_outcome(:not_connectable) == :skip From 9dd3b5aa6f0dd176fdd583b0b4989063b18fa581 Mon Sep 17 00:00:00 2001 From: Daniel Urumov Date: Fri, 11 Sep 2026 20:41:42 +0300 Subject: [PATCH 3/9] fix(tracker): name the torrent and tracker in announce failures The line read 'request failure reason: ' with no info_hash and no announce URL, which made the most actionable message in the log useless: a tracker's own bencoded failure reason asks the user to re-add a torrent it never named. Logging moves out of retry_interval_seconds/2, which is now pure, into parallel_tracker_error/4, where the request ref has been resolved back to its announce URL. Two classification gaps went with it. HTTPoison surfaces hackney's connect timeout as {:timeout, {:gen_statem, :call, [pid, :connect, 8000]}}, so the connect-timeout case expected_tracker_failure_reason?/1 exists to cover never matched -- 42 of 89 warnings in a 21-minute window. HTTP 4xx/5xx was not classified at all, though a 403/404/521 from a public announce-list entry is a dead tracker and BEP 12 already fails over tiers. Both are expected now; a bencoded failure reason still warns. Co-authored-by: Cursor --- lib/elixir_torrent/peer_discovery/announce.ex | 101 +++++++++++++----- ...discovery_announce_coverage_batch_test.exs | 29 ++++- test/peer_discovery_announce_test.exs | 22 ++++ 3 files changed, 126 insertions(+), 26 deletions(-) diff --git a/lib/elixir_torrent/peer_discovery/announce.ex b/lib/elixir_torrent/peer_discovery/announce.ex index b6d7f01..2c67cf7 100644 --- a/lib/elixir_torrent/peer_discovery/announce.ex +++ b/lib/elixir_torrent/peer_discovery/announce.ex @@ -637,17 +637,18 @@ defmodule PeerDiscovery.Announce do def handle_info({ref, %Tracker.Error{retry_in: retry_in} = error}, state) when not is_nil(retry_in) do - timeout = retry_interval_seconds(retry_in, error.reason) - {:noreply, parallel_tracker_error(state, ref, timeout)} + timeout = retry_interval_seconds(retry_in) + {:noreply, parallel_tracker_error(state, ref, timeout, error.reason)} end def handle_info({ref, %Tracker.Error{reason: reason}}, state) do - timeout = retry_interval_seconds(nil, reason) - {:noreply, parallel_tracker_error(state, ref, timeout)} + timeout = retry_interval_seconds(nil) + {:noreply, parallel_tracker_error(state, ref, timeout, reason)} end def handle_info({ref, _}, state) do - {:noreply, parallel_tracker_error(state, ref, Tracker.default_failure_interval())} + timeout = Tracker.default_failure_interval() + {:noreply, parallel_tracker_error(state, ref, timeout, :unexpected_reply)} end @spec start_parallel_tier(%__MODULE__{}, non_neg_integer(), [String.t()]) :: %__MODULE__{} @@ -1131,29 +1132,43 @@ defmodule PeerDiscovery.Announce do end end - @spec parallel_tracker_error(%__MODULE__{}, reference(), non_neg_integer()) :: %__MODULE__{} - defp parallel_tracker_error(%__MODULE__{} = state, ref, timeout_seconds) do + @spec parallel_tracker_error(%__MODULE__{}, reference(), non_neg_integer(), term()) :: + %__MODULE__{} + defp parallel_tracker_error(%__MODULE__{} = state, ref, timeout_seconds, reason) do {meta, requests} = Map.pop(state.requests, ref) case meta do nil -> state - {:scrape, _announce} -> + {:scrape, announce} -> # Scrape failures are non-fatal — the tracker's announce endpoint may # still work. Drop the request, leave parallel state and `disabled` # untouched. Retry on next @scrape_interval_ms tick. + log_scrape_failure(state.hash, announce, reason) %{state | requests: requests} {announce, tier_index, _tracker_index} -> - state - |> Map.put(:requests, requests) - |> Map.update!(:peers, &Map.delete(&1, announce)) - |> put_tracker_retry_after(announce, timeout_seconds) - |> dec_tier_batch(tier_index) + log_tracker_failure(state.hash, announce, reason) + drop_failed_announce(state, requests, announce, tier_index, timeout_seconds) end end + @spec drop_failed_announce( + %__MODULE__{}, + map(), + String.t(), + non_neg_integer(), + non_neg_integer() + ) :: %__MODULE__{} + defp drop_failed_announce(state, requests, announce, tier_index, timeout_seconds) do + state + |> Map.put(:requests, requests) + |> Map.update!(:peers, &Map.delete(&1, announce)) + |> put_tracker_retry_after(announce, timeout_seconds) + |> dec_tier_batch(tier_index) + end + @spec put_tracker_retry_after(%__MODULE__{}, String.t(), non_neg_integer()) :: %__MODULE__{} defp put_tracker_retry_after(%__MODULE__{} = state, announce, timeout_seconds) do deadline_ms = System.monotonic_time(:millisecond) + timeout_seconds * 1_000 @@ -1529,30 +1544,50 @@ defmodule PeerDiscovery.Announce do end end - @spec retry_interval_seconds(term(), term()) :: non_neg_integer() - defp retry_interval_seconds(retry_in, _reason) when is_integer(retry_in) and retry_in >= 0, - do: retry_in + @spec retry_interval_seconds(term()) :: non_neg_integer() + defp retry_interval_seconds(retry_in) when is_integer(retry_in) and retry_in >= 0, do: retry_in - defp retry_interval_seconds(retry_in, _reason) when retry_in in ["never", :never], do: 0 + defp retry_interval_seconds(retry_in) when retry_in in ["never", :never], do: 0 - defp retry_interval_seconds(retry_in, _reason) when is_binary(retry_in) do + defp retry_interval_seconds(retry_in) when is_binary(retry_in) do case parse_retry_in_seconds(retry_in) do nil -> Tracker.default_failure_interval() n -> n end end - defp retry_interval_seconds(_, reason) do - # Dead public trackers (NXDOMAIN / black-hole UDP / connect timeouts) are the - # common case in real announce-lists — BEP 12 already fails over tiers. Warn - # only on unexpected reasons so server.log stays readable under CGNAT churn. + defp retry_interval_seconds(_), do: Tracker.default_failure_interval() + + # Logged from `parallel_tracker_error/4`, where the request ref has been + # resolved back to its announce URL. The previous "request failure reason: …" + # line carried neither the info_hash nor the tracker, so nothing in it could + # be acted on — worst for the tracker's own bencoded `failure reason` text, + # which asks the *user* to do something ("Please redownload the torrent…") + # about a torrent it never named. + @spec log_tracker_failure(Torrent.hash(), String.t(), term()) :: :ok + defp log_tracker_failure(hash, announce, reason) do + message = + "[tracker] announce_failed hash=#{Torrent.hex_encoded_hash(hash)} " <> + "announce=#{announce} reason=#{inspect(reason)}" + + # Dead public trackers are the common case in real announce-lists and BEP 12 + # already fails over tiers, so only unexpected reasons warn — otherwise + # server.log is unreadable under CGNAT churn. if expected_tracker_failure_reason?(reason) do - Logger.debug("request failure reason: #{inspect(reason)}") + Logger.debug(message) else - Logger.warning("request failure reason: #{inspect(reason)}") + Logger.warning(message) end + end - Tracker.default_failure_interval() + # BEP 48 scrape is an optional side channel; its failure never blocks announce, + # so it stays at :debug regardless of reason. + @spec log_scrape_failure(Torrent.hash(), String.t(), term()) :: :ok + defp log_scrape_failure(hash, announce, reason) do + Logger.debug( + "[tracker] scrape_failed hash=#{Torrent.hex_encoded_hash(hash)} " <> + "announce=#{announce} reason=#{inspect(reason)}" + ) end @doc false @@ -1571,6 +1606,22 @@ defmodule PeerDiscovery.Announce do do: true def expected_tracker_failure_reason?({:nxdomain, _}), do: true + + # HTTPoison surfaces hackney's connect timeout as the `gen_statem` call that + # timed out, not as a bare `:timeout`, so the connect-timeout case this list + # exists to cover never matched and every one of them warned instead. + def expected_tracker_failure_reason?({:timeout, {:gen_statem, :call, _}}), do: true + + # The endpoint answered, but not as a working tracker: 404 (gone), 403 + # (private/banned without a passkey) and Cloudflare's origin 5xx (521 "web + # server is down") are all routine in a public announce-list, and the + # per-tracker retry cooldown already spaces the retries. A bencoded + # `failure reason` string is deliberately not covered — that is the tracker + # speaking BEP 3 to us and can be actionable. + def expected_tracker_failure_reason?({:http_status, status}) + when is_integer(status) and status >= 400, + do: true + def expected_tracker_failure_reason?(_), do: false @spec extract_tiers(map()) :: [list(String.t())] diff --git a/test/peer_discovery_announce_coverage_batch_test.exs b/test/peer_discovery_announce_coverage_batch_test.exs index ba7a643..680a568 100644 --- a/test/peer_discovery_announce_coverage_batch_test.exs +++ b/test/peer_discovery_announce_coverage_batch_test.exs @@ -397,7 +397,34 @@ defmodule PeerDiscovery.AnnounceCoverageBatchTest do }) end) - assert log =~ "request failure reason" + # The line must name the torrent and the tracker — a bare reason is not + # actionable when a torrent carries dozens of announce URLs. + assert log =~ "[tracker] announce_failed" + assert log =~ "reason=:invalid_bencode" + assert log =~ "announce=#{announce}" + assert log =~ "hash=#{Torrent.hex_encoded_hash(state.hash)}" + end + + test "an expected dead-tracker reason stays out of the warning stream" do + ref = make_ref() + announce = "http://127.0.0.1:1/gone" + + state = + base_state( + requests: %{ref => {announce, 0, 0}}, + tier_batches: %{0 => 1} + ) + + log = + capture_log([level: :warning], fn -> + _ = + Announce.dispatch_task_message(state, { + ref, + %Error{reason: {:http_status, 404}, retry_in: nil} + }) + end) + + refute log =~ "announce_failed" end end diff --git a/test/peer_discovery_announce_test.exs b/test/peer_discovery_announce_test.exs index ce3556f..d657c08 100644 --- a/test/peer_discovery_announce_test.exs +++ b/test/peer_discovery_announce_test.exs @@ -1220,6 +1220,28 @@ defmodule PeerDiscoveryAnnounceTest do refute Announce.expected_tracker_failure_reason?(:invalid_bencode) end + test "expected_tracker_failure_reason?/1 covers hackney's wrapped connect timeout" do + # HTTPoison hands back the gen_statem call that timed out rather than a bare + # :timeout, so this shape is what a real connect timeout looks like. + assert Announce.expected_tracker_failure_reason?( + {:timeout, {:gen_statem, :call, [self(), :connect, 8000]}} + ) + end + + test "expected_tracker_failure_reason?/1 treats HTTP error statuses as dead trackers" do + for status <- [403, 404, 410, 500, 521, 522] do + assert Announce.expected_tracker_failure_reason?({:http_status, status}) + end + + refute Announce.expected_tracker_failure_reason?({:http_status, 200}) + + # A bencoded "failure reason" is the tracker talking to us and can be + # actionable (e.g. a private tracker asking for a passkey) — keep it loud. + refute Announce.expected_tracker_failure_reason?( + "Please redownload the torrent. PID system is active and pid was not found in the torrent" + ) + end + defp safe_stop(pid, timeout) do GenServer.stop(pid, :normal, timeout) catch From 0994d5d32c472563c1afa2d2759f82028bc7a5fe Mon Sep 17 00:00:00 2001 From: Daniel Urumov Date: Fri, 11 Sep 2026 21:15:16 +0300 Subject: [PATCH 4/9] fix(tracker): carry the term when a tracker task returns something unmatched Every shape a tracker task is meant to return is matched by an earlier handle_info clause, so the catch-all fires only on something nobody wrote. It used to drop the reply silently; the previous commit made it audible as :unexpected_reply, which named the category and no evidence -- the same gap that commit set out to close. It now carries the term. Co-authored-by: Cursor --- lib/elixir_torrent/peer_discovery/announce.ex | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/elixir_torrent/peer_discovery/announce.ex b/lib/elixir_torrent/peer_discovery/announce.ex index 2c67cf7..7d47922 100644 --- a/lib/elixir_torrent/peer_discovery/announce.ex +++ b/lib/elixir_torrent/peer_discovery/announce.ex @@ -646,9 +646,13 @@ defmodule PeerDiscovery.Announce do {:noreply, parallel_tracker_error(state, ref, timeout, reason)} end - def handle_info({ref, _}, state) do + # Every shape a tracker task is supposed to return is matched above, so this + # clause means we got something nobody wrote. Carry the term — naming only the + # category would leave the same "reason with no evidence" this module just + # stopped emitting. + def handle_info({ref, other}, state) do timeout = Tracker.default_failure_interval() - {:noreply, parallel_tracker_error(state, ref, timeout, :unexpected_reply)} + {:noreply, parallel_tracker_error(state, ref, timeout, {:unexpected_reply, other})} end @spec start_parallel_tier(%__MODULE__{}, non_neg_integer(), [String.t()]) :: %__MODULE__{} From ce40e6fc712f1a9740bcdb327ea7d2786ab35a55 Mon Sep 17 00:00:00 2001 From: Daniel Urumov Date: Sat, 12 Sep 2026 17:27:44 +0300 Subject: [PATCH 5/9] fix(tracker): disable a DNS-dead HTTP tracker instead of retrying it forever MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tracker.openbittorrent.com` has neither an A nor an AAAA record, and Hackney raises `:error, :badarg` out of its connect path for a name with no addresses rather than returning `:nxdomain`. `http_announce_bound/4` caught that and returned `%Error{reason: :badarg}` with no `retry_in`, which falls into the generic failure clause and gets the default retry interval — so the dead host was re-announced every cycle forever, six times an hour across four torrents. The UDP side has never had this problem: it resolves up front via `resolve_hosts/1` and answers `retry_in: "never"`, which `PeerDiscovery.Announce` uses to drop a tracker from the rotation for the session. That clause was written for the defunct rarbg trackers; openbittorrent is the same species of host and simply took a code path that could not reach it. The `:badarg` catch now resolves the host itself and claims `{:nxdomain, host}` with `retry_in: "never"` only when DNS genuinely has nothing. `:badarg` has other possible sources and a permanent disable is too destructive to apply on a guess, so anything else keeps the old opaque reason. Found one hour after the announce-failure logging started naming the reason: `reason=:badarg` is self-evidently not a network condition. It is the third instance of one root pattern — a library wraps or replaces the error shape our classification was written against, and the mismatch fails open into "retry forever / warn forever" rather than into an error, so nothing signals that the guard has gone dead. Co-authored-by: Cursor --- lib/elixir_torrent/tracker.ex | 31 +++++++++++++++++++++++++++++-- test/tracker_http_decode_test.exs | 22 ++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/lib/elixir_torrent/tracker.ex b/lib/elixir_torrent/tracker.ex index e167505..0dc104e 100644 --- a/lib/elixir_torrent/tracker.ex +++ b/lib/elixir_torrent/tracker.ex @@ -752,7 +752,7 @@ defmodule Tracker do rescue e in CaseClauseError -> if badarg_clause?(e) do - %Error{reason: :badarg} + badarg_error(url) else reraise e, __STACKTRACE__ end @@ -761,7 +761,30 @@ defmodule Tracker do %Error{reason: reason} :error, :badarg -> - %Error{reason: :badarg} + badarg_error(url) + end + end + + # Hackney raises `:badarg` out of its connect path — rather than returning + # `:nxdomain` — when the host has no address records at all. The UDP side + # resolves up front (`resolve_hosts/1`) and answers `retry_in: "never"`, which + # `PeerDiscovery.Announce` uses to drop a dead tracker from the rotation for + # the session. The HTTP side hands the URL straight to Hackney, so the same + # dead name came back as a bare `:badarg` with no `retry_in` and was + # re-announced on every cycle forever. Observed on the defunct + # `tracker.openbittorrent.com` (no A and no AAAA record): 6 announces an hour + # across 4 torrents, which is exactly what the "never" clause was written to + # stop for the rarbg trackers. + # + # `:badarg` has other possible sources, so confirm the DNS case before writing + # the tracker off; anything else keeps the old opaque reason. + @spec badarg_error(binary()) :: Error.t() + defp badarg_error(url) do + with host when is_binary(host) <- URI.parse(url).host, + {:error, _} <- resolve_hosts(host) do + %Error{reason: {:nxdomain, host}, retry_in: "never"} + else + _ -> %Error{reason: :badarg} end end @@ -859,6 +882,10 @@ defmodule Tracker do @spec http_hackney_opts_for_test(:inet | :inet6, :inet.ip_address() | nil) :: keyword() def http_hackney_opts_for_test(family, ip), do: http_hackney_opts(family, ip) + @doc false + @spec badarg_error_for_test(binary()) :: Error.t() + def badarg_error_for_test(url), do: badarg_error(url) + @doc false @spec loopback_tracker_for_test(binary()) :: boolean() def loopback_tracker_for_test(announce) do diff --git a/test/tracker_http_decode_test.exs b/test/tracker_http_decode_test.exs index 956a656..11c24e0 100644 --- a/test/tracker_http_decode_test.exs +++ b/test/tracker_http_decode_test.exs @@ -3,6 +3,28 @@ defmodule TrackerHTTPDecodeTest do alias Tracker.{Error, Response} + describe "badarg_error_for_test/1 (Hackney raises :badarg for a DNS-dead host)" do + test "a host with no address records is written off for the session" do + # Hackney raises :badarg instead of returning :nxdomain when the name has + # neither an A nor an AAAA record, so without this the defunct tracker was + # re-announced every cycle. `retry_in: "never"` is what makes + # PeerDiscovery.Announce drop it from the rotation. + assert %Error{reason: {:nxdomain, "tracker.invalid"}, retry_in: "never"} = + Tracker.badarg_error_for_test("http://tracker.invalid:80/announce") + end + + test "a host that does resolve keeps the opaque reason" do + # :badarg has other sources than dead DNS; only the DNS case may disable a + # tracker permanently. + assert %Error{reason: :badarg, retry_in: nil} = + Tracker.badarg_error_for_test("http://127.0.0.1:1/announce") + end + + test "a url with no host at all is not mistaken for a dead name" do + assert %Error{reason: :badarg} = Tracker.badarg_error_for_test("not a url") + end + end + describe "decode_http_response_for_test/1 (BEP 23 compact + dictionary peers)" do test "parses compact IPv4 peers and interval fields" do peers_bin = <<1, 2, 3, 4, 6881::16, 5, 6, 7, 8, 8080::16>> From ccf14d062e6ef588aa145e533b0e4bf2688173d0 Mon Sep 17 00:00:00 2001 From: Daniel Urumov Date: Sat, 12 Sep 2026 17:27:59 +0300 Subject: [PATCH 6/9] fix(stats): average the download rate over a window, not one quantized sample MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Torrent.Model` differenced `downloaded` over its 5 s tick, but `downloaded` advances only when a whole piece completes and verifies. The sample was therefore quantized to piece size: at 55 KB/s with 1 MiB pieces a piece lands every ~19 s, so three ticks in four read exactly 0.0. Live, two torrents at 39.7% and 52.0% both reported 0 B/s while gaining a combined 196 KB/s, and `compute_eta/4` in the WebUI saw `kbps <= 0` and answered `:infinity` for everything. `download_rate/1` now averages `delta / elapsed` over a 60 s window — the same measurement a human audit performs by summing `left` deltas — holds the last published average while a window containing progress matures, and clamps by `piece_length / elapsed` only when nothing has arrived, since were the torrent still running that fast the first piece of the window would already have landed. Nothing completed for 10 minutes reports 0. Two narrower approaches were tried against live traffic first and are recorded in the code comment, because each looks obviously right: * An EMA over the quantized samples. Any time constant short enough to track a fast torrent still collapses between a slow torrent's pieces — measured decaying to 1e-39 — and one long enough for the slow torrent is uselessly laggy for the fast one. * Timing each arrival against the previous arrival. A piece completing inside a single tick makes the measured interval ~5 s, a real burst rate but a bad basis for a stall cutoff, so the readout alternated between the burst and 0. The subtle one inside the current design: applying the silence ceiling *while* the window had progress in it made a true 100 KB/s read as 11 KB/s, because the ceiling shrinks as the window ages. Progress in the window has to disable it. Measured before/after on the same live torrent: constantly 0.0 while gaining ~100 KB/s, to readings of 12-157 KB/s around a true 64 KB/s average. The residual oscillation is inherent to a piece-granular source on a bursty swarm; a complete fix needs byte-granular reporting from `Peer.Controller`, which already tracks `downloaded_bytes` per connection. Co-authored-by: Cursor --- lib/elixir_torrent/torrent/model.ex | 95 +++++++++++++++++++++++- test/torrent_model_speed_test.exs | 110 ++++++++++++++++++++++++++++ 2 files changed, 203 insertions(+), 2 deletions(-) create mode 100644 test/torrent_model_speed_test.exs diff --git a/lib/elixir_torrent/torrent/model.ex b/lib/elixir_torrent/torrent/model.ex index 1b1204e..57afe55 100644 --- a/lib/elixir_torrent/torrent/model.ex +++ b/lib/elixir_torrent/torrent/model.ex @@ -11,6 +11,10 @@ defmodule Torrent.Model do require Logger @timeout_detect_the_speed 5 * 1_000 + # Averaging window for the download readout, and the point past which a torrent + # that has not completed a single piece is called stopped — see `download_rate/1`. + @rate_window 60 * 1_000 + @rate_window_max 10 * 60 * 1_000 # Mid-download resume checkpoint (BEP-adjacent): persist bitfield + counters to # `.term` so a restart loads Session.apply/2 and Resume runs :verify_saved on # only the pieces we claim — not a blind re-download from peers. Without this, @@ -166,11 +170,11 @@ defmodule Torrent.Model do do: {:noreply, torrent} @spec handle_info(term(), Torrent.t()) :: {:noreply, Torrent.t()} - def handle_info({:detected_the_speed, download, upload}, %Torrent{} = torrent) do + def handle_info({:detected_the_speed, _download, upload}, %Torrent{} = torrent) do message_for_next_detection(torrent) speed = %{ - download: detected_the_speed(torrent.downloaded, download), + download: download_rate(torrent), upload: detected_the_speed(torrent.uploaded, upload) } @@ -220,10 +224,97 @@ defmodule Torrent.Model do defp do_get(key, torrent), do: Map.get(torrent, key) + @doc false + @spec download_rate_for_test(Torrent.t()) :: number() + def download_rate_for_test(torrent), do: download_rate(torrent) + # Kb/s defp detected_the_speed(current, old), do: (current - old) / @timeout_detect_the_speed + # Download rate in the same Kb/s units, but averaged over a window long enough to + # contain several pieces instead of differenced over the 5 s tick. + # + # `downloaded` advances only when a whole piece completes and verifies, so a 5 s + # difference is quantized to piece size: at 55 KB/s with 1 MiB pieces one lands + # every ~19 s, so three ticks in four read exactly 0. That is what reported + # 0 B/s for torrents demonstrably progressing (#53b). + # + # Two narrower fixes were tried live and both failed, which is why the window is + # the shape it is: + # * An EMA over the quantized samples. Any time constant short enough to track + # a fast torrent still collapses between a slow torrent's pieces — observed + # decaying to 1e-39 — and one long enough for the slow torrent is uselessly + # laggy for the fast one. + # * Timing each arrival against the previous one. A piece completing inside a + # single tick makes the measured interval ~5 s, which is a real burst rate + # (209 KB/s on a 1 MiB piece) but a bad basis for deciding the torrent has + # stalled, so the readout alternated between the burst and 0. + # + # Averaging `delta / elapsed` over a fixed @rate_window sidesteps both: the window + # spans enough pieces that quantization averages out, and it is the same + # measurement used by hand when auditing this node (sum of `left` deltas over + # ≥100 s). While a window is still open the last published average is held rather + # than a burst rate, clamped by `piece_length / elapsed` — were the torrent still + # going that fast, the next piece would already have landed. A torrent that + # completes nothing for @rate_window_max is called stopped; that is generous on + # purpose, since a genuinely slow torrent can need minutes per piece. + defp download_rate(%Torrent{} = torrent) do + now = System.monotonic_time(:millisecond) + key = progress_key(torrent) + {start_at, start_downloaded} = rate_window(key, now, torrent.downloaded) + elapsed = max(now - start_at, 1) + delta = torrent.downloaded - start_downloaded + + case rate_verdict(torrent, delta, elapsed) do + {:publish, rate} -> + Process.put(key, {now, torrent.downloaded}) + rate + + :hold -> + torrent.speed.download + + {:ceiling, ceiling} -> + min(torrent.speed.download, ceiling) + end + end + + defp rate_verdict(%Torrent{left: 0}, _delta, _elapsed), do: {:publish, 0.0} + + defp rate_verdict(_torrent, delta, elapsed) when delta > 0 and elapsed >= @rate_window, + do: {:publish, delta / elapsed} + + # Bytes have landed inside this window, so the torrent is demonstrably moving: + # hold the last published average until the window matures. Deliberately no + # ceiling here — progress is proof, and applying one anyway is what made the + # readout sawtooth from a true 100 KB/s down to 11 as the window aged, since the + # ceiling is `piece_length / elapsed` and `elapsed` grows all window long. + defp rate_verdict(_torrent, delta, _elapsed) when delta > 0, do: :hold + + defp rate_verdict(_torrent, 0, elapsed) when elapsed >= @rate_window_max, + do: {:publish, 0.0} + + # Nothing at all has arrived yet. Now the ceiling is meaningful: were the torrent + # still running faster than this, the first piece of the window would have landed. + defp rate_verdict(torrent, 0, elapsed), + do: {:ceiling, do_piece_length(torrent) / elapsed} + + defp progress_key(%Torrent{hash: hash}), do: {:speed_rate_window, hash} + + # The first tick has to open the window, otherwise `elapsed` would be recomputed + # from `now` every tick and could never grow. + defp rate_window(key, now, downloaded) do + case Process.get(key) do + nil -> + window = {now, downloaded} + Process.put(key, window) + window + + stored -> + stored + end + end + defp message_for_next_detection(torrent) do message = {:detected_the_speed, torrent.downloaded, torrent.uploaded} Process.send_after(self(), message, @timeout_detect_the_speed) diff --git a/test/torrent_model_speed_test.exs b/test/torrent_model_speed_test.exs new file mode 100644 index 0000000..7b78d97 --- /dev/null +++ b/test/torrent_model_speed_test.exs @@ -0,0 +1,110 @@ +defmodule TorrentModelSpeedTest do + use ExUnit.Case, async: true + + # `downloaded` advances only when a whole piece completes and verifies, so + # differencing it over the 5 s tick is quantized to piece size and read 0 for most + # of a slow torrent's life — the UI showed 0 B/s for torrents demonstrably + # progressing (PLAN.md #53b). `download_rate/1` therefore averages over a window + # long enough to span several pieces. These tests pin that, and the window is + # driven by rewinding the stored anchor rather than by sleeping. + @piece_bytes 1024 * 1024 + @rate_window 60 * 1_000 + @rate_window_max 10 * 60 * 1_000 + + defp torrent(downloaded, left, speed) do + %Torrent{ + hash: :crypto.strong_rand_bytes(20), + metadata: %{"info" => %{"piece length" => @piece_bytes, "name" => "t"}}, + left: left, + last_index: 100, + last_piece_length: @piece_bytes, + downloaded: downloaded, + speed: %{download: speed, upload: 0} + } + end + + defp key(torrent), do: {:speed_rate_window, torrent.hash} + + # Opens the window and then backdates it, so a full window can be exercised + # without waiting a minute. + defp open_window(torrent, age_ms, downloaded_at_start) do + Torrent.Model.download_rate_for_test(torrent) + Process.put(key(torrent), {System.monotonic_time(:millisecond) - age_ms, downloaded_at_start}) + torrent + end + + describe "download_rate/1" do + test "a matured window reports the average, not a burst" do + # 3 MiB over a 60 s window is 3 MiB/60 s — the same figure a hand audit gets + # from summing `left` deltas. The old code reported either 0 or the ~209 KB/s + # burst of a single piece landing inside one 5 s tick. + t = torrent(3 * @piece_bytes, @piece_bytes * 10, 0) + open_window(t, @rate_window, 0) + + rate = Torrent.Model.download_rate_for_test(t) + + assert_in_delta rate, 3 * @piece_bytes / @rate_window, 1.0 + end + + test "a matured window starts a fresh one so the next average is independent" do + t = torrent(3 * @piece_bytes, @piece_bytes * 10, 0) + open_window(t, @rate_window, 0) + + Torrent.Model.download_rate_for_test(t) + + expected = 3 * @piece_bytes + assert {_at, ^expected} = Process.get(key(t)) + end + + test "the readout holds between pieces instead of collapsing to zero" do + # The defect itself: every tick between two piece completions used to publish + # exactly 0.0 while bytes were arriving normally. + t = torrent(@piece_bytes, @piece_bytes * 10, 55.0) + open_window(t, 20_000, 0) + + assert Torrent.Model.download_rate_for_test(t) > 0 + end + + test "a moving torrent is held unclamped while its window matures" do + # A piece has landed in this window, so the rate must be held exactly, not + # pulled down by piece_length/elapsed. Clamping here made a true 100 KB/s read + # as 11 KB/s once the window had aged 95 s. + t = torrent(@piece_bytes, @piece_bytes * 10, 100.0) + open_window(t, 50_000, 0) + + assert Torrent.Model.download_rate_for_test(t) == 100.0 + end + + test "with nothing arriving the rate is clamped by what the silence proves" do + # No piece at all in the window: were the torrent still running this fast, the + # first one would already have landed, so the claim is pulled down. + absurd = @piece_bytes * 1.0 + t = torrent(@piece_bytes, @piece_bytes * 10, absurd) + open_window(t, 30_000, @piece_bytes) + + rate = Torrent.Model.download_rate_for_test(t) + + assert rate < absurd + assert_in_delta rate, @piece_bytes / 30_000, @piece_bytes / 30_000 * 0.2 + end + + test "a torrent that completes nothing for the maximum window reports zero" do + t = torrent(@piece_bytes, @piece_bytes * 10, 55.0) + open_window(t, @rate_window_max, @piece_bytes) + + assert Torrent.Model.download_rate_for_test(t) == 0.0 + end + + test "a completed torrent reports zero" do + t = torrent(@piece_bytes * 10, 0, 500.0) + + assert Torrent.Model.download_rate_for_test(t) == 0.0 + end + + test "a fresh window reports zero rather than guessing" do + t = torrent(0, @piece_bytes * 10, 0) + + assert Torrent.Model.download_rate_for_test(t) == 0.0 + end + end +end From 9c4dfaa7a2c883e8dd06521298602abb746c0c31 Mon Sep 17 00:00:00 2001 From: Daniel Urumov Date: Sat, 12 Sep 2026 17:28:06 +0300 Subject: [PATCH 7/9] test(dht): compare against one routing table instead of building two `mark_bad`/`mark_query_failed` on an id that is not in the table are no-ops, and the assertions said so by comparing the result against a second `RoutingTables.new(@hash)`. The struct carries `last_changed_ms` from the monotonic clock, so under full-suite load the two constructions land in different milliseconds and the equality fails; it passed in isolation and failed once in ten full runs. The claim is that the call is a no-op, not that `new/1` is deterministic, so bind the table once and compare against that value. Co-authored-by: Cursor --- test/cycle2_pure_protocol_coverage_test.exs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/test/cycle2_pure_protocol_coverage_test.exs b/test/cycle2_pure_protocol_coverage_test.exs index 7a6d893..294344e 100644 --- a/test/cycle2_pure_protocol_coverage_test.exs +++ b/test/cycle2_pure_protocol_coverage_test.exs @@ -185,10 +185,16 @@ defmodule Cycle2PureProtocolCoverageTest do assert RoutingTables.mark_good(tables, @local_id) != tables missing = <<0xCC, 0::152>> - assert RoutingTables.mark_bad(RoutingTables.new(@hash), missing) == RoutingTables.new(@hash) - assert RoutingTables.mark_query_failed(RoutingTables.new(@hash), missing) == - RoutingTables.new(@hash) + # Compared against a second `RoutingTables.new(@hash)` these two flaked: the + # struct carries `last_changed_ms` from the monotonic clock, so under + # full-suite load the two constructions land in different milliseconds. The + # claim here is that marking an id that is not in the table is a no-op — not + # that `new/1` is deterministic — so compare against the same value. + empty = RoutingTables.new(@hash) + + assert RoutingTables.mark_bad(empty, missing) == empty + assert RoutingTables.mark_query_failed(empty, missing) == empty v6 = %{id: <<0xDD, 0::152>>, ip: {0x2001, 0, 0, 0, 0, 0, 0, 1}, port: 6882} tables = RoutingTables.new(@hash) |> RoutingTables.insert(v6, now_ms: 0) From 238de748427cc7b076d403e784582cf50fe2e861 Mon Sep 17 00:00:00 2001 From: Daniel Urumov Date: Sat, 12 Sep 2026 17:29:19 +0300 Subject: [PATCH 8/9] chore: release 0.6.7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The peer-id prefix is derived from the package version (BEP 20), so this bump is visible on the wire as `ET0-6-7` and worth its own commit: every peer that sees us during this release identifies the build by it. The changelog entry groups the four fixes by what they had in common rather than by subsystem — each one was the engine misbehaving while its own instruments read normal, which is why all four survived so long. Co-authored-by: Cursor --- CHANGELOG.md | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++ README.md | 6 +++--- mix.exs | 2 +- 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d1c9591..2789960 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,62 @@ # Changelog +## 0.6.7 - 2026-09-12 + +A measurement release. Every fix here is a case where the engine was doing +something wrong *and the instruments said it was fine* — a tracker retried +forever because its failure had no reason attached, healthy peers marked +unreachable because a local error was counted as a network one, and a download +speed of 0 B/s reported on a torrent moving at 100 KB/s. + +### Fixed + +- A tracker whose hostname no longer resolves is dropped for the session instead + of being announced to forever. Hackney raises `:badarg` out of its connect path + for a name with no A and no AAAA record rather than returning `:nxdomain`, so + the HTTP announce returned a reason with no `retry_in` and got the default retry + interval — `tracker.openbittorrent.com`, defunct, was re-announced six times an + hour across four torrents. The UDP side never had this problem because it + resolves up front and answers `retry_in: "never"`. The `:badarg` catch now + resolves the host itself and only writes a tracker off when DNS genuinely has + nothing; other sources of `:badarg` keep the old opaque reason, because a + permanent disable is too destructive to apply on a guess. +- Announce failures name the torrent and the announce URL. The line read + `request failure reason: ` with neither, which made the most actionable + message in the log useless: a tracker's own bencoded failure reason asks the + user to re-add a torrent it never identified. Two classification gaps went with + it — HTTPoison surfaces hackney's connect timeout as + `{:timeout, {:gen_statem, :call, [pid, :connect, 8000]}}`, which the + connect-timeout case never matched (42 of 89 warnings in a 21-minute window), + and HTTP 4xx/5xx was not classified at all even though a 403 or 521 from a + public announce-list entry is a dead tracker that BEP 12 already fails over. +- `:add_peer_failed` no longer counts as a peer being unreachable. It is emitted + only *after* TCP connect and the full BEP 3 handshake succeed — we already hold + the peer's id — and means the supervisor could not start, which is a local + fault. It was depressing the measured per-family dial yield that drives the + address-family throttle and writing `DialBackoff` rows toward the hard-fail + threshold: one torrent held 61 of its 62 known endpoints sticky-blocked while + connected to 4 peers. The catch-all also hid the outcome behind it — peer + supervisors register under `{peer_id, hash}`, so a farm handing one id out from + many IPs loses the race at `start_child`, which is `:already_connected`. +- The download speed readout no longer reports 0 B/s on a moving torrent. + `Torrent.Model` differenced `downloaded` over its 5 s tick, but that counter + advances only when a whole piece verifies, so the sample was quantized to piece + size: at 55 KB/s with 1 MiB pieces three ticks in four read exactly 0.0. Two + live torrents reported 0 B/s while gaining a combined 196 KB/s, and the ETA + derived from it was `:infinity` throughout. The rate is now averaged over a 60 s + window, held while a window containing progress matures, and clamped by + `piece_length / elapsed` only when nothing has arrived at all. A torrent with + nothing completed for 10 minutes reports 0. This is an improvement, not a cure: + the source is still piece-granular, so a swarm delivering in bursts still + oscillates, and a torrent slower than one piece per window reads 0 until its + first piece lands. + +### Changed + +- The HTTP stack behind tracker announces and BEP 19 web seeds moved up: hackney + 4.7.2 → 4.7.4, h2 0.11.0 → 0.12.0, webtransport 0.4.4 → 0.4.5, quic 1.8.0 → + 1.8.2. + ## 0.6.6 - 2026-08-27 A swarm-health release. Under CGNAT, where a torrent runs on a handful of peers, diff --git a/README.md b/README.md index 7f604af..64f98b7 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # ElixirTorrent -[![GitHub release](https://img.shields.io/badge/release-0.6.6-181717?logo=github)](https://github.com/daniboybye/ElixirTorrent/releases/tag/0.6.6) [![Changelog](https://img.shields.io/badge/changelog-blue)](https://hexdocs.pm/elixir_torrent/changelog.html) [![Hex.pm](https://img.shields.io/hexpm/v/elixir_torrent.svg)](https://hex.pm/packages/elixir_torrent/0.6.6) [![HexDocs](https://img.shields.io/badge/hexdocs-0.6.6-8E44AD)](https://hexdocs.pm/elixir_torrent/0.6.6) [![Hex.pm Downloads](https://img.shields.io/hexpm/dt/elixir_torrent.svg)](https://hex.pm/packages/elixir_torrent) [![License](https://img.shields.io/hexpm/l/elixir_torrent.svg)](https://github.com/daniboybye/ElixirTorrent/blob/master/LICENSE) +[![GitHub release](https://img.shields.io/badge/release-0.6.7-181717?logo=github)](https://github.com/daniboybye/ElixirTorrent/releases/tag/0.6.7) [![Changelog](https://img.shields.io/badge/changelog-blue)](https://hexdocs.pm/elixir_torrent/changelog.html) [![Hex.pm](https://img.shields.io/hexpm/v/elixir_torrent.svg)](https://hex.pm/packages/elixir_torrent/0.6.7) [![HexDocs](https://img.shields.io/badge/hexdocs-0.6.7-8E44AD)](https://hexdocs.pm/elixir_torrent/0.6.7) [![Hex.pm Downloads](https://img.shields.io/hexpm/dt/elixir_torrent.svg)](https://hex.pm/packages/elixir_torrent) [![License](https://img.shields.io/hexpm/l/elixir_torrent.svg)](https://github.com/daniboybye/ElixirTorrent/blob/master/LICENSE) [![build](https://img.shields.io/github/actions/workflow/status/daniboybye/ElixirTorrent/build-and-publish.yml?branch=master&label=build&logo=github)](https://github.com/daniboybye/ElixirTorrent/actions/workflows/build-and-publish.yml) [![codecov](https://codecov.io/gh/daniboybye/ElixirTorrent/branch/master/graph/badge.svg)](https://codecov.io/gh/daniboybye/ElixirTorrent) [![BEPs](https://img.shields.io/badge/BEPs-23%20implemented-E8A33D)](PROTOCOL.md) [![Last commit](https://img.shields.io/github/last-commit/daniboybye/ElixirTorrent/master)](https://github.com/daniboybye/ElixirTorrent/commits/master) @@ -56,7 +56,7 @@ Full per-BEP status, including the known gaps: **[PROTOCOL.md](PROTOCOL.md)**. ```elixir def deps do [ - {:elixir_torrent, "~> 0.6.6"} + {:elixir_torrent, "~> 0.6.7"} ] end ``` @@ -173,7 +173,7 @@ Full reference: [`hexdocs.pm/elixir_torrent/ElixirTorrent.html`](https://hexdocs | `stop_all_and_serialize/0` | Graceful stop + persist for every torrent | | `remove/2` | Stop and drop from session; optional `delete_data: true` | | `get/2` | Low-level field access (prefer `stats/2`) | -| `version/0` | Version-derived client peer ID prefix (`ET0-6-6`, BEP 20) | +| `version/0` | Version-derived client peer ID prefix (`ET0-6-7`, BEP 20) | ## ElixirTorrent Web (desktop app) diff --git a/mix.exs b/mix.exs index c46acf6..ea8fdb8 100644 --- a/mix.exs +++ b/mix.exs @@ -1,7 +1,7 @@ defmodule ElixirTorrent.MixProject do use Mix.Project - @version "0.6.6" + @version "0.6.7" def project do [ From d1e26f3b15c913cc7d1dc8391a60f5c8e228569a Mon Sep 17 00:00:00 2001 From: Daniel Urumov Date: Sat, 12 Sep 2026 17:48:02 +0300 Subject: [PATCH 9/9] fix(bep52): return an error from a failed leaf read instead of raising MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Merkle.leaf_range_response_from_disk/7` documents `{:ok, [hash()]} | {:error, term()}` and already handles a failed `:file.open/2` that way, but the per-leaf reads underneath it pattern-matched `{:ok, block} = :file.pread(fd, offset, size)`. An I/O error, or a file truncated between the stat that produced `file_length` and the read, therefore raised `MatchError` out of the middle of a function whose contract says it returns errors. `HashServe` catches the raise and answers `hash_reject`, which is the correct thing to put on the wire for a hash request we cannot serve (BEP 52) — so this was low severity in the shipped path, and wrong everywhere else: any other caller got an exception for an ordinary disk condition, and the reject was reached by accident rather than by decision. `read_leaf_cache/4` now short-circuits on the first unreadable leaf, since the cache is only useful complete — every consumer does `Map.fetch!/2` on it. One subtlety worth naming: `:file.pread/3` answers a bare `:eof`, not an error tuple, when there is nothing at the offset. That is *not* the same as the legitimate case of a leaf beyond the end of the file, which BEP 52's power-of-two leaf padding makes routine and which hashes to the zero hash without reading at all. The padded case is now a guard clause on `offset >= file_length`, so a bare `:eof` from an actual read means `file_length` no longer describes the file and is reported as a failure. +2 tests: the truncated-file contract, and a whole-file read whose last block is short, so the ragged tail the new guard touches stays covered. Co-authored-by: Cursor --- CHANGELOG.md | 12 +++++ lib/elixir_torrent/torrent/merkle.ex | 53 ++++++++++++-------- test/torrent_merkle_test.exs | 72 ++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2789960..57ca70e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,18 @@ speed of 0 B/s reported on a torrent moving at 100 KB/s. oscillates, and a torrent slower than one piece per window reads 0 until its first piece lands. +- A disk error while serving a BEP 52 hash request returns an error instead of + raising. `Merkle.leaf_range_response_from_disk/7` documents + `{:error, term()}` and handles a failed `:file.open/2` that way, but the + per-leaf reads underneath it pattern-matched `{:ok, block} = :file.pread/3`, so + an I/O error — or a file truncated between the stat that produced `file_length` + and the read — raised `MatchError` from the middle of the function. `HashServe` + catches that and answers `hash_reject`, which is correct on the wire but + reached by the wrong path; any other caller got an exception for a disk + condition. `:file.pread/3` also answers a bare `:eof` rather than an error + tuple, which is now distinguished from the legitimate padded-leaf case that + reads no bytes at all. + ### Changed - The HTTP stack behind tracker announces and BEP 19 web seeds moved up: hackney diff --git a/lib/elixir_torrent/torrent/merkle.ex b/lib/elixir_torrent/torrent/merkle.ex index 784c40f..487b91b 100644 --- a/lib/elixir_torrent/torrent/merkle.ex +++ b/lib/elixir_torrent/torrent/merkle.ex @@ -679,9 +679,8 @@ defmodule Torrent.Merkle do ctx.proof_layers ) - cache = read_leaf_cache(fd, ctx.file_length, ctx.block_count, indices) - - with :ok <- + with {:ok, cache} <- read_leaf_cache(fd, ctx.file_length, ctx.block_count, indices), + :ok <- verify_piece_subtrees( cache, ctx.piece_hashes, @@ -1274,34 +1273,48 @@ defmodule Torrent.Merkle do |> MapSet.filter(&(&1 < block_count)) end + # Stops at the first unreadable leaf rather than hashing the rest, because the + # cache is only useful complete — every consumer below does `Map.fetch!/2`. defp read_leaf_cache(fd, file_length, block_count, indices) do - Enum.reduce(indices, %{}, fn leaf, acc -> - Map.put(acc, leaf, leaf_hash_from_fd(fd, file_length, block_count, leaf)) + Enum.reduce_while(indices, {:ok, %{}}, fn leaf, {:ok, acc} -> + case leaf_hash_from_fd(fd, file_length, block_count, leaf) do + {:ok, hash} -> {:cont, {:ok, Map.put(acc, leaf, hash)}} + {:error, reason} -> {:halt, {:error, reason}} + end end) end defp leaf_hash_from_fd(fd, file_length, block_count, leaf_index) do offset = leaf_index * @block_size - data = + with {:ok, data} <- read_leaf_bytes(fd, offset, file_length) do cond do - offset >= file_length -> - <<>> + leaf_index >= block_count -> {:ok, @zero_hash} + byte_size(data) == 0 -> {:ok, @zero_hash} + true -> {:ok, :crypto.hash(:sha256, data)} + end + end + end - offset + @block_size > file_length -> - size = file_length - offset - {:ok, block} = :file.pread(fd, offset, size) - block + # A leaf past the end of the file is a legitimate request on a padded tree, not + # a read at all: BEP 52 pads the leaf layer to a power of two and those leaves + # hash to `@zero_hash`. + defp read_leaf_bytes(_fd, offset, file_length) when offset >= file_length, do: {:ok, <<>>} - true -> - {:ok, block} = :file.pread(fd, offset, @block_size) - block - end + defp read_leaf_bytes(fd, offset, file_length) do + case :file.pread(fd, offset, min(@block_size, file_length - offset)) do + {:ok, block} -> + {:ok, block} - cond do - leaf_index >= block_count -> @zero_hash - byte_size(data) == 0 -> @zero_hash - true -> :crypto.hash(:sha256, data) + # `pread` answers a bare `:eof` instead of an error tuple when there is + # nothing at the offset. Reaching it here means `file_length` no longer + # describes the file — truncated under us between the `File.stat` and this + # read — so it is a real failure and not the padded-leaf case above. + :eof -> + {:error, :eof} + + {:error, reason} -> + {:error, reason} end end diff --git a/test/torrent_merkle_test.exs b/test/torrent_merkle_test.exs index fadc9fc..782875c 100644 --- a/test/torrent_merkle_test.exs +++ b/test/torrent_merkle_test.exs @@ -533,6 +533,78 @@ defmodule Torrent.MerkleTest do assert Merkle.leaf_range_response_from_disk(path, 1, [hash("x")], @block_size, 1, 2, 0) == {:error, :invalid_index} end + + test "returns an error tuple when the file is shorter than the declared length" do + # `file_length` comes from a stat taken before the read, so a file truncated + # or replaced in between leaves `:file.pread/3` reading past the real EOF. + # The function documents `{:error, term()}` and already handles a failed + # `:file.open/2` that way; a failed read used to raise `MatchError` out of + # the middle instead. `HashServe` catches that and answers `hash_reject`, + # which is the right thing on the wire (BEP 52) reached by the wrong path — + # and anything else calling this got an exception for a disk condition. + blocks = for byte <- [?a, ?b, ?c, ?d], do: :binary.copy(<>, @block_size) + content = IO.iodata_to_binary(blocks) + {:ok, tree} = Merkle.build(content) + piece_length = 2 * @block_size + {:ok, layer_bin} = Merkle.piece_layer(tree, piece_length) + + piece_hashes = + for <> do + digest + end + + dir = Path.join(System.tmp_dir!(), "merkle_short_#{System.unique_integer([:positive])}") + File.mkdir_p!(dir) + path = Path.join(dir, "truncated.bin") + # Only the first block is actually on disk. + File.write!(path, :binary.part(content, 0, @block_size)) + on_exit(fn -> File.rm_rf!(dir) end) + + assert Merkle.leaf_range_response_from_disk( + path, + byte_size(content), + piece_hashes, + piece_length, + 0, + 4, + 0 + ) == {:error, :eof} + end + + test "still serves a whole file whose last block is short" do + # The guard added alongside the error contract must not disturb the ordinary + # ragged-tail case: a final partial block is normal, not a truncated file. + full = for byte <- [?a, ?b, ?c, ?d, ?e, ?f, ?g], do: :binary.copy(<>, @block_size) + content = IO.iodata_to_binary(full) <> :binary.copy(<>, 100) + {:ok, tree} = Merkle.build(content) + root = Merkle.root(tree) + piece_length = 4 * @block_size + {:ok, layer_bin} = Merkle.piece_layer(tree, piece_length) + + piece_hashes = + for <> do + digest + end + + dir = Path.join(System.tmp_dir!(), "merkle_ragged_#{System.unique_integer([:positive])}") + File.mkdir_p!(dir) + path = Path.join(dir, "ragged.bin") + File.write!(path, content) + on_exit(fn -> File.rm_rf!(dir) end) + + assert {:ok, hashes} = + Merkle.leaf_range_response_from_disk( + path, + byte_size(content), + piece_hashes, + piece_length, + 0, + 2, + 2 + ) + + assert Merkle.verify_hashes(root, 0, 0, 2, 2, hashes, 8) + end end describe "libtorrent flat proof helpers" do