Github user chewbranca commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/101#discussion_r54459627
--- Diff: src/chttpd.erl ---
@@ -689,12 +689,9 @@ send_chunk(Resp, Data) ->
Resp:write_chunk(Data),
{ok, Resp}.
-send_response(#httpd{mochi_req=MochiReq}=Req, Code, Headers0, Body) ->
- couch_stats:increment_counter([couchdb, httpd_status_codes, Code]),
- Headers = Headers0 ++ server_header() ++
- [timing(), reqid() | couch_httpd_auth:cookie_auth_header(Req,
Headers0)],
- {ok, MochiReq:respond({Code, Headers, Body})}.
-
+send_response(#httpd{} = Req, Code, Headers0, Body) ->
+ Headers1 = [timing(), reqid() | Headers0],
+ couch_httpd:send_response(Req, Code, Headers1, Body).
--- End diff --
Switching this to send the response the `couch_httpd:send_response/4` [1]
seems a bit odd to me, or at the very least backwards. But either way,
switching this changes the underlying semantics in a few ways that we should be
clear we want. As you can see in [1] there is a fair bit of things going on,
that we may or may not want.
First, in [2] we log the request through couch_httpd, but we're also
logging the request in chttpd [3], so it seems like we'll be getting double log
items for requests.
Second, we'll be adding in the `http_1_0_keep_alive/2` hack [4]. I would
like to hear from @davisp @rnewson (or anyone else familiar with the nuances
here) as to whether reintroducing this to chttpd is a good idea.
Third, we're introducing an additional layer of logging that logs the body
of requests in `error` for 500s, and `debug` for 400s. Is this a change we
explicitly want? IMO it would be handy to have the response bodies logged for
500s, but let's be explicit about deciding to add that in to requests through
chttpd.
[1]
https://github.com/apache/couchdb-couch/blob/master/src/couch_httpd.erl#L723-L737
[2]
https://github.com/apache/couchdb-couch/blob/master/src/couch_httpd.erl#L724
[3]
https://github.com/cloudant/couchdb-chttpd/blob/2945-remove-couch_http_cors/src/chttpd.erl#L358-L377
[4]
https://github.com/apache/couchdb-couch/blob/master/src/couch_httpd.erl#L726
[5]
https://github.com/apache/couchdb-couch/blob/master/src/couch_httpd.erl#L727-L732
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
