Skip to content

Commit ceb7bd9

Browse files
jcpetruzzafacebook-github-bot
authored andcommitted
Make state type more precise
Summary: # Context The DAP protocol goes through several faces: first one waits for an initialization request; then for a launch/attach request; etc. On each state, we have more information than on the previous one. The state isrepresented by the `edb_dap_server:state()` type # Problem The current representation loses information: we don't know which fields are mandatory in which state, etc. This makes eqwalizer less helpful and can lead to typing issues. It is also arguably harder to understand from looking at the state how it is supposed to work # This diff We improve the representation of the state type so that it looks closer like a state-machine, and each state has exectaly the information that is known at that point. Doing this actually simplifies some of the handlers, Reviewed By: TheGeorge Differential Revision: D70342434 fbshipit-source-id: ee4899307979d4a154351c970cf29b8d7ec744d2
1 parent b6af267 commit ceb7bd9

18 files changed

+122
-60
lines changed

edb/include/edb_dap.hrl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,5 +38,5 @@
3838
%%%---------------------------------------------------------------------------------
3939
%%% Reserved for implementation-defined server-errors.
4040
%%%---------------------------------------------------------------------------------
41-
-define(ERROR_SERVER_NOT_INITIALIZED, -32002).
41+
-define(ERROR_PRECONDITION_VIOLATION, -32002).
4242
-define(ERROR_NOT_SUPPORTED, -32003).

edb/src/edb_dap_internal_events.erl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ nodedown_impl(State, Node, Reason) ->
6565
_ -> 1
6666
end,
6767
#{
68+
state => #{state => terminating},
6869
actions => [
6970
{event, edb_dap_event:exited(ExitCode)},
7071
{event, edb_dap_event:terminated()}

edb/src/edb_dap_request.erl

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@
2929
%% Public API
3030
-export([dispatch/2]).
3131

32+
%% Helpers for behaviour implementations
33+
-export([unexpected_request/0]).
34+
35+
-include("edb_dap.hrl").
36+
3237
%% ------------------------------------------------------------------
3338
%% Types
3439
%% ------------------------------------------------------------------
@@ -104,3 +109,10 @@ known_handlers() ->
104109
~"threads" => edb_dap_request_threads,
105110
~"variables" => edb_dap_request_variables
106111
}.
112+
113+
%% ------------------------------------------------------------------
114+
%% Helpers for behaviour implementations
115+
%% ------------------------------------------------------------------
116+
-spec unexpected_request() -> reaction().
117+
unexpected_request() ->
118+
#{error => {user_error, ?ERROR_PRECONDITION_VIOLATION, ~"Request sent when it was not expected"}}.

edb/src/edb_dap_request_continue.erl

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@ parse_arguments(Args) ->
5858
-spec handle(State, Args) -> edb_dap_request:reaction(response_body()) when
5959
State :: edb_dap_server:state(),
6060
Args :: arguments().
61-
handle(_State, _Args) ->
61+
handle(#{state := attached}, _Args) ->
6262
edb_dap_id_mappings:reset(),
6363
{ok, _} = edb:continue(),
64-
#{response => #{success => true, body => #{allThreadsContinued => true}}}.
64+
#{response => #{success => true, body => #{allThreadsContinued => true}}};
65+
handle(_UnexpectedState, _) ->
66+
edb_dap_request:unexpected_request().

edb/src/edb_dap_request_initialize.erl

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@
2424

2525
-export([parse_arguments/1, handle/2]).
2626

27-
-include("edb_dap.hrl").
28-
2927
%% ------------------------------------------------------------------
3028
%% Types
3129
%% ------------------------------------------------------------------
@@ -155,14 +153,14 @@ parse_arguments(Args) ->
155153
-spec handle(State, Args) -> edb_dap_request:reaction(capabilities()) when
156154
State :: edb_dap_server:state(),
157155
Args :: arguments().
158-
handle(State = #{state := started}, _Args) ->
156+
handle(#{state := started}, ClientInfo) ->
159157
Capabilities = capabilities(),
160158
#{
161159
response => #{success => true, body => Capabilities},
162-
state => State#{state => initialized}
160+
state => #{state => initialized, client_info => ClientInfo}
163161
};
164-
handle(_InitializedState, _Args) ->
165-
#{error => {user_error, ?JSON_RPC_ERROR_INVALID_REQUEST, ~"Already initialized"}}.
162+
handle(_InvalidState, _Args) ->
163+
edb_dap_request:unexpected_request().
166164

167165
%% ------------------------------------------------------------------
168166
%% Helpers

edb/src/edb_dap_request_launch.erl

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ parse_arguments(Args) ->
7979
-spec handle(State, Args) -> edb_dap_request:reaction() when
8080
State :: edb_dap_server:state(),
8181
Args :: arguments().
82-
handle(State0, Args) ->
82+
handle(State0 = #{state := initialized}, Args) ->
8383
#{launchCommand := LaunchCommand, targetNode := TargetNode} = Args,
8484
#{cwd := Cwd, command := Command} = LaunchCommand,
8585
AttachTimeoutInSecs = maps:get(timeout, Args, ?DEFAULT_ATTACH_TIMEOUT_IN_SECS),
@@ -94,6 +94,7 @@ handle(State0, Args) ->
9494
env => Env#{~"ERL_FLAGS" => ?ERL_FLAGS}
9595
}),
9696
State1 = State0#{
97+
state => launching,
9798
context => #{
9899
target_node => TargetNode,
99100
attach_timeout => AttachTimeoutInSecs,
@@ -106,7 +107,9 @@ handle(State0, Args) ->
106107
response => #{success => true},
107108
actions => [{reverse_request, RunInTerminalRequest}],
108109
state => State1
109-
}.
110+
};
111+
handle(_InvalidState, _Args) ->
112+
edb_dap_request:unexpected_request().
110113

111114
%% ------------------------------------------------------------------
112115
%% Helpers

edb/src/edb_dap_request_next.erl

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424

2525
-export([parse_arguments/1, handle/2]).
2626

27-
-export([stepper/2]).
27+
-export([stepper/3]).
2828

2929
-include_lib("kernel/include/logger.hrl").
3030
-include("edb_dap.hrl").
@@ -59,16 +59,17 @@ parse_arguments(Args) ->
5959
-spec handle(State, Args) -> edb_dap_request:reaction() when
6060
State :: edb_dap_server:state(),
6161
Args :: arguments().
62-
handle(_State, #{threadId := ThreadId}) ->
63-
stepper(ThreadId, 'step-over').
62+
handle(State, #{threadId := ThreadId}) ->
63+
stepper(State, ThreadId, 'step-over').
6464

6565
%% ------------------------------------------------------------------
6666
%% Generic stepping implementation
6767
%% ------------------------------------------------------------------
68-
-spec stepper(ThreadId, StepType) -> edb_dap_request:reaction() when
68+
-spec stepper(State, ThreadId, StepType) -> edb_dap_request:reaction() when
69+
State :: edb_dap_server:state(),
6970
ThreadId :: edb_dap:thread_id(),
7071
StepType :: 'step-over' | 'step-out'.
71-
stepper(ThreadId, StepType) ->
72+
stepper(#{state := attached}, ThreadId, StepType) ->
7273
StepFun =
7374
case StepType of
7475
'step-over' -> fun edb:step_over/1;
@@ -90,4 +91,6 @@ stepper(ThreadId, StepType) ->
9091
{error, not_found} ->
9192
?LOG_WARNING("Cannot find pid for thread id ~p", [ThreadId]),
9293
throw(~"Unknown threadId")
93-
end.
94+
end;
95+
stepper(_UnexpectedState, _, _) ->
96+
edb_dap_request:unexpected_request().

edb/src/edb_dap_request_pause.erl

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ parse_arguments(Args) ->
4646
-spec handle(State, Args) -> edb_dap_request:reaction() when
4747
State :: edb_dap_server:state(),
4848
Args :: arguments().
49-
handle(_State, _Args) ->
49+
handle(#{state := attached}, _Args) ->
5050
ok = edb:pause(),
51-
#{response => #{success => true}}.
51+
#{response => #{success => true}};
52+
handle(_UnexpectedState, _) ->
53+
edb_dap_request:unexpected_request().

edb/src/edb_dap_request_scopes.erl

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ parse_arguments(Args) ->
100100
-spec handle(State, Args) -> edb_dap_request:reaction(response_body()) when
101101
State :: edb_dap_server:state(),
102102
Args :: arguments().
103-
handle(State, #{frameId := FrameId}) ->
103+
handle(#{state := attached, context := Context}, #{frameId := FrameId}) ->
104104
{ok, #{pid := Pid, frame_no := FrameNo}} = edb_dap_id_mappings:frame_id_to_pid_frame(FrameId),
105105
VariablesScopes =
106106
case edb:stack_frame_vars(Pid, FrameNo, _MaxTermSize = 1) of
@@ -129,7 +129,7 @@ handle(State, #{frameId := FrameId}) ->
129129
_ ->
130130
[]
131131
end,
132-
#{context := #{target_node := #{name := Node}}} = State,
132+
#{target_node := #{name := Node}} = Context,
133133
MessagesScopes =
134134
% elp:ignore W0014 (cross_node_eval)
135135
case erpc:call(Node, erlang, process_info, [Pid, message_queue_len]) of
@@ -153,4 +153,6 @@ handle(State, #{frameId := FrameId}) ->
153153
scopes => MessagesScopes ++ VariablesScopes
154154
}
155155
}
156-
}.
156+
};
157+
handle(_UnexpectedState, _) ->
158+
edb_dap_request:unexpected_request().

edb/src/edb_dap_request_set_breakpoints.erl

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,11 +149,11 @@ parse_arguments(Args) ->
149149
-spec handle(State, Args) -> edb_dap_request:reaction(breakpoints()) when
150150
State :: edb_dap_server:state(),
151151
Args :: arguments().
152-
handle(State, Args = #{source := #{path := Path}}) ->
152+
handle(#{state := attached, context := Context}, Args = #{source := #{path := Path}}) ->
153153
Module = binary_to_atom(filename:basename(Path, ".erl")),
154154

155155
% TODO(T202772655): Remove once edb:set_breakpoint/2 takes care of auto-loading modules
156-
#{context := #{target_node := #{name := Node}}} = State,
156+
#{target_node := #{name := Node}} = Context,
157157
% elp:ignore W0014 (cross_node_eval)
158158
erpc:call(Node, code, ensure_loaded, [Module]),
159159

@@ -176,4 +176,6 @@ handle(State, Args = #{source := #{path := Path}}) ->
176176
LineResults
177177
),
178178
Body = #{breakpoints => Breakpoints},
179-
#{response => #{success => true, body => Body}}.
179+
#{response => #{success => true, body => Body}};
180+
handle(_UnexpectedState, _) ->
181+
edb_dap_request:unexpected_request().

0 commit comments

Comments
 (0)