Use scripts plumbing for connecting code (#2399)

Solves #2395. 

This uses `scripts/run` to push connecting code to the device rather
than broadcasting it to every device node.

I added telemetry events for when the connecting code succeeds or fails,
as well as logging when it fails. The contract for "failing" a script
run is in NHL, which I don't love, but can be fixed at a later time.

---------

Co-authored-by: Jon Carstens <[email protected]>
Co-authored-by: Jon Carstens <[email protected]>

Author: nshoes

lib/nerves_hub/logger.ex

@@ -30,6 +30,7 @@ defmodule NervesHub.Logger do
         [:phoenix, :endpoint, :stop],
         [:nerves_hub, :devices, :invalid_auth],
         [:nerves_hub, :devices, :connect],
+        [:nerves_hub, :devices, :connecting_code_failure],
         [:nerves_hub, :devices, :disconnect],
         [:nerves_hub, :devices, :duplicate_connection],
         [:nerves_hub, :devices, :update, :automatic],
@@ -94,6 +95,14 @@ defmodule NervesHub.Logger do
     )
   end
 
+  def log_event([:nerves_hub, :devices, :connecting_code_failure], _, metadata, _) do
+    Logger.info("Connecting code failure",
+      event: "nerves_hub.devices.connecting_code_failure",
+      output: metadata[:output],
+      identifier: metadata[:identifier]
+    )
+  end
+
   def log_event([:nerves_hub, :devices, :duplicate_connection], _, metadata, _) do
     Logger.info("Device duplicate connection detected",
       event: "nerves_hub.devices.duplicate_connection",

lib/nerves_hub/scripts/runner.ex

@@ -62,11 +62,7 @@ defmodule NervesHub.Scripts.Runner do
   def handle_info({:error, :incompatible_version}, state) do
     text = ~s/#{state.text}\n# [NERVESHUB:END]/
 
-    text
-    |> String.graphemes()
-    |> Enum.each(fn character ->
-      Endpoint.broadcast_from!(self(), state.send_channel, "dn", %{"data" => character})
-    end)
+    Endpoint.broadcast_from!(self(), state.send_channel, "dn", %{"data" => text})
 
     _ = Endpoint.subscribe(state.receive_channel)
 

lib/nerves_hub_web/channels/console_channel.ex

@@ -2,7 +2,6 @@
 defmodule NervesHubWeb.ConsoleChannel do
   use Phoenix.Channel
 
-  alias NervesHub.Repo
   alias Phoenix.Socket.Broadcast
 
   def join("console", payload, %{assigns: %{device: device}} = socket) do
@@ -75,30 +74,6 @@ defmodule NervesHubWeb.ConsoleChannel do
       %{}
     )
 
-    # now that the console is connected, push down the device's elixir, line by line
-    device = socket.assigns.device
-    device = Repo.preload(device, [:deployment_group])
-
-    if device.deployment_group && device.deployment_group.connecting_code do
-      device.deployment_group.connecting_code
-      |> String.graphemes()
-      |> Enum.each(fn character ->
-        push(socket, "dn", %{"data" => character})
-      end)
-
-      push(socket, "dn", %{"data" => "\r"})
-    end
-
-    if device.connecting_code do
-      device.connecting_code
-      |> String.graphemes()
-      |> Enum.each(fn character ->
-        push(socket, "dn", %{"data" => character})
-      end)
-
-      push(socket, "dn", %{"data" => "\r"})
-    end
-
     {:noreply, socket}
   end
 

lib/nerves_hub_web/channels/device_channel.ex

@@ -63,7 +63,36 @@ defmodule NervesHubWeb.DeviceChannel do
 
   def handle_info({:after_join, params}, socket) do
     %{device: device, reference_id: reference_id} = socket.assigns
+
+    # :deployment_group is manually set to nil in DeviceLink, need to force reload here
+    device = NervesHub.Repo.preload(device, :deployment_group, force: true)
+
+    connecting_codes =
+      [
+        get_in(device, [Access.key(:deployment_group), Access.key(:connecting_code)]),
+        device.connecting_code
+      ]
+      |> Enum.filter(&(not is_nil(&1) and byte_size(&1) > 0))
+
+    case [safe_to_run_scripts?(socket), Enum.empty?(connecting_codes)] do
+      [true, false] ->
+        connecting_code = Enum.join(connecting_codes, "\n")
+        # connecting code first incase it attempts to change things before the other messages
+        push(socket, "scripts/run", %{"text" => connecting_code, "ref" => "connecting_code"})
+
+      [false, false] ->
+        connecting_code = Enum.join(connecting_codes, "\n")
+        text = ~s/#{connecting_code}\n\r/
+        topic = "device:console:#{device.id}"
+
+        socket.endpoint.broadcast_from!(self(), topic, "dn", %{"data" => text})
+
+      _ ->
+        :ok
+    end
+
     :ok = DeviceLink.after_join(device, reference_id, params)
+
     {:noreply, socket}
   end
 
@@ -80,7 +109,7 @@ defmodule NervesHubWeb.DeviceChannel do
   end
 
   def handle_info({:run_script, pid, text}, socket) do
-    if Version.match?(socket.assigns.device_api_version, ">= 2.1.0") do
+    if safe_to_run_scripts?(socket) do
       ref = Base.encode64(:crypto.strong_rand_bytes(4), padding: false)
 
       push(socket, "scripts/run", %{"text" => text, "ref" => ref})
@@ -172,8 +201,28 @@ defmodule NervesHubWeb.DeviceChannel do
     {:noreply, socket}
   end
 
+  def handle_in(
+        "scripts/run",
+        %{"ref" => "connecting_code", "result" => result, "return" => return, "output" => output},
+        socket
+      )
+      when result == "error" or return == "nil" do
+    :telemetry.execute([:nerves_hub, :devices, :connecting_code_failure], %{
+      output: output,
+      identifier: socket.assigns.device.identifier
+    })
+
+    {:noreply, socket}
+  end
+
+  def handle_in("scripts/run", %{"ref" => "connecting_code"}, socket) do
+    :telemetry.execute([:nerves_hub, :devices, :connecting_code_success], %{count: 1})
+
+    {:noreply, socket}
+  end
+
   def handle_in("scripts/run", params, socket) do
-    if pid = socket.assigns.script_refs[params["ref"]] do
+    if pid = get_in(socket.assigns, [:script_refs, params["ref"]]) do
       output = Enum.join([params["output"], params["return"]], "\n")
       output = String.trim(output)
       send(pid, {:output, output})
@@ -260,4 +309,6 @@ defmodule NervesHubWeb.DeviceChannel do
       "deployment:#{device.deployment_id}"
     end
   end
+
+  defp safe_to_run_scripts?(socket), do: Version.match?(socket.assigns.device_api_version, ">= 2.1.0")
 end

test/support/socket_client.ex

@@ -188,6 +188,10 @@ defmodule SocketClient do
     {:ok, socket}
   end
 
+  def handle_message("device", "scripts/run", %{}, socket) do
+    {:ok, socket}
+  end
+
   @impl Slipstream
   def handle_call(:connected?, _from, socket) do
     {:reply, socket.assigns.connected?, socket}