fix graph 500 error on dns check page (#75)

Float.round/2 crashes on integer values from the status dataset. Adds integer guard clause to format_value/2.

Reviewed-on: graham/towerops-web#75

lib/towerops/monitoring/executors/ping_executor.ex

@@ -118,7 +118,11 @@ defmodule Towerops.Monitoring.Executors.PingExecutor do
   defp parse_packet_loss(output) do
     # Match patterns like "3 packets transmitted, 3 packets received, 0.0% packet loss"
     # or "3 packets transmitted, 3 received, 0% packet loss"
-    case Regex.run(~r/(\d+) packets? transmitted, (\d+) (?:packets? )?received, ([\d.]+)% packet loss/, output) do
+    # or "1 packets transmitted, 0 received, +1 errors, 100% packet loss"
+    case Regex.run(
+           ~r/(\d+) packets? transmitted, (\d+) (?:packets? )?received,(?:\s*\+\d+ errors,)? ([\d.]+)% packet loss/,
+           output
+         ) do
       [_, transmitted, _received, loss_str] ->
         {loss_pct, _} = Float.parse(loss_str)
         {:ok, loss_pct, "#{transmitted} packets"}

lib/towerops/workers/check_executor_worker.ex

@@ -10,7 +10,7 @@ defmodule Towerops.Workers.CheckExecutorWorker do
   - http → HttpExecutor
   - tcp → TcpExecutor
   - dns → DnsExecutor
-  - ping → PingExecutor (future)
+  - ping → PingExecutor (server-side fallback, skipped when agent assigned)
 
   Records results in check_results TimescaleDB hypertable and updates
   check state (OK/WARNING/CRITICAL/UNKNOWN).
@@ -63,6 +63,10 @@ defmodule Towerops.Workers.CheckExecutorWorker do
             Logger.debug("Skipping service check #{check_id} - device assigned to agent")
             schedule_next_check(check)
 
+          should_skip_ping_check?(check) ->
+            Logger.debug("Skipping ping check #{check_id} - device assigned to agent")
+            schedule_next_check(check)
+
           true ->
             execute_and_record(check)
             schedule_next_check(check)
@@ -87,6 +91,12 @@ defmodule Towerops.Workers.CheckExecutorWorker do
 
   defp should_skip_service_check?(_check), do: false
 
+  defp should_skip_ping_check?(%{check_type: "ping"} = check) do
+    check.device_id != nil and not Agents.should_phoenix_poll_device?(%Device{id: check.device_id})
+  end
+
+  defp should_skip_ping_check?(_check), do: false
+
   defp execute_and_record(check) do
     Logger.debug("Executing check #{check.id} (#{check.check_type})")
 

lib/towerops_web/live/graph_live/show.ex

@@ -761,8 +761,12 @@ defmodule ToweropsWeb.GraphLive.Show do
   end
 
   # For other units, format with one decimal place
-  defp format_value(value, unit) do
-    "#{Float.round(value, 1)}#{unit}"
+  defp format_value(value, unit) when is_float(value) do
+    "#{Float.round(value, 1)} #{unit}"
+  end
+
+  defp format_value(value, unit) when is_integer(value) do
+    "#{value} #{unit}"
   end
 
   # Live polling functions

test/towerops/monitoring/executors/ping_executor_test.exs

@@ -115,6 +115,20 @@ defmodule Towerops.Monitoring.Executors.PingExecutorTest do
       assert reason =~ "packet loss"
     end
 
+    test "parses Linux ping output with errors field" do
+      output = """
+      PING 10.250.1.50 (10.250.1.50) 56(84) bytes of data.
+      From 204.110.191.185 icmp_seq=1 Destination Host Unreachable
+
+      --- 10.250.1.50 ping statistics ---
+      1 packets transmitted, 0 received, +1 errors, 100% packet loss, time 0ms
+      """
+
+      assert {:error, reason} = PingExecutor.parse_output(output)
+      assert reason =~ "100"
+      assert reason =~ "packet loss"
+    end
+
     test "returns error for partial packet loss" do
       output = """
       PING 10.0.0.1 (10.0.0.1): 56 data bytes