Document the REST API and improve error handling

apps/xprof_core/src/xprof_core.erl

@@ -136,12 +136,12 @@ get_trace_status() ->
 %% @doc Start capturing arguments and return values of function calls that
 %% lasted longer than the specified time threshold in ms.
 -spec capture(xprof_core:mfa_id(), non_neg_integer(), non_neg_integer()) ->
-                     {ok, CaptureId :: non_neg_integer()}.
+                     {ok, CaptureId :: non_neg_integer()} | {error, not_found}.
 capture(MFA, Threshold, Limit) ->
     xprof_core_trace_handler:capture(MFA, Threshold, Limit).
 
 %% @doc Stop capturing long calls of the given function.
--spec capture_stop(xprof_core:mfa_id()) -> ok | {error, not_found}.
+-spec capture_stop(xprof_core:mfa_id()) -> ok | {error, not_found | not_captured}.
 capture_stop(MFA) ->
     xprof_core_trace_handler:capture_stop(MFA).
 

apps/xprof_core/src/xprof_core_trace_handler.erl

@@ -55,15 +55,20 @@ data(MFA, FromEpoch) ->
 %% @doc Starts capturing args and results from function calls that lasted longer
 %% than specified time threshold.
 -spec capture(xprof_core:mfa_id(), non_neg_integer(), non_neg_integer()) ->
-                     {ok, non_neg_integer()}.
+                     {ok, non_neg_integer()} | {error, not_found}.
 capture(MFA = {M,F,A}, Threshold, Limit) ->
     lager:info("Capturing ~p calls to ~w:~w/~w that exceed ~p ms:",
                [Limit, M, F, A, Threshold]),
 
     Name = xprof_core_lib:mfa2atom(MFA),
-    gen_server:call(Name, {capture, Threshold, Limit}).
+    try
+        gen_server:call(Name, {capture, Threshold, Limit})
+    catch
+        exit:{noproc, _} ->
+            {error, not_found}
+    end.
 
--spec capture_stop(xprof_core:mfa_id()) -> ok | {error, not_found}.
+-spec capture_stop(xprof_core:mfa_id()) -> ok | {error, not_found | not_captured}.
 capture_stop(MFA) ->
     Name = xprof_core_lib:mfa2atom(MFA),
     try
@@ -75,27 +80,34 @@ capture_stop(MFA) ->
 
 %% @doc
 -spec get_captured_data(xprof_core:mfa_id(), non_neg_integer()) ->
-                               empty | {ok,
-                                        {Index :: non_neg_integer(),
-                                         Threshold :: non_neg_integer(),
-                                         OrigLimit :: non_neg_integer(),
-                                         HasMore :: boolean()
-                                        }, [tuple()]}.
+                               {ok,
+                                {Index :: non_neg_integer(),
+                                 Threshold :: non_neg_integer(),
+                                 OrigLimit :: non_neg_integer(),
+                                 HasMore :: boolean()
+                                }, [tuple()]} |
+                               {error, not_found}.
 get_captured_data(MFA, Offset) when Offset >= 0 ->
     Name = xprof_core_lib:mfa2atom(MFA),
     try
-        Items = lists:sort(ets:select(Name,
-                                      [{
-                                         {{args_res, '$1'},
-                                          {'$2', '$3','$4','$5'}},
-                                         [{'>','$1',Offset}],
-                                         [{{'$1', '$2', '$3', '$4', '$5'}}]
-                                       }])),
-
-        Res = ets:lookup(Name, capture_spec),
-        [{capture_spec, Index, Threshold, Limit, OrigLimit}] = Res,
-        HasMore = Offset + length(Items) < Limit,
-        {ok, {Index, Threshold, OrigLimit, HasMore}, Items}
+        [{capture_spec, Index, Threshold, Limit, OrigLimit}] =
+            ets:lookup(Name, capture_spec),
+        case Index of
+            -1 ->
+                %% no capturing was done for this MFA yet,
+                %% no need to traverse the table
+                {ok, {Index, Threshold, OrigLimit, _HasMore = false}, _Items = []};
+            _ ->
+                MS = [{
+                        {{args_res, '$1'},
+                         {'$2', '$3','$4','$5'}},
+                        [{'>','$1',Offset}],
+                        [{{'$1', '$2', '$3', '$4', '$5'}}]
+                      }],
+                Items = lists:sort(ets:select(Name, MS)),
+                HasMore = Offset + length(Items) < Limit,
+                {ok, {Index, Threshold, OrigLimit, HasMore}, Items}
+        end
     catch error:badarg ->
             {error, not_found}
     end.
@@ -127,6 +139,9 @@ handle_call({capture, Threshold, Limit}, _From,
     capture_args_trace_on(MFA),
     {Timeout, NewState2} = maybe_make_snapshot(NewState),
     {reply, {ok, NewId}, NewState2, Timeout};
+handle_call(capture_stop, _From, State = #state{capture_spec = undefined}) ->
+    {Timeout, NewState} = maybe_make_snapshot(State),
+    {reply, {error, not_captured}, NewState, Timeout};
 handle_call(capture_stop, _From, State = #state{mfa = MFA}) ->
     capture_args_trace_off(MFA),
     #state{capture_spec = {Threshold, Limit},

apps/xprof_core/test/xprof_tracing_SUITE.erl

@@ -13,7 +13,10 @@
         ]).
 
 %% Test cases
--export([monitor_many_funs/1,
+-export([not_found_error/1,
+         already_traced_error/1,
+         not_captured_error/1,
+         monitor_many_funs/1,
          monitor_recursive_fun/1,
          monitor_keep_recursive_fun/1,
          monitor_crashing_fun/1,
@@ -32,7 +35,10 @@
 %% CT funs
 
 all() ->
-    [monitor_many_funs,
+    [not_found_error,
+     already_traced_error,
+     not_captured_error,
+     monitor_many_funs,
      monitor_recursive_fun,
      monitor_keep_recursive_fun,
      monitor_crashing_fun,
@@ -133,6 +139,35 @@ dead_proc_tracing(_Config) ->
                  xprof_core:get_trace_status()),
     ok.
 
+not_found_error(_Config) ->
+    MFA = {?MODULE, no_such_fun, 1},
+    ?assertEqual(ok, xprof_core:demonitor(MFA)),
+    ?assertEqual({error, not_found}, xprof_core:get_data(MFA, 0)),
+    ?assertEqual({error, not_found}, xprof_core:capture(MFA, 1, 1)),
+    ?assertEqual({error, not_found}, xprof_core:capture_stop(MFA)),
+    ?assertEqual({error, not_found}, xprof_core:get_captured_data(MFA, 0)),
+    ok.
+
+already_traced_error(_Config) ->
+    MFA = {?MODULE, test_fun, 1},
+    ok = xprof_core:monitor(MFA),
+    try
+        ?assertEqual({error, already_traced}, xprof_core:monitor(MFA))
+    after
+        xprof_core:demonitor(MFA)
+    end.
+
+not_captured_error(_Config) ->
+    MFA = {?MODULE, test_fun, 1},
+    ok = xprof_core:monitor(MFA),
+    try
+        ?assertEqual({error, not_captured}, xprof_core:capture_stop(MFA)),
+        ?assertEqual({ok, {-1,-1,-1,false}, []},
+                     xprof_core:get_captured_data(MFA, 0))
+    after
+        xprof_core:demonitor(MFA)
+    end.
+
 monitor_crashing_fun(_Config) ->
     xprof_core:monitor(MFA = {?MODULE, maybe_crash_test_fun, 1}),
     ok = xprof_core:trace(self()),

apps/xprof_gui/src/xprof_gui_app.erl

@@ -10,6 +10,7 @@
 
 -define(APP, xprof_gui).
 -define(DEF_WEB_IF_PORT, 7890).
+-define(LISTENER, xprof_http_listener).
 
 %% Application callbacks
 
@@ -29,17 +30,18 @@ stop(_State) ->
 
 start_cowboy() ->
     Port = application:get_env(?APP, port, ?DEF_WEB_IF_PORT),
-    Dispatch = cowboy_router:compile(cowboy_routes()),
-    cowboy:start_http(xprof_http_listener, 100, [{port, Port}],
-                      [{env, [{dispatch, Dispatch}]}]).
-
-cowboy_routes() ->
-    [{'_', [{"/api/:what", xprof_gui_cowboy1_handler, []},
-            {"/build/[...]", cowboy_static, {priv_dir, ?APP, "build"}},
-            {"/styles/[...]", cowboy_static, {priv_dir, ?APP, "styles"}},
-            {"/img/[...]", cowboy_static, {priv_dir, ?APP, "img"}},
-            {"/", cowboy_static, {priv_file, ?APP, "index.html"}}
-           ]}].
+    Dispatch = cowboy_dispatch(xprof_gui_cowboy1_handler),
+    xprof_gui_cowboy1_handler:start_listener(?LISTENER, Port, Dispatch).
+
+cowboy_dispatch(Mod) ->
+    Routes =
+        [{'_', [{"/api/:what", Mod, []},
+                {"/build/[...]", cowboy_static, {priv_dir, ?APP, "build"}},
+                {"/styles/[...]", cowboy_static, {priv_dir, ?APP, "styles"}},
+                {"/img/[...]", cowboy_static, {priv_dir, ?APP, "img"}},
+                {"/", cowboy_static, {priv_file, ?APP, "index.html"}}
+               ]}],
+    cowboy_router:compile(Routes).
 
 stop_cowboy() ->
-    cowboy:stop_listener(xprof_http_listener).
+    cowboy:stop_listener(?LISTENER).