fix(esi): honour 304 response headers when revalidating cached ESI data

ESI GETs used Req's built-in `cache: true` step. On a 304, Req replaces
the response with the whole stored 200 response, including its original
headers, and does not refresh the on-disk entry. `maybe_cache_response/4`
then saw a 200 whose `expires` was already in the past, computed a
negative TTL, and the `:api_cache` entry expired immediately. Every
tracker tick for location/online therefore missed the in-memory cache and
re-requested ESI, receiving another 304 each time.

This surfaced once ESI started returning 304s on the location/online
routes, causing a noticeable increase in wasted requests.

Replace Req's cache with conditional requests handled in ApiClient:

  * On 200, store the body plus `etag`/`last-modified` validators in
    `:api_cache` under `{:esi_validators, path}`.
  * Send `if-none-match` / `if-modified-since` on subsequent requests.
  * On 304, reuse the stored body but take `expires`, `etag` and
    `last-modified` from the 304 response, extending the fresh cache
    until the new `expires`.
  * If a 304 arrives without a stored body, retry once unconditionally.
  * Skip caching when the computed TTL is not positive.

An optional `:esi_req_options` app env is merged into the Req options so
tests can route requests through `Req.Test`.
This commit is contained in:
Dmitry Popov committed 2026-09-27 10:05:34 +02:00
1 parent 6c5d43936e
commit cbbaf14fee
2 files changed
+197 -11

No files matched your search

+87 -11
View File
@@ -13,6 +13,7 @@ defmodule WandererApp.Esi.ApiClient do
@retry_opts [retry: false, retry_log_level: :warning]
@timeout_opts [pool_timeout: 15_000, receive_timeout: :timer.minutes(1)]
@api_retry_count 1
@validators_ttl :timer.hours(1)
@logger Application.compile_env(:wanderer_app, :logger)
@@ -24,7 +25,8 @@ defmodule WandererApp.Esi.ApiClient do
# Helper function to get Req options with appropriate Finch pool
defp req_options_for_pool(pool) do
[base_url: "https://esi.evetech.net", finch: pool]
[base_url: "https://esi.evetech.net", finch: pool] ++
Application.get_env(:wanderer_app, :esi_req_options, [])
end
def get_server_status, do: do_get("/status", [], @cache_opts)
@@ -310,18 +312,17 @@ defmodule WandererApp.Esi.ApiClient do
(opts |> with_refresh_token()) ++ @cache_opts
)
defp with_user_agent_opts(opts),
defp with_user_agent_opts(opts, extra_headers \\ []),
do:
opts
|> Keyword.merge(
headers: [{:user_agent, "Wanderer/#{WandererApp.Env.vsn()} #{@wanderrer_user_agent}"}]
headers:
[{:user_agent, "Wanderer/#{WandererApp.Env.vsn()} #{@wanderrer_user_agent}"}] ++
extra_headers
)
defp with_refresh_token(opts), do: opts |> Keyword.merge(refresh_token?: true)
defp with_cache_opts(opts),
do: opts |> Keyword.merge(@cache_opts) |> Keyword.merge(cache_dir: System.tmp_dir!())
defp do_get(path, api_opts, opts, pool \\ @general_pool) do
case Cachex.get(:api_cache, path) do
{:ok, cached_data} when not is_nil(cached_data) ->
@@ -339,17 +340,20 @@ defmodule WandererApp.Esi.ApiClient do
|> Req.get(
api_opts
|> Keyword.merge(url: path)
|> with_user_agent_opts()
|> with_cache_opts()
|> with_user_agent_opts(conditional_headers(path, opts))
|> Keyword.merge(@retry_opts)
|> Keyword.merge(@timeout_opts)
)
|> case do
{:ok, %{status: 200, body: body, headers: headers}} ->
maybe_store_validators(path, body, headers, opts)
maybe_cache_response(path, body, headers, opts)
{:ok, body}
{:ok, %{status: 304, headers: headers}} ->
handle_not_modified(path, headers, api_opts, opts, pool)
{:ok, %{status: 504}} ->
{:error, :timeout}
@@ -476,10 +480,10 @@ defmodule WandererApp.Esi.ApiClient do
defp maybe_cache_response(path, body, %{"expires" => [expires]} = _headers, opts)
when is_binary(path) and not is_nil(expires) do
try do
if opts |> Keyword.get(:cache, false) do
cached_ttl =
DateTime.diff(Timex.parse!(expires, "{RFC1123}"), DateTime.utc_now(), :millisecond)
cached_ttl =
DateTime.diff(Timex.parse!(expires, "{RFC1123}"), DateTime.utc_now(), :millisecond)
if opts |> Keyword.get(:cache, false) and cached_ttl > 0 do
Cachex.put(
:api_cache,
path,
@@ -497,6 +501,78 @@ defmodule WandererApp.Esi.ApiClient do
defp maybe_cache_response(_path, _body, _headers, _opts), do: :ok
# Conditional request support (ETag / Last-Modified).
#
# On 304 ESI returns fresh cache headers (expires, etag, ...) while the body is the one we
# already have. The body is kept from the stored 200 response, but headers must be taken from
# the 304 response, otherwise a stale `expires` makes the response uncacheable and we re-fetch
# it on every tick.
defp validators_key(path), do: {:esi_validators, path}
defp conditional_headers(path, opts) do
with true <- Keyword.get(opts, :cache, false),
false <- Keyword.get(opts, :skip_conditional, false),
{:ok, %{} = validators} <- Cachex.get(:api_cache, validators_key(path)) do
[
{"if-none-match", validators.etag},
{"if-modified-since", validators.last_modified}
]
|> Enum.reject(fn {_name, value} -> is_nil(value) end)
else
_ -> []
end
end
defp maybe_store_validators(path, body, headers, opts) do
etag = get_header(headers, "etag")
last_modified = get_header(headers, "last-modified")
if Keyword.get(opts, :cache, false) and not (is_nil(etag) and is_nil(last_modified)) do
Cachex.put(
:api_cache,
validators_key(path),
%{body: body, etag: etag, last_modified: last_modified},
ttl: @validators_ttl
)
end
:ok
end
defp handle_not_modified(path, headers, api_opts, opts, pool) do
case Cachex.get(:api_cache, validators_key(path)) do
{:ok, %{body: body} = validators} ->
Cachex.put(
:api_cache,
validators_key(path),
%{
validators
| etag: get_header(headers, "etag") || validators.etag,
last_modified: get_header(headers, "last-modified") || validators.last_modified
},
ttl: @validators_ttl
)
maybe_cache_response(path, body, headers, opts)
{:ok, body}
_ ->
if Keyword.get(opts, :skip_conditional, false) do
{:error, "Unexpected status: 304"}
else
do_get_request(path, api_opts, Keyword.put(opts, :skip_conditional, true), pool)
end
end
end
defp get_header(headers, name) do
case Map.get(headers, name) do
[value | _] -> value
_ -> nil
end
end
defp do_post(url, opts) do
try do
case Req.post("#{url}", opts |> with_user_agent_opts()) do
@@ -0,0 +1,110 @@
defmodule WandererApp.Esi.ApiClientConditionalTest do
# Uses the shared :api_cache Cachex instance and global app env
use ExUnit.Case, async: false
alias WandererApp.Esi.ApiClient
@path "/status"
@validators_key {:esi_validators, @path}
setup do
previous = Application.get_env(:wanderer_app, :esi_req_options)
Application.put_env(:wanderer_app, :esi_req_options, plug: {Req.Test, ApiClient})
clear_cache()
on_exit(fn ->
clear_cache()
if previous,
do: Application.put_env(:wanderer_app, :esi_req_options, previous),
else: Application.delete_env(:wanderer_app, :esi_req_options)
end)
:ok
end
test "caches 200 response until expires" do
stub(fn conn -> ok_response(conn, %{"players" => 1}, "\"v1\"", future(30)) end)
assert {:ok, %{"players" => 1}} = ApiClient.get_server_status()
assert {:ok, %{"players" => 1}} = ApiClient.get_server_status()
assert_received {:request, _}
refute_received {:request, _}
end
test "304 reuses stored body and extends cache with headers from the 304 response" do
stub(fn conn -> ok_response(conn, %{"players" => 1}, "\"v1\"", future(30)) end)
assert {:ok, %{"players" => 1}} = ApiClient.get_server_status()
assert_received {:request, _}
# Simulate the fresh cache window passing
Cachex.del(:api_cache, @path)
stub(fn conn ->
conn
|> Plug.Conn.put_resp_header("etag", "\"v1\"")
|> Plug.Conn.put_resp_header("expires", future(60))
|> Plug.Conn.send_resp(304, "")
end)
assert {:ok, %{"players" => 1}} = ApiClient.get_server_status()
assert_received {:request, headers}
assert {"if-none-match", "\"v1\""} in headers
assert {:ok, ttl} = Cachex.ttl(:api_cache, @path)
assert is_integer(ttl) and ttl > 30_000
# Served from the extended cache, no new request
assert {:ok, %{"players" => 1}} = ApiClient.get_server_status()
refute_received {:request, _}
end
test "304 without stored body retries unconditionally" do
stub(fn conn ->
if Plug.Conn.get_req_header(conn, "if-none-match") == [] do
ok_response(conn, %{"players" => 2}, "\"v2\"", future(30))
else
Plug.Conn.send_resp(conn, 304, "")
end
end)
Cachex.put(:api_cache, @validators_key, %{etag: "\"v1\"", last_modified: nil})
assert {:ok, %{"players" => 2}} = ApiClient.get_server_status()
end
test "does not cache responses with expires in the past" do
stub(fn conn -> ok_response(conn, %{"players" => 1}, "\"v1\"", past(10)) end)
assert {:ok, %{"players" => 1}} = ApiClient.get_server_status()
assert {:ok, nil} = Cachex.get(:api_cache, @path)
end
defp stub(fun) do
test_pid = self()
Req.Test.stub(ApiClient, fn conn ->
send(test_pid, {:request, conn.req_headers})
fun.(conn)
end)
end
defp ok_response(conn, body, etag, expires) do
conn
|> Plug.Conn.put_resp_header("etag", etag)
|> Plug.Conn.put_resp_header("expires", expires)
|> Req.Test.json(body)
end
defp future(seconds), do: http_date(DateTime.add(DateTime.utc_now(), seconds, :second))
defp past(seconds), do: http_date(DateTime.add(DateTime.utc_now(), -seconds, :second))
defp http_date(datetime), do: Calendar.strftime(datetime, "%a, %d %b %Y %H:%M:%S GMT")
defp clear_cache do
Cachex.del(:api_cache, @path)
Cachex.del(:api_cache, @validators_key)
end
end