Prevent caching handle-less 409 must-refetch responses

.changeset/handleless-409-no-store.md

@@ -0,0 +1,5 @@
+---
+"@core/sync-service": patch
+---
+
+Prevent handle-less `409 must-refetch` responses from being stored by caches, while preserving cacheable redirects for `409` responses that include an `electric-handle`.

packages/sync-service/lib/electric/shapes/api/response.ex

@@ -256,18 +256,16 @@ defmodule Electric.Shapes.Api.Response do
     |> put_cache_header("cache-control", "no-cache", api)
   end
 
-  # Briefly cache 409s as they act as shape redirects, when the requested shape
-  # is either invalidated or does not match the requested definition, and thus
-  # can benefit from persisting this cache for a brief period of time to avoid
-  # surges of traffic hitting the server whenever a shape is invalidated
-  defp put_cache_headers(conn, %__MODULE__{status: status, api: api, handle: handle})
-       when status in [409] do
-    # if handle is not present, cache for a minimum time just to allow request coalescing,
-    # as 409s without handles are suboptimal redirects
-    age = if is_nil(handle), do: 1, else: 60
-
+  # Briefly cache 409s with handles as they act as shape redirects, when the
+  # requested shape is either invalidated or does not match the requested
+  # definition, and thus can benefit from persisting this cache for a brief
+  # period of time to avoid surges of traffic hitting the server whenever a
+  # shape is invalidated. Handle-less 409s fall through to the general 4xx
+  # no-store handling below.
+  defp put_cache_headers(conn, %__MODULE__{status: 409, api: api, handle: handle})
+       when not is_nil(handle) do
     conn
-    |> put_cache_header("cache-control", "public, max-age=#{age}, must-revalidate", api)
+    |> put_cache_header("cache-control", "public, max-age=60, must-revalidate", api)
   end
 
   # All other 4xx and 5xx responses should never be cached

packages/sync-service/test/electric/plug/serve_shape_plug_test.exs

@@ -720,6 +720,9 @@ defmodule Electric.Plug.ServeShapePlugTest do
 
       assert conn.status == 409
       assert [%{"headers" => %{"control" => "must-refetch"}}] = Jason.decode!(conn.resp_body)
+      assert get_resp_header(conn, "electric-handle") == []
+      assert get_resp_header(conn, "cache-control") == ["no-store"]
+      assert get_resp_header(conn, "surrogate-control") == ["no-store"]
     end
 
     test "sends an up-to-date response after a timeout if no changes are observed",