Fix an extensions bug related to channels not sharing assigns (#1843)

We don't know of a devices firmware metadata until it connects for the
first time. When it does connect, the `DeviceSocket` will fetch the
device and the `firmware_metadata` field will be nil. This device is the
one shared with other channels.

In `Extensions.Health` we are using
`socket.assigns.device.firmware_metadata` which is going to be nil
during its first connection to NervesHub, and will continue to exit
sending reports and creating Sentry errors.

This test replicates the scenario of a device connecting for the first
time, then its firmware metadata being updated, and then connecting to
the health extension, exposing the error.

Sadly, in tests, channels share assigns, so this was much harder to
replicate.

Author: joshk

lib/nerves_hub/extensions/health.ex

@@ -54,7 +54,7 @@ defmodule NervesHub.Extensions.Health do
           do: {to_string(key), to_string(val)}
 
     # Separate metrics from health report to store in metrics table
-    metrics = device_status["metrics"]
+    metrics = device_status["metrics"] || %{}
 
     health_report =
       device_status

lib/nerves_hub_web/channels/extensions_channel.ex

@@ -3,13 +3,19 @@ defmodule NervesHubWeb.ExtensionsChannel do
 
   alias NervesHub.Extensions
   alias NervesHub.Helpers.Logging
+  alias NervesHub.Repo
   alias Phoenix.PubSub
   alias Phoenix.Socket.Broadcast
 
   require Logger
 
   @impl Phoenix.Channel
   def join("extensions", extension_versions, socket) do
+    # the assigns are not shared between channels, so if we don't
+    # reload the device we are likely to have incorrect data, especially
+    # after the first connect event
+    socket = reload_device(socket)
+
     extensions = parse_extensions(socket.assigns.device, extension_versions)
     socket = assign(socket, :extensions, extensions)
 
@@ -115,4 +121,13 @@ defmodule NervesHubWeb.ExtensionsChannel do
   end
 
   def handle_info(_msg, socket), do: {:noreply, socket}
+
+  defp reload_device(%{assigns: %{device: device}} = socket) do
+    device =
+      device
+      |> Repo.reload()
+      |> Repo.preload(:product)
+
+    assign(socket, :device, device)
+  end
 end

test/nerves_hub_web/channels/extensions_channel_test.exs

@@ -2,8 +2,10 @@ defmodule NervesHubWeb.ExtensionsChannelTest do
   use NervesHubWeb.ChannelCase
   use DefaultMocks
 
+  alias NervesHub.Devices
   alias NervesHub.Fixtures
   alias NervesHub.Products
+  alias NervesHub.Repo
   alias NervesHub.Support.Utils
   alias NervesHubWeb.DeviceChannel
   alias NervesHubWeb.DeviceSocket
@@ -23,6 +25,57 @@ defmodule NervesHubWeb.ExtensionsChannelTest do
     assert_push("extensions:get", _extensions)
   end
 
+  test "joining extensions channel works when the device has connected for the first time" do
+    user = Fixtures.user_fixture()
+    org = Fixtures.org_fixture(user)
+    product = Fixtures.product_fixture(user, org)
+
+    {:ok, device} =
+      Devices.create_device(%{
+        product_id: product.id,
+        org_id: org.id,
+        identifier: Ecto.UUID.generate()
+      })
+
+    %{db_cert: certificate, cert: _cert} = Fixtures.device_certificate_fixture(device)
+
+    {:ok, socket} =
+      connect(DeviceSocket, %{}, connect_info: %{peer_data: %{ssl_cert: certificate.der}})
+
+    # simulate the device channel updating the params
+    params = %{
+      "nerves_fw_uuid" => Ecto.UUID.generate(),
+      "nerves_fw_product" => product.name,
+      "nerves_fw_architecture" => "arm64",
+      "nerves_fw_version" => "0.0.0",
+      "nerves_fw_platform" => "test_host"
+    }
+
+    # taken from `DeviceChannel`, I don't love just stealing this, but it will do for now
+    with {:ok, metadata} <- NervesHub.Firmwares.metadata_from_device(params),
+         {:ok, device} <- NervesHub.Devices.update_firmware_metadata(device, metadata) do
+      NervesHub.Devices.firmware_update_successful(device)
+    end
+
+    assert {:ok, ["health"], extensions_channel} =
+             subscribe_and_join_with_default_device_api_version(
+               socket,
+               ExtensionsChannel,
+               "extensions",
+               %{"health" => "0.0.1"}
+             )
+
+    push(extensions_channel, "health:attached")
+    assert_push("health:check", _)
+
+    @endpoint.subscribe("device:#{device.id}:extensions")
+
+    push(extensions_channel, "health:report", %{"value" => dummy_health_report()})
+    assert_broadcast("health_check_report", _)
+
+    assert Repo.aggregate(Devices.DeviceHealth, :count) == 1
+  end
+
   test "joining extensions channel suggests attaching geo and health" do
     user = Fixtures.user_fixture()
     {device, _firmware, _deployment} = device_fixture(user, %{identifier: "123"})
@@ -332,6 +385,16 @@ defmodule NervesHubWeb.ExtensionsChannelTest do
     {device, firmware, deployment}
   end
 
+  def dummy_health_report() do
+    %{
+      alarms: %{},
+      checks: %{},
+      metadata: %{},
+      timestamp: "2025-01-20T20:00:37.106480Z",
+      connectivity: %{}
+    }
+  end
+
   defp subscribe_and_join_with_default_device_api_version(socket, channel, topic),
     do: subscribe_and_join(socket, channel, topic, %{"device_api_version" => "2.2.0"})