fix(sync): classify unexpected 5xx statuses as transient so vendor syncs retry

SyncErrors.transient?/1 only had explicit clauses for the {:http_error, ...}
shape, but vendor API clients return {:unexpected_status, status, body}. A 502
was retried only by accident via the catch-all, and a 4xx in that shape would
have been retried 20 times. Add explicit {:unexpected_status, ...} clauses: 5xx
is transient (Oban backs off and retries), 4xx is permanent.
This commit is contained in:
Graham McIntire 2026-05-14 09:27:17 -05:00
parent 8e3f20a589
commit 6b82205a58
3 changed files with 56 additions and 0 deletions

View file

@ -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`,

View file

@ -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

View file

@ -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, "<html>502 Bad Gateway</html>"})
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