You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by kx...@apache.org on 2015/11/24 21:16:18 UTC

[1/4] chttpd commit: updated refs/heads/master to 7dcf127

Repository: couchdb-chttpd
Updated Branches:
  refs/heads/master ddba20778 -> 7dcf1279b


Handle errors from before_request/after_request

This refactoring fixes the handling of errors from before_request.
Unify how we deal with errors among before_request and handle_request.


Project: http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/repo
Commit: http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/commit/6a1858f8
Tree: http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/tree/6a1858f8
Diff: http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/diff/6a1858f8

Branch: refs/heads/master
Commit: 6a1858f8b2604c4af96b7ca4d423ebacd2f98603
Parents: ddba207
Author: ILYA Khlopotov <ii...@ca.ibm.com>
Authored: Wed Nov 18 08:25:39 2015 -0800
Committer: ILYA Khlopotov <ii...@ca.ibm.com>
Committed: Wed Nov 18 08:32:50 2015 -0800

----------------------------------------------------------------------
 src/chttpd.erl | 138 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 84 insertions(+), 54 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/blob/6a1858f8/src/chttpd.erl
----------------------------------------------------------------------
diff --git a/src/chttpd.erl b/src/chttpd.erl
index d3bc4e7..0e93728 100644
--- a/src/chttpd.erl
+++ b/src/chttpd.erl
@@ -157,11 +157,6 @@ handle_request_int(MochiReq) ->
     {"/" ++ Path, _, _} = mochiweb_util:urlsplit_path(RawUri),
 
     Peer = MochiReq:get(peer),
-    LogForClosedSocket = io_lib:format("mochiweb_recv_error for ~s - ~p ~s", [
-        Peer,
-        MochiReq:get(method),
-        RawUri
-    ]),
 
     Method1 =
     case MochiReq:get(method) of
@@ -209,21 +204,58 @@ handle_request_int(MochiReq) ->
                 || Part <- string:tokens(Path, "/")]
     },
 
-    {ok, HttpReq} = chttpd_plugin:before_request(HttpReq0),
+    % put small token on heap to keep requests synced to backend calls
+    erlang:put(nonce, Nonce),
+
+    % suppress duplicate log
+    erlang:put(dont_log_request, true),
+
+    Result0 = case before_request(HttpReq0) of
+        {ok, HttpReq1} ->
+            process_request(HttpReq1);
+        {error, Response} ->
+            {HttpReq0, Response}
+    end,
+
+    {HttpReq3, HttpResp0} = result(Result0, HttpReq0),
+
+    case after_request(HttpReq3, HttpResp0) of
+        #httpd_resp{status = ok, response = Resp} ->
+            {ok, Resp};
+        #httpd_resp{status = aborted, reason = Reason} ->
+            couch_log:error("Response abnormally terminated: ~p", [Reason]),
+            exit(normal)
+    end.
+
+before_request(HttpReq) ->
+    try
+        chttpd_plugin:before_request(HttpReq)
+    catch Tag:Error ->
+        {error, catch_error(HttpReq, Tag, Error)}
+    end.
+
+after_request(HttpReq, HttpResp0) ->
+    {ok, HttpResp1} =
+        try
+            chttpd_plugin:after_request(HttpReq, HttpResp0)
+        catch _Tag:Error ->
+            Stack = erlang:get_stacktrace(),
+            send_error(HttpReq, {Error, nil, Stack}),
+            {ok, HttpResp0#httpd_resp{status = aborted}}
+        end,
+    HttpResp2 = update_stats(HttpReq, HttpResp1),
+    maybe_log(HttpReq, HttpResp2),
+    HttpResp2.
 
+process_request(#httpd{mochi_req = MochiReq} = HttpReq) ->
     HandlerKey =
         case HttpReq#httpd.path_parts of
             [] -> <<>>;
             [Key|_] -> ?l2b(quote(Key))
         end,
 
-    % put small token on heap to keep requests synced to backend calls
-    erlang:put(nonce, Nonce),
-
-    % suppress duplicate log
-    erlang:put(dont_log_request, true),
+    RawUri = MochiReq:get(raw_path),
 
-    Result0 =
     try
         couch_httpd:validate_host(HttpReq),
         check_request_uri_length(RawUri),
@@ -242,49 +274,47 @@ handle_request_int(MochiReq) ->
         Response ->
             Response
         end
-    catch
-        throw:{http_head_abort, Resp0} ->
-            {ok, Resp0};
-        throw:{http_abort, Resp0, Reason0} ->
-            {aborted, Resp0, Reason0};
-        throw:{invalid_json, _} ->
-            send_error(HttpReq, {bad_request, "invalid UTF-8 JSON"});
-        exit:{mochiweb_recv_error, E} ->
-            couch_log:notice(LogForClosedSocket ++ " - ~p", [E]),
-            exit(normal);
-        exit:{uri_too_long, _} ->
-            send_error(HttpReq, request_uri_too_long);
-        exit:{body_too_large, _} ->
-            send_error(HttpReq, request_entity_too_large);
-        throw:Error ->
-            send_error(HttpReq, Error);
-        error:database_does_not_exist ->
-            send_error(HttpReq, database_does_not_exist);
-        Tag:Error ->
-            Stack = erlang:get_stacktrace(),
-            % TODO improve logging and metrics collection for client disconnects
-            case {Tag, Error, Stack} of
-                {exit, normal, [{mochiweb_request, send, _, _} | _]} ->
-                    exit(normal); % Client disconnect (R15+)
-                {exit, normal, [{mochiweb_request, send, _} | _]} ->
-                    exit(normal); % Client disconnect (R14)
-                _Else ->
-                    send_error(HttpReq, {Error, nil, Stack})
-            end
-    end,
-
-    {HttpReq1, HttpResp0} = result(Result0, HttpReq),
-    {ok, HttpResp1} = chttpd_plugin:after_request(HttpReq1, HttpResp0),
-
-    HttpResp2 = update_stats(HttpReq1, HttpResp1),
-    maybe_log(HttpReq1, HttpResp2),
+    catch Tag:Error ->
+        catch_error(HttpReq, Tag, Error)
+    end.
 
-    case HttpResp2 of
-        #httpd_resp{status = ok, response = Resp} ->
-            {ok, Resp};
-        #httpd_resp{status = aborted, reason = Reason} ->
-            couch_log:error("Response abnormally terminated: ~p", [Reason]),
-            exit(normal)
+catch_error(_HttpReq, throw, {http_head_abort, Resp}) ->
+    {ok, Resp};
+catch_error(_HttpReq, throw, {http_abort, Resp, Reason}) ->
+    {aborted, Resp, Reason};
+catch_error(HttpReq, throw, {invalid_json, _}) ->
+    send_error(HttpReq, {bad_request, "invalid UTF-8 JSON"});
+catch_error(HttpReq, exit, {mochiweb_recv_error, E}) ->
+    #httpd{
+        mochi_req = MochiReq,
+        peer = Peer,
+        original_method = Method
+    } = HttpReq,
+    LogForClosedSocket = io_lib:format("mochiweb_recv_error for ~s - ~p ~s", [
+        Peer,
+        Method,
+        MochiReq:get(raw_path)
+    ]),
+    couch_log:notice(LogForClosedSocket ++ " - ~p", [E]),
+    exit(normal);
+catch_error(HttpReq, exit, {uri_too_long, _}) ->
+    send_error(HttpReq, request_uri_too_long);
+catch_error(HttpReq, exit, {body_too_large, _}) ->
+    send_error(HttpReq, request_entity_too_large);
+catch_error(HttpReq, throw, Error) ->
+    send_error(HttpReq, Error);
+catch_error(HttpReq, error, database_does_not_exist) ->
+    send_error(HttpReq, database_does_not_exist);
+catch_error(HttpReq, Tag, Error) ->
+    Stack = erlang:get_stacktrace(),
+    % TODO improve logging and metrics collection for client disconnects
+    case {Tag, Error, Stack} of
+        {exit, normal, [{mochiweb_request, send, _, _} | _]} ->
+            exit(normal); % Client disconnect (R15+)
+        {exit, normal, [{mochiweb_request, send, _} | _]} ->
+            exit(normal); % Client disconnect (R14)
+        _Else ->
+            send_error(HttpReq, {Error, nil, Stack})
     end.
 
 result({#httpd{} = Req, Result}, _) ->


[3/4] chttpd commit: updated refs/heads/master to 7dcf127

Posted by kx...@apache.org.
Refactor logging statement


Project: http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/repo
Commit: http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/commit/21abde55
Tree: http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/tree/21abde55
Diff: http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/diff/21abde55

Branch: refs/heads/master
Commit: 21abde5588b244743294bf647c745dbd75bb4997
Parents: 5e44123
Author: ILYA Khlopotov <ii...@ca.ibm.com>
Authored: Thu Nov 19 10:10:39 2015 -0800
Committer: ILYA Khlopotov <ii...@ca.ibm.com>
Committed: Thu Nov 19 10:10:39 2015 -0800

----------------------------------------------------------------------
 src/chttpd.erl | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/blob/21abde55/src/chttpd.erl
----------------------------------------------------------------------
diff --git a/src/chttpd.erl b/src/chttpd.erl
index 1bfc223..5459f9b 100644
--- a/src/chttpd.erl
+++ b/src/chttpd.erl
@@ -290,12 +290,11 @@ catch_error(HttpReq, exit, {mochiweb_recv_error, E}) ->
         peer = Peer,
         original_method = Method
     } = HttpReq,
-    LogForClosedSocket = io_lib:format("mochiweb_recv_error for ~s - ~p ~s", [
+    couch_log:notice("mochiweb_recv_error for ~s - ~p ~s - ~p", [
         Peer,
         Method,
-        MochiReq:get(raw_path)
-    ]),
-    couch_log:notice(LogForClosedSocket ++ " - ~p", [E]),
+        MochiReq:get(raw_path),
+        E]),
     exit(normal);
 catch_error(HttpReq, exit, {uri_too_long, _}) ->
     send_error(HttpReq, request_uri_too_long);


[2/4] chttpd commit: updated refs/heads/master to 7dcf127

Posted by kx...@apache.org.
Drop R14 support


Project: http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/repo
Commit: http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/commit/5e44123a
Tree: http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/tree/5e44123a
Diff: http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/diff/5e44123a

Branch: refs/heads/master
Commit: 5e44123a4accc1e61546037dec9577806a40f796
Parents: 6a1858f
Author: ILYA Khlopotov <ii...@ca.ibm.com>
Authored: Thu Nov 19 10:09:52 2015 -0800
Committer: ILYA Khlopotov <ii...@ca.ibm.com>
Committed: Thu Nov 19 10:09:52 2015 -0800

----------------------------------------------------------------------
 src/chttpd.erl | 2 --
 1 file changed, 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/blob/5e44123a/src/chttpd.erl
----------------------------------------------------------------------
diff --git a/src/chttpd.erl b/src/chttpd.erl
index 0e93728..1bfc223 100644
--- a/src/chttpd.erl
+++ b/src/chttpd.erl
@@ -311,8 +311,6 @@ catch_error(HttpReq, Tag, Error) ->
     case {Tag, Error, Stack} of
         {exit, normal, [{mochiweb_request, send, _, _} | _]} ->
             exit(normal); % Client disconnect (R15+)
-        {exit, normal, [{mochiweb_request, send, _} | _]} ->
-            exit(normal); % Client disconnect (R14)
         _Else ->
             send_error(HttpReq, {Error, nil, Stack})
     end.


[4/4] chttpd commit: updated refs/heads/master to 7dcf127

Posted by kx...@apache.org.
Get rid of confusing function


Project: http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/repo
Commit: http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/commit/7dcf1279
Tree: http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/tree/7dcf1279
Diff: http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/diff/7dcf1279

Branch: refs/heads/master
Commit: 7dcf1279bcdb546b4b284bc8bcbd51843849b04a
Parents: 21abde5
Author: ILYA Khlopotov <ii...@ca.ibm.com>
Authored: Thu Nov 19 10:11:25 2015 -0800
Committer: ILYA Khlopotov <ii...@ca.ibm.com>
Committed: Thu Nov 19 10:11:25 2015 -0800

----------------------------------------------------------------------
 src/chttpd.erl | 51 ++++++++++++++++++++++-----------------------------
 1 file changed, 22 insertions(+), 29 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/couchdb-chttpd/blob/7dcf1279/src/chttpd.erl
----------------------------------------------------------------------
diff --git a/src/chttpd.erl b/src/chttpd.erl
index 5459f9b..7e214a9 100644
--- a/src/chttpd.erl
+++ b/src/chttpd.erl
@@ -210,16 +210,24 @@ handle_request_int(MochiReq) ->
     % suppress duplicate log
     erlang:put(dont_log_request, true),
 
-    Result0 = case before_request(HttpReq0) of
+    {HttpReq2, Response} = case before_request(HttpReq0) of
         {ok, HttpReq1} ->
             process_request(HttpReq1);
-        {error, Response} ->
-            {HttpReq0, Response}
+        {error, Response0} ->
+            {HttpReq0, Response0}
     end,
 
-    {HttpReq3, HttpResp0} = result(Result0, HttpReq0),
+    {Status, Code, Reason, Resp} = split_response(Response),
 
-    case after_request(HttpReq3, HttpResp0) of
+    HttpResp = #httpd_resp{
+        code = Code,
+        status = Status,
+        response = Resp,
+        nonce = HttpReq2#httpd.nonce,
+        reason = Reason
+    },
+
+    case after_request(HttpReq2, HttpResp) of
         #httpd_resp{status = ok, response = Resp} ->
             {ok, Resp};
         #httpd_resp{status = aborted, reason = Reason} ->
@@ -269,13 +277,13 @@ process_request(#httpd{mochi_req = MochiReq} = HttpReq) ->
                     fun chttpd_auth_request:authorize_request/1),
                 {AuthorizedReq, HandlerFun(AuthorizedReq)};
             Response ->
-                Response
+                {HttpReq, Response}
             end;
         Response ->
-            Response
+            {HttpReq, Response}
         end
     catch Tag:Error ->
-        catch_error(HttpReq, Tag, Error)
+        {HttpReq, catch_error(HttpReq, Tag, Error)}
     end.
 
 catch_error(_HttpReq, throw, {http_head_abort, Resp}) ->
@@ -314,27 +322,12 @@ catch_error(HttpReq, Tag, Error) ->
             send_error(HttpReq, {Error, nil, Stack})
     end.
 
-result({#httpd{} = Req, Result}, _) ->
-    result(Result, Req);
-result(Result, #httpd{nonce = Nonce} = Req) ->
-    {Status, Code, Reason} = case Result of
-        {ok, #delayed_resp{resp=Resp}} ->
-            {ok, Resp:get(code), undefined};
-        {ok, Resp} ->
-            {ok, Resp:get(code), undefined};
-        {aborted, Resp, AbortReason} ->
-            {aborted, Resp:get(code), AbortReason}
-        end,
-
-    HttpResp = #httpd_resp{
-        code = Code,
-        status = Status,
-        response = Resp,
-        nonce = Nonce,
-        reason = Reason
-    },
-    {Req, HttpResp}.
-
+split_response({ok, #delayed_resp{resp=Resp}}) ->
+    {ok, Resp:get(code), undefined, Resp};
+split_response({ok, Resp}) ->
+    {ok, Resp:get(code), undefined, Resp};
+split_response({aborted, Resp, AbortReason}) ->
+    {aborted, Resp:get(code), AbortReason, Resp}.
 
 update_stats(HttpReq, #httpd_resp{end_ts = undefined} = Res) ->
     update_stats(HttpReq, Res#httpd_resp{end_ts = os:timestamp()});