Repository: livebook-dev/kino
Pull request #453 (2024-07-03 09:26:08)

Fix dbg pipeline when result renders as Kino.JS.Live
Currently a pipeline with Explorer operations and `|> dbg()` hangs and times out. The pipeline `c:Kino.JS.Live.init/1` calls `Kino.Frame.render/2`, which may result in `Kino.start_child/1` underneath. In the past `Kino.Frame.render/2` used `GenServer.cast`, so the `init` callback would not block, finish and unlock the other `Kino.start_child/1`. However, we changed `Kino.Frame.render/2` to use `GenServer.call` (https://github.com/livebook-dev/kino/commit/aae7a1b0a22f1b5273a5d058c5831b916513438d), so now `init` blocks, but the nested `Kino.start_child/1` hangs, until `init` times out.

To fix it, I moved `Kino.Frame.render/2` from `init` to a subsequent `handle_info`.


Patch:
diff --git a/lib/kino.ex b/lib/kino.ex
index ac387e4..bb5066f 100644
--- a/lib/kino.ex
+++ b/lib/kino.ex
@@ -428,9 +428,11 @@ defmodule Kino do
   > or `c:Kino.JS.Live.init/2`. If you do that, starting the process
   > will block forever.
   >
-  > Note that creating many kinos uses `start_child/1` underneath,
-  > hence the same restriction applies to starting those. See
-  > `c:Kino.JS.Live.init/2` for more details.
+  > On creation, many kinos use `start_child/1` underneath, which means
+  > that you cannot use functions such as `Kino.DataTable.new/1` in
+  > `c:GenServer.init/1`. If you need to do that, you must either
+  > create the kinos beforehand and pass in the `GenServer` argument,
+  > or create them in `c:GenServer.handle_continue/2`.
   """
   @spec start_child(
           Supervisor.child_spec()
diff --git a/lib/kino/debug/pipeline.ex b/lib/kino/debug/pipeline.ex
index 0393568..c5a44e6 100644
--- a/lib/kino/debug/pipeline.ex
+++ b/lib/kino/debug/pipeline.ex
@@ -31,6 +31,8 @@ defmodule Kino.Debug.Pipeline do
 
     %{id: last_id} = List.last(items)
 
+    send(self(), :update_result_frame)
+
     {:ok,
      ctx
      |> assign(
@@ -44,8 +46,7 @@ defmodule Kino.Debug.Pipeline do
        error: nil,
        call_count: 1,
        changed?: false
-     )
-     |> update_result_frame()}
+     )}
   end
 
   @impl true
@@ -127,6 +128,10 @@ defmodule Kino.Debug.Pipeline do
     {:noreply, ctx}
   end
 
+  def handle_info(:update_result_frame, ctx) do
+    {:noreply, update_result_frame(ctx)}
+  end
+
   defp handle_items_change(ctx, items) do
     id =
       ctx.assigns.items
diff --git a/lib/kino/js/live.ex b/lib/kino/js/live.ex
index 3a92dc1..d71663e 100644
--- a/lib/kino/js/live.ex
+++ b/lib/kino/js/live.ex
@@ -202,44 +202,8 @@ defmodule Kino.JS.Live do
 
   See `c:GenServer.init/1` for more details.
 
-  > #### Starting other kinos {: .warning}
-  >
-  > It is generally not possible to start kinos inside the `c:init/2`
-  > callback, as such operation would block forever. In case you need
-  > to start other kinos during initialization, you must start them
-  > beforehand and pass as an argument to `c:init/2`. So instead of
-  >
-  >     defmodule KinoDocs.Custom do
-  >       def new() do
-  >         Kino.JS.Live.new(__MODULE__, {})
-  >       end
-  >
-  >       @impl true
-  >       def init({}, ctx) do
-  >         frame = Kino.Frame.new()
-  >         {:ok, assign(ctx, frame: frame)}
-  >       end
-  >
-  >       ...
-  >     end
-  >
-  > do the following
-  >
-  >     defmodule KinoDocs.Custom do
-  >       def new() do
-  >         frame = Kino.Frame.new()
-  >         Kino.JS.Live.new(__MODULE__, {frame})
-  >       end
-  >
-  >       @impl true
-  >       def init({frame}, ctx) do
-  >         {:ok, assign(ctx, frame: frame)}
-  >       end
-  >
-  >       ...
-  >     end
-  >
-  > Also see `Kino.start_child/1`.
+  If you want to create other kinos on initialization, see the
+  limitations described in `Kino.start_child/1`.
   """
   @callback init(arg :: term(), ctx :: Context.t()) ::
               {:ok, ctx :: Context.t()} | {:ok, ctx :: Context.t(), opts :: keyword()}


Test patch:
diff --git a/test/kino/debug_test.exs b/test/kino/debug_test.exs
index 7eff841..aa882a4 100644
--- a/test/kino/debug_test.exs
+++ b/test/kino/debug_test.exs
@@ -54,6 +54,23 @@ defmodule Kino.Debug.Test do
       assert dbg_line == __ENV__.line - 28
     end
 
+    test "initial render with result requiring Kino.start_child/1" do
+      # Rendering %UserAsTable{} creates a Kino.JS.Live
+      call_dbg(
+        %UserAsTable{id: 1}
+        |> Map.replace!(:id, 2)
+        |> Map.replace!(:id, 3)
+      )
+
+      {_kino, _output, frame_ref} = assert_dbg_pipeline_render()
+
+      assert_output(%{
+        type: :frame_update,
+        ref: ^frame_ref,
+        update: {:replace, [%{type: :js}]}
+      })
+    end
+
     test "updates result when a pipeline step is disabled" do
       call_dbg(
         1..5
diff --git a/test/support/test_modules/user_as_table.ex b/test/support/test_modules/user_as_table.ex
new file mode 100644
index 0000000..c5eb423
--- /dev/null
+++ b/test/support/test_modules/user_as_table.ex
@@ -0,0 +1,9 @@
+defmodule UserAsTable do
+  defstruct [:id]
+
+  defimpl Kino.Render do
+    def to_livebook(user) do
+      Kino.DataTable.new([%{id: user.id}]) |> Kino.Render.to_livebook()
+    end
+  end
+end