From b1320d3f4e59cd2575a334fdcd2b2ce2f4232e34 Mon Sep 17 00:00:00 2001 From: Marc Worrell Date: Thu, 5 Jan 2012 17:50:32 +0100 Subject: [PATCH] Catch exceptions, better error handling. Added 400 support. --- src/webmachine_decision_core.erl | 249 ++++++++++++++++--------------- src/webmachine_error_handler.erl | 4 + src/webmachine_mochiweb.erl | 4 +- 3 files changed, 138 insertions(+), 119 deletions(-) diff --git a/src/webmachine_decision_core.erl b/src/webmachine_decision_core.erl index 9526de5..e8fd7c1 100644 --- a/src/webmachine_decision_core.erl +++ b/src/webmachine_decision_core.erl @@ -30,26 +30,25 @@ handle_request(Resource, ReqData) -> try d(v3b13, Resource, ReqData) catch - error:X -> - ?WM_DBG(X), - error_response(erlang:get_stacktrace(), Resource, ReqData) + error:Error -> + error_response({Error, erlang:get_stacktrace()}, Resource, ReqData) end. %% @doc Call the resource or a default. %% @spec resource_call(atom(), Resource, ReqData) -> {term(), NewResource, NewReqData} resource_call(Fun, Rs, Rd) -> - case cacheable(Fun) of - true -> - case proplists:lookup(Fun, Rd#wm_reqdata.cache) of - none -> - {T, Rs1, Rd1} = Rs:do(Fun, Rd), - {T, Rs1, Rd1#wm_reqdata{cache=[{Fun,T}|Rd1#wm_reqdata.cache]}}; - {Fun, Cached} -> - {Cached, Rs, Rd} - end; - false -> - Rs:do(Fun, Rd) - end. + case cacheable(Fun) of + true -> + case proplists:lookup(Fun, Rd#wm_reqdata.cache) of + none -> + {T, Rs1, Rd1} = Rs:do(Fun, Rd), + {T, Rs1, Rd1#wm_reqdata{cache=[{Fun,T}|Rd1#wm_reqdata.cache]}}; + {Fun, Cached} -> + {Cached, Rs, Rd} + end; + false -> + Rs:do(Fun, Rd) + end. cacheable(charsets_provided) -> true; cacheable(content_types_provided) -> true; @@ -99,24 +98,40 @@ respond(Code, Headers, Rs, Rd) -> RdHs = wrq:set_resp_headers(Headers, Rd), respond(Code, Rs, RdHs). + +error_response(Code, _Reason, Rs, Rd) + when Code == 403; Code == 404; Code == 304 -> + respond(403, Rs, Rd); error_response(Code, Reason, Rs, Rd) -> {ok, ErrorHandler} = application:get_env(webzmachine, error_handler), {ErrorHTML, Rd1} = ErrorHandler:render_error(Code, Rd, Reason), Rd2 = wrq:set_resp_body(ErrorHTML, Rd1), - respond(Code, Rs, Rd2). + RdResp = wrq:set_response_code(Code, Rd2), + resource_call(finish_request, Rs, RdResp). + +error_response({error, Reason}, Rs, Rd) -> + error_response(reason_to_code(Reason), Reason, Rs, Rd); +error_response({error, Reason0, Reason1}, Rs, Rd) -> + error_response(reason_to_code(Reason0), {Reason0, Reason1}, Rs, Rd); error_response(Reason, Rs, Rd) -> - error_response(500, Reason, Rs, Rd). + error_response(reason_to_code(Reason), Reason, Rs, Rd). + + reason_to_code({throw, Error, _Trace}) -> reason_to_code(Error); + reason_to_code({bad_request, _}) -> 400; + reason_to_code(timeout) -> 503; + reason_to_code(_) -> 500. + decision_test({Test, Rs, Rd}, TestVal, TrueFlow, FalseFlow) -> decision_test(Test, TestVal, TrueFlow, FalseFlow, Rs, Rd). decision_test(Test,TestVal,TrueFlow,FalseFlow, Rs, Rd) -> case Test of - {error, Reason} -> error_response(Reason, Rs, Rd); - {error, Reason0, Reason1} -> error_response({Reason0, Reason1}, Rs, Rd); - {halt, Code} -> respond(Code, Rs, Rd); - TestVal -> decision_flow(TrueFlow, Test, Rs, Rd); - _ -> decision_flow(FalseFlow, Test, Rs, Rd) + {error, Reason} -> error_response(Reason, Rs, Rd); + {error, Reason0, Reason1} -> error_response({Reason0, Reason1}, Rs, Rd); + {halt, Code} -> respond(Code, Rs, Rd); + TestVal -> decision_flow(TrueFlow, Test, Rs, Rd); + _ -> decision_flow(FalseFlow, Test, Rs, Rd) end. decision_test_fn({Test, Rs, Rd}, TestFn, TrueFlow, FalseFlow) -> @@ -164,7 +179,7 @@ do_log(LogData) -> %% "Service Available" decision(v3b13, Rs, Rd) -> decision_test(resource_call(ping, Rs, Rd), pong, v3b13b, 503); -decision(v3b13b, Rs, Rd) -> +decision(v3b13b, Rs, Rd) -> decision_test(resource_call(service_available, Rs, Rd), true, v3b12, 503); %% "Known method?" decision(v3b12, Rs, Rd) -> @@ -177,11 +192,11 @@ decision(v3b11, Rs, Rd) -> decision(v3b10, Rs, Rd) -> {Methods, Rs1, Rd1} = resource_call(allowed_methods, Rs, Rd), case lists:member(method(Rd1), Methods) of - true -> - d(v3b9, Rs1, Rd1); - false -> - RdAllow = wrq:set_resp_header("Allow", string:join([atom_to_list(M) || M <- Methods], ", "), Rd1), - respond(405, Rs1, RdAllow) + true -> + d(v3b9, Rs1, Rd1); + false -> + RdAllow = wrq:set_resp_header("Allow", string:join([atom_to_list(M) || M <- Methods], ", "), Rd1), + respond(405, Rs1, RdAllow) end; %% "Malformed?" decision(v3b9, Rs, Rd) -> @@ -190,12 +205,12 @@ decision(v3b9, Rs, Rd) -> decision(v3b8, Rs, Rd) -> {IsAuthorized, Rs1, Rd1} = resource_call(is_authorized, Rs, Rd), case IsAuthorized of - true -> - d(v3b7, Rs1, Rd1); - {error, Reason} -> - error_response(Reason, Rs1, Rd1); - {halt, Code} -> - respond(Code, Rs1, Rd1); + true -> + d(v3b7, Rs1, Rd1); + {error, Reason} -> + error_response(Reason, Rs1, Rd1); + {halt, Code} -> + respond(Code, Rs1, Rd1); AuthHead -> RdAuth = wrq:set_resp_header("WWW-Authenticate", AuthHead, Rd1), respond(401, Rs1, RdAuth) @@ -206,28 +221,28 @@ decision(v3b7, Rs, Rd) -> %% "Upgrade?" decision(v3b6_upgrade, Rs, Rd) -> case get_header_val("upgrade", Rd) of - undefined -> - decision(v3b6, Rs, Rd); - UpgradeHdr -> - case get_header_val("connection", Rd) of - undefined -> - decision(v3b6, Rs, Rd); - Connection -> - case string:strip(string:to_lower(Connection)) of - "upgrade" -> - {Choosen, Rs1, Rd1} = choose_upgrade(UpgradeHdr, Rs, Rd), - case Choosen of - none -> - decision(v3b6, Rs1, Rd1); - {_Protocol, UpgradeFunc} -> - %% TODO: log the upgrade action - {upgrade, UpgradeFunc, Rs1, Rd1} - end; - _ -> - decision(v3b6, Rs, Rd) - end - end - end; + undefined -> + decision(v3b6, Rs, Rd); + UpgradeHdr -> + case get_header_val("connection", Rd) of + undefined -> + decision(v3b6, Rs, Rd); + Connection -> + case string:strip(string:to_lower(Connection)) of + "upgrade" -> + {Choosen, Rs1, Rd1} = choose_upgrade(UpgradeHdr, Rs, Rd), + case Choosen of + none -> + decision(v3b6, Rs1, Rd1); + {_Protocol, UpgradeFunc} -> + %% TODO: log the upgrade action + {upgrade, UpgradeFunc, Rs1, Rd1} + end; + _ -> + decision(v3b6, Rs, Rd) + end + end + end; %% "Okay Content-* Headers?" decision(v3b6, Rs, Rd) -> decision_test(resource_call(valid_content_headers, Rs, Rd), true, v3b5, 501); @@ -240,22 +255,22 @@ decision(v3b4, Rs, Rd) -> %% "OPTIONS?" decision(v3b3, Rs, Rd) -> case wrq:method(Rd) of - 'OPTIONS' -> - {Hdrs, Rs1, Rd1} = resource_call(options, Rs, Rd), - respond(200, Hdrs, Rs1, Rd1); - _ -> - d(v3c3, Rs, Rd) + 'OPTIONS' -> + {Hdrs, Rs1, Rd1} = resource_call(options, Rs, Rd), + respond(200, Hdrs, Rs1, Rd1); + _ -> + d(v3c3, Rs, Rd) end; %% Accept exists? decision(v3c3, Rs, Rd) -> {ContentTypes, Rs1, Rd1} = resource_call(content_types_provided, Rs, Rd), PTypes = [Type || {Type,_Fun} <- ContentTypes], case get_header_val("accept", Rd1) of - undefined -> - {ok, RdCT} = webmachine_request:set_metadata('content-type', hd(PTypes), Rd1), - d(v3d4, Rs1, RdCT); - _ -> - d(v3c4, Rs1, Rd1) + undefined -> + {ok, RdCT} = webmachine_request:set_metadata('content-type', hd(PTypes), Rd1), + d(v3d4, Rs1, RdCT); + _ -> + d(v3c4, Rs1, Rd1) end; %% Acceptable media type available? decision(v3c4, Rs, Rd) -> @@ -263,11 +278,11 @@ decision(v3c4, Rs, Rd) -> PTypes = [Type || {Type,_Fun} <- ContentTypesProvided], AcceptHdr = get_header_val("accept", Rd1), case webmachine_util:choose_media_type(PTypes, AcceptHdr) of - none -> - respond(406, Rs1, Rd1); - MType -> - {ok, RdCT} = webmachine_request:set_metadata('content-type', MType, Rd1), - d(v3d4, Rs, RdCT) + none -> + respond(406, Rs1, Rd1); + MType -> + {ok, RdCT} = webmachine_request:set_metadata('content-type', MType, Rd1), + d(v3d4, Rs, RdCT) end; %% Accept-Language exists? decision(v3d4, Rs, Rd) -> @@ -341,15 +356,15 @@ decision(v3h12, Rs, Rd) -> decision(v3i4, Rs, Rd) -> {MovedPermanently, Rs1, Rd1} = resource_call(moved_permanently, Rs, Rd), case MovedPermanently of - {true, MovedURI} -> - RdLoc = wrq:set_resp_header("Location", MovedURI, Rd1), - respond(301, Rs1, RdLoc); - false -> - d(v3p3, Rs1, Rd1); - {error, Reason} -> - error_response(Reason, Rs1, Rd1); - {halt, Code} -> - respond(Code, Rs1, Rd1) + {true, MovedURI} -> + RdLoc = wrq:set_resp_header("Location", MovedURI, Rd1), + respond(301, Rs1, RdLoc); + false -> + d(v3p3, Rs1, Rd1); + {error, Reason} -> + error_response(Reason, Rs1, Rd1); + {halt, Code} -> + respond(Code, Rs1, Rd1) end; %% PUT? decision(v3i7, Rs, Rd) -> @@ -367,15 +382,15 @@ decision(v3j18, Rs, Rd) -> decision(v3k5, Rs, Rd) -> {MovedPermanently, Rs1, Rd1} = resource_call(moved_permanently, Rs, Rd), case MovedPermanently of - {true, MovedURI} -> - RdLoc = wrq:set_resp_header("Location", MovedURI, Rd1), - respond(301, Rs1, RdLoc); - false -> - d(v3l5, Rs1, Rd1); - {error, Reason} -> - error_response(Reason, Rs1, Rd1); - {halt, Code} -> - respond(Code, Rs1, Rd1) + {true, MovedURI} -> + RdLoc = wrq:set_resp_header("Location", MovedURI, Rd1), + respond(301, Rs1, RdLoc); + false -> + d(v3l5, Rs1, Rd1); + {error, Reason} -> + error_response(Reason, Rs1, Rd1); + {halt, Code} -> + respond(Code, Rs1, Rd1) end; %% "Previously existed?" decision(v3k7, Rs, Rd) -> @@ -393,15 +408,15 @@ decision(v3k13, Rs, Rd) -> decision(v3l5, Rs, Rd) -> {MovedTemporarily, Rs1, Rd1} = resource_call(moved_temporarily, Rs, Rd), case MovedTemporarily of - {true, MovedURI} -> - RdLoc = wrq:set_resp_header("Location", MovedURI, Rd1), - respond(307, Rs1, RdLoc); - false -> - d(v3m5, Rs1, Rd1); - {error, Reason} -> - error_response(Reason, Rs1, Rd1); - {halt, Code} -> - respond(Code, Rs1, Rd1) + {true, MovedURI} -> + RdLoc = wrq:set_resp_header("Location", MovedURI, Rd1), + respond(307, Rs1, RdLoc); + false -> + d(v3m5, Rs1, Rd1); + {error, Reason} -> + error_response(Reason, Rs1, Rd1); + {halt, Code} -> + respond(Code, Rs1, Rd1) end; %% "POST?" decision(v3l7, Rs, Rd) -> @@ -679,15 +694,15 @@ choose_encoding(AccEncHdr, Rs, Rd) -> {EncodingsProvided, Rs1, Rd1} = resource_call(encodings_provided, Rs, Rd), Encs = [Enc || {Enc,_Fun} <- EncodingsProvided], case webmachine_util:choose_encoding(Encs, AccEncHdr) of - none -> - {none, Rs1, Rd1}; - ChosenEnc -> + none -> + {none, Rs1, Rd1}; + ChosenEnc -> RdEnc = case ChosenEnc of "identity" -> Rd1; _ -> wrq:set_resp_header("Content-Encoding",ChosenEnc, Rd1) end, {ok, RdEnc1} = webmachine_request:set_metadata('content-encoding',ChosenEnc,RdEnc), - {ChosenEnc, Rs1, RdEnc1} + {ChosenEnc, Rs1, RdEnc1} end. choose_charset(AccCharHdr, Rs, Rd) -> @@ -708,19 +723,19 @@ choose_charset(AccCharHdr, Rs, Rd) -> choose_upgrade(UpgradeHdr, Rs, Rd) -> {UpgradesProvided, Rs1, Rd1} = resource_call(upgrades_provided, Rs, Rd), - Provided1 = [ {string:to_lower(Prot), Prot, PFun} || {Prot, PFun} <- UpgradesProvided], - Requested = [ string:to_lower(string:strip(Up)) || Up <- string:tokens(UpgradeHdr, ",") ], - {choose_upgrade1(Requested, Provided1), Rs1, Rd1}. + Provided1 = [ {string:to_lower(Prot), Prot, PFun} || {Prot, PFun} <- UpgradesProvided], + Requested = [ string:to_lower(string:strip(Up)) || Up <- string:tokens(UpgradeHdr, ",") ], + {choose_upgrade1(Requested, Provided1), Rs1, Rd1}. - choose_upgrade1([], _) -> - none; - choose_upgrade1([Req|Requested], Provided) -> - case lists:keysearch(Req, 1, Provided) of - false -> - choose_upgrade1(Requested, Provided); - {value, {_, Protocol, UpgradeFun}} -> - {Protocol, UpgradeFun} - end. + choose_upgrade1([], _) -> + none; + choose_upgrade1([Req|Requested], Provided) -> + case lists:keysearch(Req, 1, Provided) of + false -> + choose_upgrade1(Requested, Provided); + {value, {_, Protocol, UpgradeFun}} -> + {Protocol, UpgradeFun} + end. variances(Rs, Rd) -> @@ -732,9 +747,9 @@ variances(Rs, Rd) -> end, {EncodingsProvided, Rs2, Rd2} = resource_call(encodings_provided, Rs1, Rd1), AcceptEncoding = case length(EncodingsProvided) of - 1 -> []; - 0 -> []; - _ -> ["Accept-Encoding"] + 1 -> []; + 0 -> []; + _ -> ["Accept-Encoding"] end, {CharsetsProvided, Rs3, Rd3} = resource_call(charsets_provided, Rs2, Rd2), AcceptCharset = case CharsetsProvided of diff --git a/src/webmachine_error_handler.erl b/src/webmachine_error_handler.erl index 7804d45..d47611c 100644 --- a/src/webmachine_error_handler.erl +++ b/src/webmachine_error_handler.erl @@ -31,6 +31,10 @@ render_error(Code, ReqData, Reason) -> {false,_} -> render_error_body(Code, ReqData, Reason) end. +render_error_body(400, ReqData, _Reason) -> + ReqData1 = wrq:set_resp_header("Content-Type", "text/html", ReqData), + {<<"400 Bad Request

Bad Request

The request was invalid.

mochiweb+webmachine web server
">>, ReqData1}; + render_error_body(404, ReqData, _Reason) -> ReqData1 = wrq:set_resp_header("Content-Type", "text/html", ReqData), {<<"404 Not Found

Not Found

The requested document was not found on this server.

mochiweb+webmachine web server
">>, ReqData1}; diff --git a/src/webmachine_mochiweb.erl b/src/webmachine_mochiweb.erl index 11f0f79..c90bded 100644 --- a/src/webmachine_mochiweb.erl +++ b/src/webmachine_mochiweb.erl @@ -140,8 +140,8 @@ loop(MochiReq, LoopOpts) -> erlang:put(mochiweb_request_force_close, true) end catch - error:_ -> - ?WM_DBG({error, erlang:get_stacktrace()}), + error:E -> + ?WM_DBG({error, E, erlang:get_stacktrace()}), {ok,RD3} = webmachine_request:send_response(500, RD2), Resource:stop(RD3), case application:get_env(webzmachine, webmachine_logger_module) of