Merge pull request #914 from TheGeorge/dap_cookie

[dap] improve dap startup and shutdown

apps/els_core/src/els_distribution_server.erl

@@ -155,24 +155,32 @@ start(Node) ->
   wait_connect_and_monitor(Node),
   ok.
 
--spec wait_connect_and_monitor(atom()) -> ok.
+-spec wait_connect_and_monitor(atom()) ->  ok | error.
 wait_connect_and_monitor(Node) ->
   wait_connect_and_monitor(Node, ?WAIT_ATTEMPTS).
 
--spec wait_connect_and_monitor(Node :: atom(), Attempts :: pos_integer()) -> ok.
-wait_connect_and_monitor(Node, 0) ->
-  ?LOG_ERROR( "Failed to connect to node ~p after ~p attempts"
-            , [Node, ?WAIT_ATTEMPTS]),
-  ok;
+-spec wait_connect_and_monitor(Node :: atom(), Attempts :: pos_integer()) ->  ok | error.
 wait_connect_and_monitor(Node, Attempts) ->
+  wait_connect_and_monitor(Node, Attempts, Attempts).
+
+-spec wait_connect_and_monitor(
+  Node :: atom(),
+  Attempts :: pos_integer(),
+  MaxAttempts :: pos_integer()
+) -> ok | error.
+wait_connect_and_monitor(Node, 0, MaxAttempts) ->
+  ?LOG_ERROR( "Failed to connect to node ~p after ~p attempts"
+            , [Node, MaxAttempts]),
+  error;
+wait_connect_and_monitor(Node, Attempts, MaxAttempts) ->
   timer:sleep(?WAIT_INTERVAL),
   case connect_and_monitor(Node) of
     ok ->
       ok;
     error ->
       ?LOG_WARNING( "Trying to connect to node ~p (~p/~p)"
-                  , [Node, ?WAIT_ATTEMPTS - Attempts + 1, ?WAIT_ATTEMPTS]),
-      wait_connect_and_monitor(Node, Attempts - 1)
+                  , [Node, MaxAttempts - Attempts + 1, MaxAttempts]),
+      wait_connect_and_monitor(Node, Attempts - 1, MaxAttempts)
   end.
 
 %% @doc Ensure the Erlang Port Mapper Daemon (EPMD) is up and running

apps/els_dap/src/els_dap_general_provider.erl

@@ -52,6 +52,7 @@
                          , scope_bindings =>
                           #{pos_integer() => {binding_type(), bindings()}}
                          , breakpoints := breakpoints()
+                         , timeout := timeout()
                          }.
 -type bindings()     :: [{varname(), term()}].
 -type varname()      :: atom() | string().
@@ -72,7 +73,8 @@ init() ->
   #{ threads => #{}
    , launch_params => #{}
    , scope_bindings => #{}
-   , breakpoints => #{}}.
+   , breakpoints => #{}
+   , timeout => 30}.
 
 -spec handle_request(request(), state()) -> {result(), state()}.
 handle_request({<<"initialize">>, _Params}, State) ->
@@ -86,11 +88,32 @@ handle_request({<<"launch">>, Params}, State) ->
   #{<<"cwd">> := Cwd} = Params,
   ok = file:set_cwd(Cwd),
   Name = filename:basename(Cwd),
-  ProjectNode =
-    case Params of
-      #{ <<"projectnode">> := Node } -> binary_to_atom(Node, utf8);
-      _ -> els_distribution_server:node_name(<<"erlang_ls_dap_project">>, Name)
-    end,
+
+  %% start distribution
+  LocalNode = els_distribution_server:node_name(<<"erlang_ls_dap">>, Name),
+  els_distribution_server:start_distribution(LocalNode),
+  ?LOG_INFO("Distribution up on: [~p]", [LocalNode]),
+
+  %% get default and final launch config
+  DefaultConfig = #{
+    <<"projectnode">> =>
+      atom_to_binary(
+        els_distribution_server:node_name(<<"erlang_ls_dap_project">>, Name),
+        utf8
+      ),
+    <<"cookie">> => atom_to_binary(erlang:get_cookie(), utf8),
+    <<"timeout">> => 30
+  },
+  #{ <<"projectnode">> := ConfProjectNode
+   , <<"cookie">>  := ConfCookie
+   , <<"timeout">> := TimeOut} = maps:merge(DefaultConfig, Params),
+  ProjectNode = binary_to_atom(ConfProjectNode, utf8),
+  Cookie = binary_to_atom(ConfCookie, utf8),
+
+  %% set cookie
+  true = erlang:set_cookie(LocalNode, Cookie),
+
+
   case Params of
     #{ <<"runinterminal">> := Cmd
      } ->
@@ -112,26 +135,21 @@ handle_request({<<"launch">>, Params}, State) ->
                   , "--sname"
                   , ProjectNode
                   , "--setcookie"
-                  , erlang:atom_to_list(erlang:get_cookie())
+                  , erlang:atom_to_list(Cookie)
                   ]
                 )
             end)
   end,
-  LocalNode = els_distribution_server:node_name(<<"erlang_ls_dap">>, Name),
-  els_distribution_server:start_distribution(LocalNode),
-  ?LOG_INFO("Distribution up on: [~p]", [LocalNode]),
 
   els_dap_server:send_event(<<"initialized">>, #{}),
 
-  {#{}, State#{project_node => ProjectNode, launch_params => Params}};
+  {#{}, State#{project_node => ProjectNode, launch_params => Params, timeout => TimeOut}};
 handle_request( {<<"configurationDone">>, _Params}
               , #{ project_node := ProjectNode
-                 , launch_params := LaunchParams} = State
+                 , launch_params := LaunchParams
+                 , timeout := Timeout} = State
               ) ->
-   els_distribution_server:wait_connect_and_monitor(ProjectNode),
-
-  inject_dap_agent(ProjectNode),
-
+  ensure_connected(ProjectNode, Timeout),
   %% TODO: Fetch stack_trace mode from Launch Config
   els_dap_rpc:stack_trace(ProjectNode, all),
   MFA = {els_dap_agent, int_cb, [self()]},
@@ -152,16 +170,15 @@ handle_request( {<<"configurationDone">>, _Params}
   {#{}, State};
 handle_request( {<<"setBreakpoints">>, Params}
               , #{ project_node := ProjectNode
-                 , breakpoints := Breakpoints0} = State
+                 , breakpoints := Breakpoints0
+                 , timeout := Timeout} = State
               ) ->
+  ensure_connected(ProjectNode, Timeout),
   #{<<"source">> := #{<<"path">> := Path}} = Params,
   SourceBreakpoints = maps:get(<<"breakpoints">>, Params, []),
   _SourceModified = maps:get(<<"sourceModified">>, Params, false),
   Module = els_uri:module(els_uri:uri(Path)),
 
-  els_distribution_server:wait_connect_and_monitor(ProjectNode),
-
-
   {module, Module} = els_dap_rpc:i(ProjectNode, Module),
   Lines = [Line || #{<<"line">> := Line} <- SourceBreakpoints],
 
@@ -181,11 +198,11 @@ handle_request({<<"setExceptionBreakpoints">>, _Params}, State) ->
   {#{}, State};
 handle_request({<<"setFunctionBreakpoints">>, Params}
               , #{ project_node := ProjectNode
-                 , breakpoints := Breakpoints0} = State
+                 , breakpoints := Breakpoints0
+                 , timeout := Timeout} = State
               ) ->
+  ensure_connected(ProjectNode, Timeout),
   FunctionBreakPoints = maps:get(<<"breakpoints">>, Params, []),
-  els_distribution_server:wait_connect_and_monitor(ProjectNode),
-
   MFAs = [
       begin
           Spec = {Mod, _, _} = parse_mfa(MFA),
@@ -373,11 +390,12 @@ handle_request({<<"variables">>, #{<<"variablesReference">> := Ref
   {Variables, MoreBindings} = build_variables(Type, Bindings),
   { #{<<"variables">> => Variables}
   , State#{ scope_bindings => maps:merge(RestBindings, MoreBindings)}};
-handle_request({<<"disconnect">>, _Params}, State) ->
+handle_request({<<"disconnect">>, _Params}, State = #{project_node := ProjectNode}) ->
+  els_dap_rpc:halt(ProjectNode),
   els_utils:halt(0),
   {#{}, State}.
 
--spec handle_info(any(), state()) -> state().
+-spec handle_info(any(), state()) -> state() | no_return().
 handle_info( {int_cb, ThreadPid}
            , #{ threads := Threads
               , project_node := ProjectNode
@@ -391,7 +409,12 @@ handle_info( {int_cb, ThreadPid}
   els_dap_server:send_event(<<"stopped">>, #{ <<"reason">> => <<"breakpoint">>
                                             , <<"threadId">> => ThreadId
                                             }),
-  State#{threads => maps:put(ThreadId, Thread, Threads)}.
+  State#{threads => maps:put(ThreadId, Thread, Threads)};
+handle_info({nodedown, Node}, State) ->
+  %% the project node is down, there is nothing left to do then to exit
+  ?LOG_NOTICE("project node ~p terminated, ending debug session", [Node]),
+  stop_debugger(),
+  State.
 
 %%==============================================================================
 %% API
@@ -692,3 +715,27 @@ do_function_breaks(Node, Module, FBreaks, Breaks) ->
     #{Module := ModBreaks} -> Breaks#{ Module => ModBreaks#{function => FBreaks}};
     _ -> Breaks#{ Module => #{line => [], function => FBreaks}}
   end.
+
+-spec ensure_connected(node(), timeout()) -> ok.
+ensure_connected(Node, Timeout) ->
+  case is_node_connected(Node) of
+    true -> ok;
+    false ->
+      % connect and monitore project node
+      case els_distribution_server:wait_connect_and_monitor(Node, Timeout) of
+        ok -> inject_dap_agent(Node);
+        _ -> stop_debugger()
+      end
+  end.
+
+-spec stop_debugger() -> no_return().
+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">>}),
+  ?LOG_NOTICE("terminating debug adapter"),
+  els_utils:halt(0).
+
+-spec is_node_connected(node()) -> boolean().
+is_node_connected(Node) ->
+  lists:member(Node, erlang:nodes(connected)).

apps/els_dap/src/els_dap_rpc.erl

@@ -9,6 +9,7 @@
         , continue/2
         , eval/3
         , get_meta/2
+        , halt/1
         , i/2
         , load_binary/4
         , meta/4
@@ -64,6 +65,10 @@ eval(Node, Input, Bindings) ->
 get_meta(Node, Pid) ->
   rpc:call(Node, dbg_iserver, safe_call, [{get_meta, Pid}]).
 
+-spec halt(node()) -> true.
+halt(Node) ->
+  rpc:cast(Node, erlang, halt, []).
+
 -spec i(node(), module()) -> any().
 i(Node, Module) ->
   rpc:call(Node, int, i, [Module]).

apps/els_dap/test/els_dap_general_provider_SUITE.erl

@@ -15,17 +15,16 @@
 -export([
     initialize/1,
     launch_mfa/1,
+    launch_mfa_with_cookie/1,
     configuration_done/1,
     configuration_done_with_breakpoint/1,
     frame_variables/1,
     navigation_and_frames/1,
     set_variable/1,
-    breakpoints/1
+    breakpoints/1,
+    project_node_exit/1
 ]).
 
-%% TODO: cleanup after dropping support for OTP 21 and 22
--compile({no_auto_import, [atom_to_binary/1, binary_to_atom/1]}).
-
 %%==============================================================================
 %% Includes
 %%==============================================================================
@@ -65,6 +64,7 @@ init_per_testcase(TestCase, Config) when
     TestCase =:= undefined orelse
         TestCase =:= initialize orelse
         TestCase =:= launch_mfa orelse
+        TestCase =:= launch_mfa_with_cookie orelse
         TestCase =:= configuration_done orelse
         TestCase =:= configuration_done_with_breakpoint
 ->
@@ -86,7 +86,7 @@ init_per_testcase(_TestCase, Config0) ->
 -spec end_per_testcase(atom(), config()) -> ok.
 end_per_testcase(_TestCase, Config) ->
     NodeName = ?config(node, Config),
-    Node = binary_to_atom(NodeName),
+    Node = binary_to_atom(NodeName, utf8),
     unset_all_env(els_core),
     ok = gen_server:stop(?config(provider, Config)),
     gen_server:stop(els_config),
@@ -123,7 +123,7 @@ path_to_test_module(AppDir, Module) ->
 
 -spec wait_for_break(binary(), module(), non_neg_integer()) -> boolean().
 wait_for_break(NodeName, WantModule, WantLine) ->
-    Node = binary_to_atom(NodeName),
+    Node = binary_to_atom(NodeName, utf8),
     Checker = fun() ->
         Snapshots = rpc:call(Node, int, snapshot, []),
         lists:any(
@@ -140,14 +140,6 @@ wait_for_break(NodeName, WantModule, WantLine) ->
     end,
     els_dap_test_utils:wait_for_fun(Checker, 200, 20).
 
--spec atom_to_binary(atom()) -> binary().
-atom_to_binary(Atom) ->
-    list_to_binary(atom_to_list(Atom)).
-
--spec binary_to_atom(binary()) -> atom().
-binary_to_atom(Binary) ->
-    list_to_atom(binary_to_list(Binary)).
-
 %%==============================================================================
 %% Testcases
 %%==============================================================================
@@ -171,6 +163,19 @@ launch_mfa(Config) ->
     els_dap_test_utils:wait_until_mock_called(els_dap_server, send_event),
     ok.
 
+-spec launch_mfa_with_cookie(config()) -> ok.
+launch_mfa_with_cookie(Config) ->
+    Provider = ?config(provider, Config),
+    DataDir = ?config(data_dir, Config),
+    Node = ?config(node, Config),
+    els_provider:handle_request(Provider, request_initialize(#{})),
+    els_provider:handle_request(
+        Provider,
+        request_launch(DataDir, Node, <<"some_cookie">>, els_dap_test_module, entry, [])
+    ),
+    els_dap_test_utils:wait_until_mock_called(els_dap_server, send_event),
+    ok.
+
 -spec configuration_done(config()) -> ok.
 configuration_done(Config) ->
     Provider = ?config(provider, Config),
@@ -231,7 +236,7 @@ frame_variables(Config) ->
         Provider,
         request_variable(VariableRef)
     ),
-    %% at this point there should be only one variable present
+    %% at this point there should be only one variable present,
     ?assertMatch(
         #{
             <<"name">> := <<"N">>,
@@ -357,7 +362,7 @@ set_variable(Config) ->
 breakpoints(Config) ->
     Provider = ?config(provider, Config),
     NodeName = ?config(node, Config),
-    Node = binary_to_atom(NodeName),
+    Node = binary_to_atom(NodeName, utf8),
     DataDir = ?config(data_dir, Config),
     els_provider:handle_request(
         Provider,
@@ -391,6 +396,23 @@ breakpoints(Config) ->
     ?assertMatch([{{els_dap_test_module, 9}, _}], els_dap_rpc:all_breaks(Node)),
     ok.
 
+-spec project_node_exit(config()) -> ok.
+project_node_exit(Config) ->
+    NodeName = ?config(node, Config),
+    Node = binary_to_atom(NodeName, utf8),
+    meck:expect(els_utils, halt, 1, meck:val(ok)),
+    meck:reset(els_dap_server),
+    erlang:monitor_node(Node, true),
+    %% kill node and wait for nodedown message
+    rpc:cast(Node, erlang, halt, []),
+    receive
+        {nodedown, Node} -> ok
+    end,
+    %% wait until els_utils:halt has been called
+    els_dap_test_utils:wait_until_mock_called(els_utils, halt),
+    ?assert(meck:called(els_dap_server, send_event, [<<"terminated">>, '_'])),
+    ?assert(meck:called(els_dap_server, send_event, [<<"exited">>, '_'])).
+
 %%==============================================================================
 %% Requests
 %%==============================================================================
@@ -405,11 +427,15 @@ request_launch(AppDir, Node, M, F, A) ->
     request_launch(#{
         <<"projectnode">> => Node,
         <<"cwd">> => AppDir,
-        <<"module">> => atom_to_binary(M),
-        <<"function">> => atom_to_binary(F),
+        <<"module">> => atom_to_binary(M, utf8),
+        <<"function">> => atom_to_binary(F, utf8),
         <<"args">> => unicode:characters_to_binary(io_lib:format("~w", [A]))
     }).
 
+request_launch(AppDir, Node, Cookie, M, F, A) ->
+    {<<"launch">>, Params} = request_launch(AppDir, Node, M, F, A),
+    {<<"launch">>, Params#{<<"cookie">> => Cookie}}.
+
 request_configuration_done(Params) ->
     {<<"configurationDone">>, Params}.