Report killed evals as errors and show UTF-8 as text

dialtone/README.md

@@ -95,7 +95,10 @@ construction. See `dialtone_backend_erlang` and repartee's
 
 - `logger` output goes to the server's console, not the client. Eval
   errors themselves are reported to the client with full stack traces.
-- Values are rendered with `~tp` and truncated beyond 8 MB.
+- Values are rendered with `~tp` and truncated beyond 8 MB. `~tp` shows
+  UTF-8 binaries and strings as text only on a node started with
+  `+pc unicode`, which `bin/dialtone` does. An embedded server prints
+  them the way your node does.
 
 ## Development
 

dialtone/bin/dialtone

@@ -7,4 +7,4 @@ EBIN="$DIR/_build/default/lib/dialtone/ebin"
 if [ ! -d "$EBIN" ]; then
     (cd "$DIR" && rebar3 compile >/dev/null) || exit 1
 fi
-exec erl -noshell -pa "$EBIN" -eval 'dialtone_cli:main()' -extra "$@"
+exec erl +pc unicode -noshell -pa "$EBIN" -eval 'dialtone_cli:main()' -extra "$@"

dialtone/src/dialtone_backend_erlang.erl

@@ -393,7 +393,15 @@ eval_forms([Exprs | Rest], Bindings0, State, EmitAll) ->
         true -> dialtone_backend:emit_value(format_value(Value));
         false -> ok
     end,
-    eval_forms(Rest, Bindings, State, EmitAll).
+    %% A later form that fails keeps the bindings of this one, as they
+    %% would be in the shell, where each form is a line of its own.
+    try
+        eval_forms(Rest, Bindings, State, EmitAll)
+    catch
+        Class:Reason:Stack ->
+            {error, dialtone_err:render(?MODULE, Class, Reason, Stack, State),
+             State#{bindings := Bindings}}
+    end.
 
 format_value(Value) ->
     io_lib:format("~tp", [Value]).

dialtone/src/dialtone_session.erl

@@ -25,7 +25,7 @@
                   running :: undefined
                            | #{req := map(), reply_to := pid(),
                                worker := pid(), mref := reference(),
-                               ref := reference()},
+                               ref := reference(), interrupted => true},
                   queue = queue:new() :: queue:queue({map(), pid()})}).
 
 start_link(Id, Backend, InitialBState) ->
@@ -97,16 +97,19 @@ handle_info({eval_result, Ref, Result},
     next(State#session{running = undefined, bstate = NewBState});
 handle_info({'DOWN', MRef, process, _Pid, Reason},
             #session{running = #{mref := MRef, req := Req,
-                                 reply_to := ReplyTo}} = State) ->
-    %% Worker died without reporting a result: killed (interrupt) or took an
-    %% exit we didn't catch (e.g. from a linked process). State unchanged.
+                                 reply_to := ReplyTo} = Running} = State) ->
+    %% Worker died without reporting a result: killed by an interrupt, or by
+    %% an exit we didn't catch (from a linked process, exit(self(), kill),
+    %% ...). Only the first is interrupted. State unchanged.
     ok = dialtone_io_server:reset(State#session.io),
-    case Reason of
-        killed ->
+    case {Running, Reason} of
+        %% A worker that died of something else before the interrupt's
+        %% kill reached it failed.
+        {#{interrupted := true}, killed} ->
             dialtone_msg:reply_done(ReplyTo, Req, [interrupted], #{});
-        Other ->
+        _ ->
             {BMod, _} = State#session.backend,
-            ErrMap = dialtone_err:render(BMod, exit, Other, [], State#session.bstate),
+            ErrMap = dialtone_err:render(BMod, exit, Reason, [], State#session.bstate),
             dialtone_msg:reply_done(ReplyTo, Req, ['eval-error'], ErrMap)
     end,
     next(State#session{running = undefined});
@@ -269,14 +272,14 @@ handle_interrupt(Req, ReplyTo, #session{running = undefined, queue = Q} = State)
             interrupt_queued(Req, ReplyTo, State)
     end;
 handle_interrupt(Req, ReplyTo,
-                 #session{running = #{req := RunningReq, worker := Worker}} = State) ->
+                 #session{running = #{req := RunningReq, worker := Worker} = Running} = State) ->
     RunningId = maps:get(<<"id">>, RunningReq, undefined),
     case maps:get(<<"interrupt-id">>, Req, RunningId) of
         RunningId ->
             exit(Worker, kill),
             %% The DOWN handler answers the interrupted eval; ack right away.
             dialtone_msg:reply_done(ReplyTo, Req, #{}),
-            State;
+            State#session{running = Running#{interrupted => true}};
         _Other ->
             case interrupt_queued_id(maps:get(<<"interrupt-id">>, Req), State) of
                 {found, State2} ->

dialtone/test/dialtone_SUITE.erl

@@ -16,8 +16,10 @@ all() ->
      clone_and_eval,
      bindings_persist,
      multi_form_eval,
+     multi_form_error_keeps_bindings,
      syntax_error,
      runtime_error,
+     eval_killed,
      unknown_op,
      unsupported_backend_op,
      missing_code,
@@ -135,6 +137,16 @@ multi_form_eval(Config) ->
     ?assertEqual([<<"1">>], [V || #{<<"value">> := V} <- Msgs3]),
     ?assert(lists:member(<<"eval-error">>, statuses_of(Msgs3))).
 
+%% The forms before the one that fails keep their bindings, which the
+%% client has already seen the values of.
+multi_form_error_keeps_bindings(Config) ->
+    {Session, Client} = clone(?config(client, Config)),
+    {Msgs, _} = eval(Client, Session, <<"Kept = 1. error(proof).">>),
+    ?assertEqual([<<"1">>], [V || #{<<"value">> := V} <- Msgs]),
+    ?assert(lists:member(<<"eval-error">>, statuses_of(Msgs))),
+    {Msgs2, _} = eval(?config(client, Config), Session, <<"Kept.">>),
+    ?assertEqual(<<"1">>, value_of(Msgs2)).
+
 syntax_error(Config) ->
     {Session, Client} = clone(?config(client, Config)),
     {Msgs, _} = eval(Client, Session, <<"1 +.">>),
@@ -153,6 +165,17 @@ runtime_error(Config) ->
     {Msgs2, _} = eval(?config(client, Config), Session, <<"ok.">>),
     ?assertEqual(<<"ok">>, value_of(Msgs2)).
 
+%% Only an interrupt makes an eval interrupted. A worker that gets killed
+%% some other way failed.
+eval_killed(Config) ->
+    {Session, Client} = clone(?config(client, Config)),
+    {Msgs, _} = eval(Client, Session, <<"exit(self(), kill).">>),
+    Statuses = statuses_of(Msgs),
+    ?assert(lists:member(<<"eval-error">>, Statuses)),
+    ?assertNot(lists:member(<<"interrupted">>, Statuses)),
+    {Msgs2, _} = eval(?config(client, Config), Session, <<"ok.">>),
+    ?assertEqual(<<"ok">>, value_of(Msgs2)).
+
 unknown_op(Config) ->
     {[Resp], _} = dialtone_client:request(?config(client, Config),
                                           #{<<"op">> => <<"frobnicate">>}),