diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 7f2d3b29..f3b4f8cf 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,3 +1,19 @@ +2026-05-14 +fix(sync): classify unexpected 5xx statuses as transient so vendor syncs back off and retry + Vendor API clients (Preseem, etc.) return errors shaped as + `{:unexpected_status, status, body}`, but `SyncErrors.transient?/1` only + had explicit clauses for the `{:http_error, ...}` shape. A 502 Bad Gateway + was retried only by accident via the catch-all, and a 4xx in the same + shape would have been retried 20 times incorrectly. + - lib/towerops/workers/sync_errors.ex: added explicit + `{:unexpected_status, status}` / `{:unexpected_status, status, _}` + clauses — 5xx is transient (Oban retries with exponential backoff), + 4xx is permanent + - test/towerops/workers/sync_errors_test.exs: new test coverage for the + classifier across permanent, transient, and unknown error shapes + Surfaced by PreseemSyncWorker hitting 502s from api.preseem.com after + IPv6 was enabled in prod. + 2026-05-12 fix(session): resolve session salts at runtime so prod release boots The `@session_options` module attribute used `Application.compile_env`, diff --git a/lib/towerops/workers/sync_errors.ex b/lib/towerops/workers/sync_errors.ex index a0c2321c..b45ada2c 100644 --- a/lib/towerops/workers/sync_errors.ex +++ b/lib/towerops/workers/sync_errors.ex @@ -24,5 +24,9 @@ defmodule Towerops.Workers.SyncErrors do def transient?(:bad_request), do: false def transient?({:http_error, status, _}) when status in 400..499, do: false def transient?({:http_error, status}) when status in 400..499, do: false + def transient?({:unexpected_status, status, _}) when status in 400..499, do: false + def transient?({:unexpected_status, status}) when status in 400..499, do: false + def transient?({:unexpected_status, status, _}) when status in 500..599, do: true + def transient?({:unexpected_status, status}) when status in 500..599, do: true def transient?(_), do: true end diff --git a/test/towerops/workers/sync_errors_test.exs b/test/towerops/workers/sync_errors_test.exs new file mode 100644 index 00000000..8ed8a578 --- /dev/null +++ b/test/towerops/workers/sync_errors_test.exs @@ -0,0 +1,36 @@ +defmodule Towerops.Workers.SyncErrorsTest do + use ExUnit.Case, async: true + + alias Towerops.Workers.SyncErrors + + describe "transient?/1" do + test "permanent auth/config errors are not transient" do + refute SyncErrors.transient?(:unauthorized) + refute SyncErrors.transient?(:forbidden) + refute SyncErrors.transient?(:not_found) + refute SyncErrors.transient?(:invalid_credentials) + refute SyncErrors.transient?(:bad_request) + end + + test "4xx http errors are not transient" do + refute SyncErrors.transient?({:http_error, 404, "body"}) + refute SyncErrors.transient?({:http_error, 403}) + end + + test "5xx unexpected statuses are transient (worth a backoff retry)" do + assert SyncErrors.transient?({:unexpected_status, 502, "502 Bad Gateway"}) + assert SyncErrors.transient?({:unexpected_status, 503}) + assert SyncErrors.transient?({:unexpected_status, 504, "gateway timeout"}) + end + + test "4xx unexpected statuses are not transient" do + refute SyncErrors.transient?({:unexpected_status, 404, "not found"}) + refute SyncErrors.transient?({:unexpected_status, 401}) + end + + test "unknown error shapes default to transient" do + assert SyncErrors.transient?(:some_unknown_error) + assert SyncErrors.transient?({:error, :timeout}) + end + end +end