Improve DAP Error Handling (erlang-ls#1246)

* Single message for distribution shutdown, with warning severity

* Treat 'ignored' from net_kernel as regular error, instead of crashing

* Do not silently fail DAP in case of distribution errors

* Fix error responses

apps/els_core/src/els_distribution_server.erl

@@ -57,14 +57,15 @@ start_link() ->
  gen_server:start_link({local, ?SERVER}, ?MODULE, unused, []).
 
 %% @doc Turns a non-distributed node into a distributed one
--spec start_distribution(atom()) -> ok.
+-spec start_distribution(atom()) -> ok | {error, any()}.
 start_distribution(Name) ->
  Cookie = els_config_runtime:get_cookie(),
  RemoteNode = els_config_runtime:get_node_name(),
  NameType = els_config_runtime:get_name_type(),
  start_distribution(Name, RemoteNode, Cookie, NameType).
 
--spec start_distribution(atom(), atom(), atom(), shortnames | longnames) -> ok.
+-spec start_distribution(atom(), atom(), atom(), shortnames | longnames) ->
+ ok | {error, any()}.
 start_distribution(Name, RemoteNode, Cookie, NameType) ->
  ?LOG_INFO("Enable distribution [name=~p]", [Name]),
  case net_kernel:start([Name, NameType]) of
@@ -75,12 +76,14 @@ start_distribution(Name, RemoteNode, Cookie, NameType) ->
  CustomCookie ->
  erlang:set_cookie(RemoteNode, CustomCookie)
  end,
- ?LOG_INFO("Distribution enabled [name=~p]", [Name]);
+ ?LOG_INFO("Distribution enabled [name=~p]", [Name]),
+ ok;
  {error, {already_started, _Pid}} ->
- ?LOG_INFO("Distribution already enabled [name=~p]", [Name]);
- {error, {{shutdown, {failed_to_start_child, net_kernel, E1}}, E2}} ->
- ?LOG_INFO("Distribution shutdown [errs=~p]", [{E1, E2}]),
- ?LOG_INFO("Distribution shut down [name=~p]", [Name])
+ ?LOG_INFO("Distribution already enabled [name=~p]", [Name]),
+ ok;
+ {error, Error} ->
+ ?LOG_WARNING("Distribution shutdown [error=~p] [name=~p]", [Error, Name]),
+ {error, Error}
  end.
 
 %% @doc Connect to an existing runtime node, if available, or start one.
@@ -152,6 +155,8 @@ connect_and_monitor(Node, Type) ->
  erlang:monitor_node(Node, true),
  ok;
  false ->
+ error;
+ ignored ->
  error
  end.
 

apps/els_dap/src/els_dap_general_provider.erl

@@ -80,7 +80,8 @@ init() ->
  , timeout => 30
  , mode => undefined}.
 
--spec handle_request(request(), state()) -> {result(), state()}.
+-spec handle_request(request(), state()) ->
+ {result(), state()} | {{error, binary()}, state()}.
 handle_request({<<"initialize">>, _Params}, State) ->
  %% quick fix to satisfy els_config initialization
  {ok, RootPath} = file:get_cwd(),
@@ -90,59 +91,63 @@ handle_request({<<"initialize">>, _Params}, State) ->
  ok = els_config:initialize(RootUri, Capabilities, InitOptions),
  {Capabilities, State};
 handle_request({<<"launch">>, #{<<"cwd">> := Cwd} = Params}, State) ->
- #{ <<"projectnode">> := ProjectNode
- , <<"cookie">> := Cookie
- , <<"timeout">> := TimeOut
- , <<"use_long_names">> := UseLongNames} = start_distribution(Params),
- case Params of
- #{ <<"runinterminal">> := Cmd
- } ->
- ParamsR
- = #{ <<"kind">> => <<"integrated">>
- , <<"title">> => ProjectNode
- , <<"cwd">> => Cwd
- , <<"args">> => Cmd
- },
- ?LOG_INFO("Sending runinterminal request: [~p]", [ParamsR]),
- els_dap_server:send_request(<<"runInTerminal">>, ParamsR),
- ok;
- _ ->
- NameTypeParam = case UseLongNames of
- true ->
- "--name";
- false ->
- "--sname"
- end,
- ?LOG_INFO("launching 'rebar3 shell`", []),
- spawn(fun() ->
- els_utils:cmd(
- "rebar3",
- [ "shell"
- , NameTypeParam
- , atom_to_list(ProjectNode)
- , "--setcookie"
- , erlang:binary_to_list(Cookie)
- ]
- )
- end)
- end,
-
- els_dap_server:send_event(<<"initialized">>, #{}),
-
- {#{}, State#{ project_node => ProjectNode
- , launch_params => Params
- , timeout => TimeOut
- }};
+ case start_distribution(Params) of
+ {ok, #{ <<"projectnode">> := ProjectNode
+ , <<"cookie">> := Cookie
+ , <<"timeout">> := TimeOut
+ , <<"use_long_names">> := UseLongNames}} ->
+ case Params of
+ #{ <<"runinterminal">> := Cmd
+ } ->
+ ParamsR
+ = #{ <<"kind">> => <<"integrated">>
+ , <<"title">> => ProjectNode
+ , <<"cwd">> => Cwd
+ , <<"args">> => Cmd
+ },
+ ?LOG_INFO("Sending runinterminal request: [~p]", [ParamsR]),
+ els_dap_server:send_request(<<"runInTerminal">>, ParamsR),
+ ok;
+ _ ->
+ NameTypeParam = case UseLongNames of
+ true ->
+ "--name";
+ false ->
+ "--sname"
+ end,
+ ?LOG_INFO("launching 'rebar3 shell`", []),
+ spawn(fun() ->
+ els_utils:cmd(
+ "rebar3",
+ [ "shell"
+ , NameTypeParam
+ , atom_to_list(ProjectNode)
+ , "--setcookie"
+ , erlang:binary_to_list(Cookie)
+ ]
+ )
+ end)
+ end,
+ els_dap_server:send_event(<<"initialized">>, #{}),
+ {#{}, State#{ project_node => ProjectNode
+ , launch_params => Params
+ , timeout => TimeOut
+ }};
+ {error, Error} ->
+ {{error, distribution_error(Error)}, State}
+ end;
 handle_request({<<"attach">>, Params}, State) ->
- #{ <<"projectnode">> := ProjectNode
- , <<"timeout">> := TimeOut} = start_distribution(Params),
-
- els_dap_server:send_event(<<"initialized">>, #{}),
-
- {#{}, State#{ project_node => ProjectNode
- , launch_params => Params
- , timeout => TimeOut
- }};
+ case start_distribution(Params) of
+ {ok, #{ <<"projectnode">> := ProjectNode
+ , <<"timeout">> := TimeOut}} ->
+ els_dap_server:send_event(<<"initialized">>, #{}),
+ {#{}, State#{ project_node => ProjectNode
+ , launch_params => Params
+ , timeout => TimeOut
+ }};
+ {error, Error} ->
+ {{error, distribution_error(Error)}, State}
+ end;
 handle_request( {<<"configurationDone">>, _Params}
  , #{ project_node := ProjectNode
  , launch_params := LaunchParams
@@ -422,7 +427,10 @@ handle_request({<<"disconnect">>, _Params}
  <<"launch">> ->
  els_dap_rpc:halt(ProjectNode)
  end,
- els_utils:halt(0),
+ stop_debugger(),
+ {#{}, State};
+handle_request({<<"disconnect">>, _Params}, State) ->
+ stop_debugger(),
  {#{}, State}.
 
 -spec evaluate_condition(els_dap_breakpoints:line_breaks(), module(),
@@ -848,7 +856,7 @@ ensure_connected(Node, Timeout) ->
 stop_debugger() ->
  %% the project node is down, there is nothing left to do then to exit
  els_dap_server:send_event(<<"terminated">>, #{}),
- els_dap_server:send_event(<<"exited">>, #{ <<"exitCode">> => <<"0">>}),
+ els_dap_server:send_event(<<"exited">>, #{ <<"exitCode">> => 0 }),
  ?LOG_NOTICE("terminating debug adapter"),
  els_utils:halt(0).
 
@@ -887,7 +895,7 @@ check_project_node_name(ProjectNode, true) ->
  binary_to_atom(ProjectNode, utf8)
  end.
 
--spec start_distribution(map()) -> map().
+-spec start_distribution(map()) -> {ok, map()} | {error, any()}.
 start_distribution(Params) ->
  #{<<"cwd">> := Cwd} = Params,
  ok = file:set_cwd(Cwd),
@@ -921,8 +929,18 @@ start_distribution(Params) ->
  %% start distribution
  LocalNode = els_distribution_server:node_name("erlang_ls_dap",
  binary_to_list(Name), NameType),
- els_distribution_server:start_distribution(LocalNode, ConfProjectNode,
- Cookie, NameType),
- ?LOG_INFO("Distribution up on: [~p]", [LocalNode]),
+ case els_distribution_server:start_distribution(LocalNode, ConfProjectNode,
+ Cookie, NameType) of
+ ok ->
+ ?LOG_INFO("Distribution up on: [~p]", [LocalNode]),
+ {ok, Config#{ <<"projectnode">> => ConfProjectNode}};
+ {error, Error} ->
+ ?LOG_ERROR("Cannot start distribution for ~p", [LocalNode]),
+ {error, Error}
+ end.
 
- Config#{ <<"projectnode">> => ConfProjectNode}.
+-spec distribution_error(any()) -> binary().
+distribution_error(Error) ->
+ els_utils:to_binary(
+ lists:flatten(
+ io_lib:format("Could not start Erlang distribution. ~p", [Error]))).

apps/els_dap/src/els_dap_methods.erl

@@ -36,21 +36,27 @@ dispatch(Command, Args, Type, State) ->
  Type:Reason:Stack ->
  ?LOG_ERROR( "Unexpected error [type=~p] [error=~p] [stack=~p]"
  , [Type, Reason, Stack]),
- Error = #{ code => ?ERR_UNKNOWN_ERROR_CODE
- , message => <<"Unexpected error while ", Command/binary>>
- },
+ Error = <<"Unexpected error while ", Command/binary>>,
  {error_response, Error, State}
  end.
 
 -spec do_dispatch(atom(), params(), state()) -> result().
 do_dispatch(Command, Args, #{status := initialized} = State) ->
  Request = {Command, Args},
- Result = els_provider:handle_request(els_dap_general_provider, Request),
- {response, Result, State};
+ case els_provider:handle_request(els_dap_general_provider, Request) of
+ {error, Error} ->
+ {error_response, Error, State};
+ Result ->
+ {response, Result, State}
+ end;
 do_dispatch(<<"initialize">>, Args, State) ->
  Request = {<<"initialize">>, Args},
- Result = els_provider:handle_request(els_dap_general_provider, Request),
- {response, Result, State#{status => initialized}};
+ case els_provider:handle_request(els_dap_general_provider, Request) of
+ {error, Error} ->
+ {error_response, Error, State};
+ Result ->
+ {response, Result, State#{status => initialized}}
+ end;
 do_dispatch(_Command, _Args, State) ->
  Message = <<"The server is not fully initialized yet, please wait.">>,
  Result = #{ code => ?ERR_SERVER_NOT_INITIALIZED

apps/els_dap/src/els_dap_protocol.erl

@@ -60,13 +60,16 @@ response(Seq, Command, Result) ->
  ?LOG_DEBUG("[Response] [message=~p]", [Message]),
  content(jsx:encode(Message)).
 
--spec error_response(number(), any(), any()) -> binary().
+-spec error_response(number(), any(), binary()) -> binary().
 error_response(Seq, Command, Error) ->
  Message = #{ type => <<"response">>
  , request_seq => Seq
  , success => false
  , command => Command
- , body => #{ error => Error
+ , body => #{ error => #{ id => Seq
+ , format => Error
+ , showUser => true
+ }
  }
  },
  ?LOG_DEBUG("[Response] [message=~p]", [Message]),