Browse Source

Add a better error message when user callbacks are not exported

Loïc Hoguin 12 years ago
parent
commit
5f6b6b6f6d
3 changed files with 65 additions and 5 deletions
  1. 24 5
      src/cowboy_rest.erl
  2. 20 0
      test/http_SUITE.erl
  3. 21 0
      test/rest_missing_callbacks.erl

+ 24 - 5
src/cowboy_rest.erl

@@ -726,9 +726,19 @@ put_resource(Req, State, OnTrue) ->
 %% list of content types, otherwise it'll shadow the ones following.
 choose_content_type(Req, State, _OnTrue, _ContentType, []) ->
 	respond(Req, State, 415);
-choose_content_type(Req, State, OnTrue, ContentType, [{Accepted, Fun}|_Tail])
+choose_content_type(Req,
+		State=#state{handler=Handler, handler_state=HandlerState},
+		OnTrue, ContentType, [{Accepted, Fun}|_Tail])
 		when Accepted =:= '*' orelse Accepted =:= ContentType ->
 	case call(Req, State, Fun) of
+		no_call ->
+			error_logger:error_msg(
+				"** Cowboy handler ~p terminating; "
+				"function ~p was not exported~n"
+				"** Request was ~p~n** State was ~p~n~n",
+				[Handler, Fun, cowboy_req:to_list(Req), HandlerState]),
+			{ok, _} = cowboy_req:reply(500, Req),
+			close;
 		{halt, Req2, HandlerState} ->
 			terminate(Req2, State#state{handler_state=HandlerState});
 		{true, Req2, HandlerState} ->
@@ -759,19 +769,28 @@ has_resp_body(Req, State) ->
 %% Set the response headers and call the callback found using
 %% content_types_provided/2 to obtain the request body and add
 %% it to the response.
-set_resp_body(Req, State=#state{content_type_a={_Type, Fun}}) ->
+set_resp_body(Req, State=#state{handler=Handler, handler_state=HandlerState,
+		content_type_a={_Type, Fun}}) ->
 	{Req2, State2} = set_resp_etag(Req, State),
 	{LastModified, Req3, State3} = last_modified(Req2, State2),
-	case LastModified of
+	Req4 = case LastModified of
 		LastModified when is_atom(LastModified) ->
-			Req4 = Req3;
+			Req3;
 		LastModified ->
 			LastModifiedStr = httpd_util:rfc1123_date(LastModified),
-			Req4 = cowboy_req:set_resp_header(
+			cowboy_req:set_resp_header(
 				<<"last-modified">>, LastModifiedStr, Req3)
 	end,
 	{Req5, State4} = set_resp_expires(Req4, State3),
 	case call(Req5, State4, Fun) of
+		no_call ->
+			error_logger:error_msg(
+				"** Cowboy handler ~p terminating; "
+				"function ~p was not exported~n"
+				"** Request was ~p~n** State was ~p~n~n",
+				[Handler, Fun, cowboy_req:to_list(Req5), HandlerState]),
+			{ok, _} = cowboy_req:reply(500, Req5),
+			close;
 		{halt, Req6, HandlerState} ->
 			terminate(Req6, State4#state{handler_state=HandlerState});
 		{Body, Req6, HandlerState} ->

+ 20 - 0
test/http_SUITE.erl

@@ -50,6 +50,8 @@
 -export([pipeline/1]).
 -export([rest_keepalive/1]).
 -export([rest_keepalive_post/1]).
+-export([rest_missing_get_callbacks/1]).
+-export([rest_missing_put_callbacks/1]).
 -export([rest_nodelete/1]).
 -export([rest_resource_etags/1]).
 -export([rest_resource_etags_if_none_match/1]).
@@ -95,6 +97,8 @@ groups() ->
 		pipeline,
 		rest_keepalive,
 		rest_keepalive_post,
+		rest_missing_get_callbacks,
+		rest_missing_put_callbacks,
 		rest_nodelete,
 		rest_resource_etags,
 		rest_resource_etags_if_none_match,
@@ -250,6 +254,8 @@ init_dispatch(Config) ->
 			{[<<"simple">>], rest_simple_resource, []},
 			{[<<"forbidden_post">>], rest_forbidden_resource, [true]},
 			{[<<"simple_post">>], rest_forbidden_resource, [false]},
+			{[<<"missing_get_callbacks">>], rest_missing_callbacks, []},
+			{[<<"missing_put_callbacks">>], rest_missing_callbacks, []},
 			{[<<"nodelete">>], rest_nodelete_resource, []},
 			{[<<"resetags">>], rest_resource_etags, []},
 			{[<<"loop_timeout">>], http_handler_loop_timeout, []},
@@ -691,6 +697,20 @@ rest_keepalive_post_loop(Config, Client, forbidden_post, N) ->
 		= lists:keyfind(<<"connection">>, 1, RespHeaders),
 	rest_keepalive_post_loop(Config, Client3, simple_post, N - 1).
 
+rest_missing_get_callbacks(Config) ->
+	Client = ?config(client, Config),
+	{ok, Client2} = cowboy_client:request(<<"GET">>,
+		build_url("/missing_get_callbacks", Config), Client),
+	{ok, 500, _, _} = cowboy_client:response(Client2).
+
+rest_missing_put_callbacks(Config) ->
+	Client = ?config(client, Config),
+	{ok, Client2} = cowboy_client:request(<<"PUT">>,
+		build_url("/missing_put_callbacks", Config),
+		[{<<"content-type">>, <<"application/json">>}],
+		<<"{}">>, Client),
+	{ok, 500, _, _} = cowboy_client:response(Client2).
+
 rest_nodelete(Config) ->
 	Client = ?config(client, Config),
 	{ok, Client2} = cowboy_client:request(<<"DELETE">>,

+ 21 - 0
test/rest_missing_callbacks.erl

@@ -0,0 +1,21 @@
+-module(rest_missing_callbacks).
+-export([init/3]).
+-export([allowed_methods/2]).
+-export([content_types_accepted/2]).
+-export([content_types_provided/2]).
+
+init(_Transport, _Req, _Opts) ->
+	{upgrade, protocol, cowboy_rest}.
+
+allowed_methods(Req, State) ->
+	{[<<"GET">>, <<"PUT">>], Req, State}.
+
+content_types_accepted(Req, State) ->
+	{[
+		{<<"application/json">>, put_application_json}
+	], Req, State}.
+
+content_types_provided(Req, State) ->
+	{[
+		{<<"text/plain">>, get_text_plain}
+	], Req, State}.