Improve debugging discoverability on app errors (#2061)

Co-authored-by: José Valim <jose.valim@dashbit.co>
This commit is contained in:
Jonatan Kłosko 2023-07-11 22:29:38 +02:00 committed by GitHub
parent bbc5ced0d5
commit ff30d0de2d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 54 additions and 21 deletions

View file

@ -154,7 +154,7 @@ defmodule Livebook.Session.Data do
# Note that technically the first state is :initial, but we always # Note that technically the first state is :initial, but we always
# expect app to start evaluating right away, so distinguishing that # expect app to start evaluating right away, so distinguishing that
# state from :executing would not bring any value # state from :executing would not bring any value
execution: :executing | :executed | :error, execution: :executing | :executed | :error | :interrupted,
lifecycle: :active | :shutting_down | :deactivated lifecycle: :active | :shutting_down | :deactivated
} }

View file

@ -139,12 +139,6 @@ defmodule LivebookWeb.AppSessionLive do
</h1> </h1>
</div> </div>
<div class="pt-4 flex flex-col space-y-6" data-el-outputs-container id="outputs"> <div class="pt-4 flex flex-col space-y-6" data-el-outputs-container id="outputs">
<%= if @data_view.app_status.execution == :error do %>
<div class="error-box flex items-center gap-2">
<.remix_icon icon="error-warning-line" class="text-xl" />
<span>Something went wrong</span>
</div>
<% else %>
<div :for={output_view <- Enum.reverse(@data_view.output_views)}> <div :for={output_view <- Enum.reverse(@data_view.output_views)}>
<LivebookWeb.Output.outputs <LivebookWeb.Output.outputs
outputs={[output_view.output]} outputs={[output_view.output]}
@ -156,6 +150,20 @@ defmodule LivebookWeb.AppSessionLive do
input_values={output_view.input_values} input_values={output_view.input_values}
/> />
</div> </div>
<%= if @data_view.app_status.execution == :error do %>
<div class="error-box flex items-center justify-between">
<div class="flex items-center gap-2">
<.remix_icon icon="error-warning-line" class="text-xl" />
<span>Something went wrong</span>
</div>
<.link
:if={@livebook_authenticated?}
navigate={~p"/sessions/#{@session.id}" <> "#cell-#{@data_view.errored_cell_id}"}
>
<span>Debug</span>
<.remix_icon icon="arrow-right-line" />
</.link>
</div>
<% end %> <% end %>
</div> </div>
<div style="height: 80vh"></div> <div style="height: 80vh"></div>
@ -303,10 +311,19 @@ defmodule LivebookWeb.AppSessionLive do
app_status: data.app_data.status, app_status: data.app_data.status,
show_source: data.notebook.app_settings.show_source, show_source: data.notebook.app_settings.show_source,
slug: data.notebook.app_settings.slug, slug: data.notebook.app_settings.slug,
multi_session: data.notebook.app_settings.multi_session multi_session: data.notebook.app_settings.multi_session,
errored_cell_id: errored_cell_id(data)
} }
end end
defp errored_cell_id(data) do
data.notebook
|> Notebook.evaluable_cells_with_section()
|> Enum.find_value(fn {cell, _section} ->
data.cell_infos[cell.id].eval.errored && cell.id
end)
end
defp input_values_for_output(output, data) do defp input_values_for_output(output, data) do
input_ids = for attrs <- Cell.find_inputs_in_output(output), do: attrs.id input_ids = for attrs <- Cell.find_inputs_in_output(output), do: attrs.id
Map.take(data.input_values, input_ids) Map.take(data.input_values, input_ids)

View file

@ -62,6 +62,17 @@ defmodule LivebookWeb.SessionLive do
session = Session.get_by_pid(session_pid) session = Session.get_by_pid(session_pid)
platform = platform_from_socket(socket) platform = platform_from_socket(socket)
socket =
if data.mode == :app do
put_flash(
socket,
:info,
"This session is a deployed app. Any changes will be reflected live."
)
else
socket
end
{:ok, {:ok,
socket socket
|> assign( |> assign(

View file

@ -108,7 +108,7 @@ defmodule LivebookWeb.AppSessionLiveTest do
Livebook.App.close(app.pid) Livebook.App.close(app.pid)
end end
test "only shows an error message when session errors", %{conn: conn} do test "shows an error message when session errors", %{conn: conn} do
slug = Livebook.Utils.random_short_id() slug = Livebook.Utils.random_short_id()
app_settings = %{Livebook.Notebook.AppSettings.new() | slug: slug} app_settings = %{Livebook.Notebook.AppSettings.new() | slug: slug}
@ -125,7 +125,8 @@ defmodule LivebookWeb.AppSessionLiveTest do
}, },
%{ %{
Livebook.Notebook.Cell.new(:code) Livebook.Notebook.Cell.new(:code)
| source: ~s/raise "oops"/ | source: ~s/raise "oops"/,
id: "error-cell"
} }
] ]
} }
@ -138,12 +139,16 @@ defmodule LivebookWeb.AppSessionLiveTest do
assert_receive {:app_created, %{pid: ^app_pid} = app} assert_receive {:app_created, %{pid: ^app_pid} = app}
assert_receive {:app_updated, assert_receive {:app_updated,
%{pid: ^app_pid, sessions: [%{app_status: %{execution: :error}}]}} %{
pid: ^app_pid,
sessions: [%{id: session_id, app_status: %{execution: :error}}]
}}
{:ok, view, _} = conn |> live(~p"/apps/#{slug}") |> follow_redirect(conn) {:ok, view, _} = conn |> live(~p"/apps/#{slug}") |> follow_redirect(conn)
assert render(view) =~ "Printed output"
assert render(view) =~ "Something went wrong" assert render(view) =~ "Something went wrong"
refute render(view) =~ "Printed output" assert render(view) =~ ~p"/sessions/#{session_id}" <> "#cell-error-cell"
Livebook.App.close(app.pid) Livebook.App.close(app.pid)
end end