From 7ab426c01daba95cfc7957e6c45d5136abfffb62 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Tue, 21 Apr 2026 13:15:27 -0500 Subject: [PATCH] fix(workers): propagate transient errors so Oban retries + counters fire CommonVolumeRadarWorker and MechanismClassifyWorker were both returning :ok on upstream failures, so Prometheus job-failure counters stayed at zero during multi-hour NEXRAD outages and classifier bugs went unnoticed. - CommonVolumeRadarWorker: distinguish permanent (4xx) from transient errors using the same pattern as RadarFrameWorker. Permanent errors still mark :unavailable + :ok so the backfill stops re-queueing them. Transient errors return {:error, reason} and leave radar_status alone so the next retry can succeed. - MechanismClassifyWorker: keep the row-status :failed update so operators can see which contacts blew up, but return {:error, inspect(e)} from the rescue instead of swallowing it as :ok. --- .../workers/common_volume_radar_worker.ex | 27 +++++++++++++-- .../workers/mechanism_classify_worker.ex | 5 ++- .../common_volume_radar_worker_test.exs | 33 ++++++++++++++++++- .../mechanism_classify_worker_test.exs | 16 +++++++++ 4 files changed, 76 insertions(+), 5 deletions(-) diff --git a/lib/microwaveprop/workers/common_volume_radar_worker.ex b/lib/microwaveprop/workers/common_volume_radar_worker.ex index 1fbfcf6c..1dcf862c 100644 --- a/lib/microwaveprop/workers/common_volume_radar_worker.ex +++ b/lib/microwaveprop/workers/common_volume_radar_worker.ex @@ -86,12 +86,33 @@ defmodule Microwaveprop.Workers.CommonVolumeRadarWorker do :ok {:error, reason} -> - Logger.info("CommonVolumeRadarWorker: no frame for #{contact.id}: #{inspect(reason)}") - mark_unavailable(contact) - :ok + if permanent_error?(reason) do + Logger.info("CommonVolumeRadarWorker: no frame for #{contact.id}: #{inspect(reason)}") + mark_unavailable(contact) + :ok + else + # Transient (5xx, timeout, connrefused) — return {:error, _} so + # Oban retries and the failure is reflected in the job-failure + # counters. Do NOT pin the contact :unavailable; it must remain + # eligible for the next attempt. + Logger.warning("CommonVolumeRadarWorker: transient error for #{contact.id}: #{inspect(reason)}") + {:error, reason} + end end end + # Permanent: 4xx means the IEM archive genuinely has no frame at this + # timestamp, so retrying is pointless. Everything else (5xx, transport + # errors, decode failures) is treated as transient. + defp permanent_error?("NEXRAD n0q HTTP " <> code) do + case Integer.parse(code) do + {status, _} when status in 400..499 -> true + _ -> false + end + end + + defp permanent_error?(_), do: false + @doc """ Aggregate n0q pixel statistics over the common volume of the two endpoints. Pure function — accepts raw pixel buffer + width so it diff --git a/lib/microwaveprop/workers/mechanism_classify_worker.ex b/lib/microwaveprop/workers/mechanism_classify_worker.ex index 614f3a05..18c2c3a9 100644 --- a/lib/microwaveprop/workers/mechanism_classify_worker.ex +++ b/lib/microwaveprop/workers/mechanism_classify_worker.ex @@ -66,8 +66,11 @@ defmodule Microwaveprop.Workers.MechanismClassifyWorker do e -> Logger.warning("MechanismClassifyWorker: failed for contact #{contact.id}: #{inspect(e)}") + # Persist row state so operators can see which contacts blew up, but + # return {:error, _} to Oban so the failure registers in job-failure + # counters and the job is retried rather than silently swallowed. mark_status(contact, :failed, nil, nil) - :ok + {:error, inspect(e)} end defp build_inputs(%Contact{} = contact) do diff --git a/test/microwaveprop/workers/common_volume_radar_worker_test.exs b/test/microwaveprop/workers/common_volume_radar_worker_test.exs index 5908d779..db6a3984 100644 --- a/test/microwaveprop/workers/common_volume_radar_worker_test.exs +++ b/test/microwaveprop/workers/common_volume_radar_worker_test.exs @@ -5,6 +5,7 @@ defmodule Microwaveprop.Workers.CommonVolumeRadarWorkerTest do alias Microwaveprop.Radio.Contact alias Microwaveprop.Radio.ContactCommonVolumeRadar alias Microwaveprop.Repo + alias Microwaveprop.Weather.NexradClient alias Microwaveprop.Workers.CommonVolumeRadarWorker setup do @@ -38,7 +39,7 @@ defmodule Microwaveprop.Workers.CommonVolumeRadarWorkerTest do test "marks radar_status :unavailable and does not insert a row when n0q frame is 404" do contact = insert_contact() - Req.Test.stub(Microwaveprop.Weather.NexradClient, fn conn -> + Req.Test.stub(NexradClient, fn conn -> Plug.Conn.send_resp(conn, 404, "not found") end) @@ -59,6 +60,36 @@ defmodule Microwaveprop.Workers.CommonVolumeRadarWorkerTest do assert %Contact{radar_status: :unavailable} = Repo.get!(Contact, contact.id) end + test "returns {:error, _} on transient 5xx so Oban retries and status stays :queued" do + contact = insert_contact() + + Req.Test.stub(NexradClient, fn conn -> + Plug.Conn.send_resp(conn, 503, "service unavailable") + end) + + assert {:error, _reason} = + perform_job(CommonVolumeRadarWorker, %{"contact_id" => contact.id}) + + refute Repo.get_by(ContactCommonVolumeRadar, contact_id: contact.id) + # Transient errors must not pin the contact as :unavailable — it needs + # to remain eligible for a retry. + refute Repo.get!(Contact, contact.id).radar_status == :unavailable + end + + test "returns {:error, _} on transport-level failures (connrefused, timeout)" do + contact = insert_contact() + + Req.Test.stub(NexradClient, fn conn -> + Req.Test.transport_error(conn, :econnrefused) + end) + + assert {:error, _reason} = + perform_job(CommonVolumeRadarWorker, %{"contact_id" => contact.id}) + + refute Repo.get_by(ContactCommonVolumeRadar, contact_id: contact.id) + refute Repo.get!(Contact, contact.id).radar_status == :unavailable + end + test "skips contacts beyond 2R so we don't waste a fetch" do # 2R = 800 km. Put the stations 2000 km apart. contact = diff --git a/test/microwaveprop/workers/mechanism_classify_worker_test.exs b/test/microwaveprop/workers/mechanism_classify_worker_test.exs index 43bcf039..fca9cf87 100644 --- a/test/microwaveprop/workers/mechanism_classify_worker_test.exs +++ b/test/microwaveprop/workers/mechanism_classify_worker_test.exs @@ -138,6 +138,22 @@ defmodule Microwaveprop.Workers.MechanismClassifyWorkerTest do }) end + test "propagates errors from classifier so Oban counts failures + retries, but still marks row :failed" do + # Force the classifier to blow up by using a non-integer band — + # `Decimal.to_integer/1` in build_inputs raises on fractional values. + # This models the "unexpected classifier explosion" that silently + # dropped failure counts in prod. + contact = create_contact(%{band: Decimal.new("10.5")}) + + assert {:error, _reason} = + MechanismClassifyWorker.perform(%Oban.Job{ + args: %{"contact_id" => contact.id} + }) + + reloaded = Repo.get!(Contact, contact.id) + assert reloaded.mechanism_status == :failed + end + test "meteor-shower calendar matches across year boundaries" do # Quadrantids peak Jan 4, ±3 day window. A 6 m 900 km contact on # Dec 31 2025 is 4 days before the Jan 4 2026 peak — should still