Unavailable secret error (#1371)

* Show an Add Secret button for System.EnvError

* Extracts render_formatted

* Rewrite the env error message

* Renames the env error type

* Applying suggestions

* Applying suggestions (live patch)

* Test for unavailable secrets

* Bump to rc.1

* Applying suggestions (error message)

* New error message

* Prefill the secret value

* Applying suggestions (error message output)

* Update test/livebook_web/live/session_live_test.exs

Co-authored-by: Jonatan Kłosko <jonatanklosko@gmail.com>

* Update lib/livebook/runtime.ex

Co-authored-by: Jonatan Kłosko <jonatanklosko@gmail.com>

Co-authored-by: Jonatan Kłosko <jonatanklosko@gmail.com>

.github/workflows/deploy.yaml

@@ -7,7 +7,7 @@ on:
       - "v*.*.*"
 env:
   otp: "25.0"
-  elixir: "1.14.0-rc.0"
+  elixir: "1.14.0-rc.1"
 jobs:
   assets:
     outputs:

.github/workflows/test.yaml

@@ -6,7 +6,7 @@ on:
       - main
 env:
   otp: "25.0"
-  elixir: "1.14.0-rc.0"
+  elixir: "1.14.0-rc.1"
 jobs:
   main:
     runs-on: ubuntu-latest

Dockerfile

@@ -1,6 +1,6 @@
 # Stage 1
 # Builds the Livebook release
-FROM hexpm/elixir:1.14.0-rc.0-erlang-24.3.4.2-debian-bullseye-20210902-slim AS build
+FROM hexpm/elixir:1.14.0-rc.1-erlang-24.3.4.2-debian-bullseye-20210902-slim AS build
 
 RUN apt-get update && apt-get upgrade -y && \
     apt-get install --no-install-recommends -y \
@@ -38,7 +38,7 @@ RUN mix do compile, release livebook
 # We use the same base image, because we need Erlang, Elixir and Mix
 # during runtime to spawn the Livebook standalone runtimes.
 # Consequently the release doesn't include ERTS as we have it anyway.
-FROM hexpm/elixir:1.14.0-rc.0-erlang-24.3.4.2-debian-bullseye-20210902-slim
+FROM hexpm/elixir:1.14.0-rc.1-erlang-24.3.4.2-debian-bullseye-20210902-slim
 
 RUN apt-get update && apt-get upgrade -y && \
     apt-get install --no-install-recommends -y \

lib/livebook/runtime.ex

@@ -61,7 +61,7 @@ defprotocol Livebook.Runtime do
           # A control element
           | {:control, attrs :: map()}
           # Internal output format for errors
-          | {:error, message :: binary()}
+          | {:error, message :: String.t(), type :: {:missing_secret, String.t()} | :other}
 
   @typedoc """
   Additional information about a complted evaluation.

lib/livebook/runtime/evaluator/default_formatter.ex

@@ -27,7 +27,7 @@ defmodule Livebook.Runtime.Evaluator.DefaultFormatter do
 
   def format_result({:error, kind, error, stacktrace}) do
     formatted = format_error(kind, error, stacktrace)
-    {:error, formatted}
+    {:error, formatted, error_type(error)}
   end
 
   @compile {:no_warn_undefined, {Kino.Render, :to_livebook, 1}}
@@ -128,4 +128,7 @@ defmodule Livebook.Runtime.Evaluator.DefaultFormatter do
   defp error_color(string) do
     IO.ANSI.format([:red, string], true)
   end
+
+  defp error_type(%System.EnvError{env: "LB_" <> secret}), do: {:missing_secret, secret}
+  defp error_type(_), do: :other
 end

lib/livebook_web/live/output.ex

@@ -2,9 +2,11 @@ defmodule LivebookWeb.Output do
   use Phoenix.Component
 
   import LivebookWeb.Helpers
+  import LivebookWeb.LiveHelpers
 
   alias Phoenix.LiveView.JS
   alias LivebookWeb.Output
+  alias LivebookWeb.Router.Helpers, as: Routes
 
   @doc """
   Renders a list of cell outputs.
@@ -32,7 +34,7 @@ defmodule LivebookWeb.Output do
 
   defp border?({:stdout, _text}), do: true
   defp border?({:text, _text}), do: true
-  defp border?({:error, _message}), do: true
+  defp border?({:error, _message, _type}), do: true
   defp border?({:grid, _, info}), do: Map.get(info, :boxed, false)
   defp border?(_output), do: false
 
@@ -233,19 +235,38 @@ defmodule LivebookWeb.Output do
     )
   end
 
-  defp render_output({:error, formatted}, %{}) do
-    assigns = %{message: formatted}
+  defp render_output({:error, formatted, {:missing_secret, secret}}, %{
+         socket: socket,
+         session_id: session_id
+       }) do
+    assigns = %{message: formatted, secret: secret}
 
     ~H"""
-    <div
-      class="whitespace-pre-wrap font-editor text-gray-500"
-      role="complementary"
-      aria-label="error"
-      phx-no-format
-    ><%= ansi_string_to_html(@message) %></div>
+    <div class="-m-4 space-x-4 py-4">
+      <div
+        class="flex items-center justify-between font-editor border-b px-4 pb-4 mb-4"
+        style="color: var(--ansi-color-red);"
+      >
+        <div class="flex space-x-2">
+          <.remix_icon icon="close-circle-line" />
+          <span>Missing secret <%= inspect(@secret) %></span>
+        </div>
+        <%= live_patch to: Routes.session_path(socket, :secrets, session_id, secret: secret),
+            class: "button-base button-gray",
+            aria_label: "add secret",
+            role: "button" do %>
+          <span>Add secret</span>
+        <% end %>
+      </div>
+      <%= render_formatted_error_message(@message) %>
+    </div>
     """
   end
 
+  defp render_output({:error, formatted, _type}, %{}) do
+    render_formatted_error_message(formatted)
+  end
+
   # TODO: remove on Livebook v0.7
   defp render_output(output, %{})
        when elem(output, 0) in [
@@ -279,4 +300,17 @@ defmodule LivebookWeb.Output do
     ><%= @message %></div>
     """
   end
+
+  defp render_formatted_error_message(formatted) do
+    assigns = %{message: formatted}
+
+    ~H"""
+    <div
+      class="whitespace-pre-wrap font-editor text-gray-500"
+      role="complementary"
+      aria-label="error"
+      phx-no-format
+    ><%= ansi_string_to_html(@message) %></div>
+    """
+  end
 end

lib/livebook_web/live/session_live.ex

@@ -404,6 +404,7 @@ defmodule LivebookWeb.SessionLive do
           module={LivebookWeb.SessionLive.SecretsComponent}
           id="secrets"
           session={@session}
+          secret={@secret}
         />
       </.modal>
     <% end %>
@@ -745,6 +746,12 @@ defmodule LivebookWeb.SessionLive do
     {:noreply, assign(socket, tab: tab)}
   end
 
+  def handle_params(params, _url, socket)
+      when socket.assigns.live_action == :secrets do
+    label = Map.get(params, "secret", "")
+    {:noreply, assign(socket, secret: %{"label" => label, "value" => ""})}
+  end
+
   def handle_params(_params, _url, socket) do
     {:noreply, socket}
   end

lib/livebook_web/live/session_live/secrets_component.ex

@@ -1,11 +1,6 @@
 defmodule LivebookWeb.SessionLive.SecretsComponent do
   use LivebookWeb, :live_component
 
-  @impl true
-  def mount(socket) do
-    {:ok, assign(socket, data: %{"label" => "", "value" => ""})}
-  end
-
   @impl true
   def update(assigns, socket) do
     socket = assign(socket, assigns)
@@ -25,7 +20,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
         </p>
         <.form
           let={f}
-          for={:data}
+          for={:secret}
           phx-submit="save"
           phx-change="validate"
           autocomplete="off"
@@ -37,7 +32,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
                 Label <span class="text-xs text-gray-500">(alphanumeric and underscore)</span>
               </div>
               <%= text_input(f, :label,
-                value: @data["label"],
+                value: @secret["label"],
                 class: "input",
                 placeholder: "secret label",
                 autofocus: true,
@@ -48,7 +43,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
             <div>
               <div class="input-label">Value</div>
               <%= text_input(f, :value,
-                value: @data["value"],
+                value: @secret["value"],
                 class: "input",
                 placeholder: "secret value",
                 aria_labelledby: "secret-value",
@@ -56,7 +51,7 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
               ) %>
             </div>
           </div>
-          <button class="mt-5 button-base button-blue" type="submit" disabled={not data_valid?(@data)}>
+          <button class="mt-5 button-base button-blue" type="submit" disabled={not valid?(@secret)}>
             Save
           </button>
         </.form>
@@ -66,17 +61,17 @@ defmodule LivebookWeb.SessionLive.SecretsComponent do
   end
 
   @impl true
-  def handle_event("save", %{"data" => data}, socket) do
-    secret = %{label: String.upcase(data["label"]), value: data["value"]}
+  def handle_event("save", %{"secret" => secret}, socket) do
+    secret = %{label: String.upcase(secret["label"]), value: secret["value"]}
     Livebook.Session.put_secret(socket.assigns.session.pid, secret)
-    {:noreply, assign(socket, data: %{"label" => "", "value" => ""})}
+    {:noreply, assign(socket, secret: %{"label" => "", "value" => ""})}
   end
 
-  def handle_event("validate", %{"data" => data}, socket) do
-    {:noreply, assign(socket, data: data)}
+  def handle_event("validate", %{"secret" => secret}, socket) do
+    {:noreply, assign(socket, secret: secret)}
   end
 
-  defp data_valid?(data) do
-    String.match?(data["label"], ~r/^\w+$/) and data["value"] != ""
+  defp valid?(secret) do
+    String.match?(secret["label"], ~r/^\w+$/) and secret["value"] != ""
   end
 end

mix.exs

@@ -5,7 +5,7 @@ defmodule Livebook.MixProject do
   @version "0.6.3"
   @description "Interactive and collaborative code notebooks - made with Phoenix LiveView"
 
-  @app_elixir_version "1.14.0-rc.0"
+  @app_elixir_version "1.14.0-rc.1"
   @app_rebar3_version "3.19.0"
 
   def project do

test/livebook_web/live/session_live_test.exs

@@ -913,16 +913,31 @@ defmodule LivebookWeb.SessionLiveTest do
     end
   end
 
-  describe "add secret" do
+  describe "secrets" do
     test "adds a secret from form", %{conn: conn, session: session} do
       {:ok, view, _} = live(conn, "/sessions/#{session.id}/secrets")
 
       view
       |> element(~s{form[phx-submit="save"]})
-      |> render_submit(%{data: %{label: "foo", value: "123"}})
+      |> render_submit(%{secret: %{label: "foo", value: "123"}})
 
       assert %{secrets: [%{label: "FOO", value: "123"}]} = Session.get_data(session.pid)
     end
+
+    test "shows the 'Add secret' button for unavailable secrets", %{conn: conn, session: session} do
+      Session.subscribe(session.id)
+      section_id = insert_section(session.pid)
+      cell_id = insert_text_cell(session.pid, section_id, :code, ~s{System.fetch_env!("LB_FOO")})
+
+      Session.queue_cell_evaluation(session.pid, cell_id)
+      assert_receive {:operation, {:add_cell_evaluation_response, _, ^cell_id, _, _}}
+
+      {:ok, view, _} = live(conn, "/sessions/#{session.id}")
+
+      assert view
+             |> element("span", "Add secret")
+             |> has_element?()
+    end
   end
 
   # Helpers