fix(push): preserve devices on provider configuration errors

lib/who_need_help/push/device_delivery_worker.ex

@@ -50,9 +50,13 @@ defmodule WhoNeedHelp.Push.DeviceDeliveryWorker do
         _ = Notifications.disable_invalid_device(device)
         :ok
 
-      {:error, {:rejected, status, _body}} when status in [400, 401, 403, 404, 410] ->
+      {:error, {:rejected, status, body}} ->
-        _ = Notifications.disable_invalid_device(device)
+        if invalid_device_rejection?(device.provider, status, body) do
-        {:cancel, :device_rejected}
+          _ = Notifications.disable_invalid_device(device)
+          {:cancel, :device_rejected}
+        else
+          {:cancel, {:provider_rejected, status}}
+        end
 
       {:error, {:invalid_configuration, _field} = reason} ->
         {:cancel, reason}
@@ -61,4 +65,15 @@ defmodule WhoNeedHelp.Push.DeviceDeliveryWorker do
         {:error, reason}
     end
   end
+
+  defp invalid_device_rejection?(:web_push, status, _body) when status in [404, 410],
+    do: true
+
+  defp invalid_device_rejection?(:fcm, _status, %{
+         "fcm_error_code" => error_code
+       })
+       when error_code in ["INVALID_ARGUMENT", "UNREGISTERED"],
+       do: true
+
+  defp invalid_device_rejection?(_provider, _status, _body), do: false
 end

lib/who_need_help/push/fcm_adapter.ex

@@ -25,10 +25,10 @@ defmodule WhoNeedHelp.Push.FCMAdapter do
           {:ok, %{id: name, duplicate: false}}
 
         {:ok, %Req.Response{status: status, body: body}} when status in @retryable_statuses ->
-          {:error, {:retryable, status, sanitize(body)}}
+          {:error, {:retryable, status, sanitize_error(body)}}
 
         {:ok, %Req.Response{status: status, body: body}} ->
-          {:error, {:rejected, status, sanitize(body)}}
+          {:error, {:rejected, status, sanitize_error(body)}}
 
         {:error, error} ->
           {:error, {:transport, transport_reason(error)}}
@@ -101,10 +101,34 @@ defmodule WhoNeedHelp.Push.FCMAdapter do
 
   defp priority(_kind), do: "normal"
 
-  defp sanitize(%{"error" => error}) when is_map(error),
+  @doc false
-    do: Map.take(error, ["code", "status"])
+  def sanitize_error(%{"error" => error}) when is_map(error) do
+    fcm_error_code =
+      error
+      |> Map.get("details", [])
+      |> Enum.find_value(fn
+        %{
+          "@type" => "type.googleapis.com/google.firebase.fcm.v1.FcmError",
+          "errorCode" => error_code
+        }
+        when is_binary(error_code) ->
+          error_code
 
-  defp sanitize(_body), do: nil
+        _detail ->
+          nil
+      end)
+
+    error
+    |> Map.take(["code", "status"])
+    |> maybe_put_fcm_error_code(fcm_error_code)
+  end
+
+  def sanitize_error(_body), do: nil
+
+  defp maybe_put_fcm_error_code(error, nil), do: error
+
+  defp maybe_put_fcm_error_code(error, fcm_error_code),
+    do: Map.put(error, "fcm_error_code", fcm_error_code)
 
   defp transport_reason(%Req.TransportError{reason: reason}), do: reason
   defp transport_reason(%{__struct__: module}), do: module

test/who_need_help/push_product_test.exs

@@ -244,7 +244,50 @@ defmodule WhoNeedHelp.PushProductTest do
     refute inspect(payload) =~ "exact"
   end
 
-  test "device delivery retries transient failures and disables rejected registrations",
+  test "FCM retains only the provider error code needed to classify a rejected FID" do
+    invalid_fid_response = %{
+      "error" => %{
+        "code" => 400,
+        "message" => "The Firebase Installation ID is invalid",
+        "status" => "INVALID_ARGUMENT",
+        "details" => [
+          %{
+            "@type" => "type.googleapis.com/google.firebase.fcm.v1.FcmError",
+            "errorCode" => "INVALID_ARGUMENT"
+          }
+        ]
+      }
+    }
+
+    assert FCMAdapter.sanitize_error(invalid_fid_response) == %{
+             "code" => 400,
+             "status" => "INVALID_ARGUMENT",
+             "fcm_error_code" => "INVALID_ARGUMENT"
+           }
+
+    invalid_payload_response = %{
+      "error" => %{
+        "code" => 400,
+        "message" => "The data payload is invalid",
+        "status" => "INVALID_ARGUMENT",
+        "details" => [
+          %{
+            "@type" => "type.googleapis.com/google.rpc.BadRequest",
+            "fieldViolations" => [%{"field" => "message.data"}]
+          }
+        ]
+      }
+    }
+
+    assert FCMAdapter.sanitize_error(invalid_payload_response) == %{
+             "code" => 400,
+             "status" => "INVALID_ARGUMENT"
+           }
+
+    refute inspect(FCMAdapter.sanitize_error(invalid_fid_response)) =~ "Firebase Installation"
+  end
+
+  test "device delivery retries transient failures and only disables a rejected FID",
        context do
     Application.put_env(:who_need_help, :fcm_adapter, RecordingDeviceAdapter)
 
@@ -282,7 +325,45 @@ defmodule WhoNeedHelp.PushProductTest do
     assert is_nil(Repo.reload!(device).disabled_at)
 
     Application.put_env(:who_need_help, :device_delivery_options, %{
-      fcm: [test_pid: self(), result: {:error, {:rejected, 404, nil}}]
+      fcm: [
+        test_pid: self(),
+        result:
+          {:error,
+           {:rejected, 403,
+            %{"status" => "PERMISSION_DENIED", "fcm_error_code" => "SENDER_ID_MISMATCH"}}}
+      ]
+    })
+
+    assert {:cancel, {:provider_rejected, 403}} =
+             perform_job(DeviceDeliveryWorker, %{
+               "notification_id" => notification.id,
+               "device_id" => device.id
+             })
+
+    assert is_nil(Repo.reload!(device).disabled_at)
+
+    Application.put_env(:who_need_help, :device_delivery_options, %{
+      fcm: [
+        test_pid: self(),
+        result: {:error, {:rejected, 400, %{"status" => "INVALID_ARGUMENT"}}}
+      ]
+    })
+
+    assert {:cancel, {:provider_rejected, 400}} =
+             perform_job(DeviceDeliveryWorker, %{
+               "notification_id" => notification.id,
+               "device_id" => device.id
+             })
+
+    assert is_nil(Repo.reload!(device).disabled_at)
+
+    Application.put_env(:who_need_help, :device_delivery_options, %{
+      fcm: [
+        test_pid: self(),
+        result:
+          {:error,
+           {:rejected, 404, %{"status" => "NOT_FOUND", "fcm_error_code" => "UNREGISTERED"}}}
+      ]
     })
 
     assert {:cancel, :device_rejected} =