CouchDB
  1. CouchDB
  2. COUCHDB-452

couch logs an [error] record if a client disconnects

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Invalid
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      If a client makes a request which returns multiple rows (eg, a query) but disconnects before reading the response, couch logs:

      [error] [<0.66.0>] Uncaught error in HTTP request:

      {exit,normal}

      [info] [<0.66.0>] Stacktrace: [

      {mochiweb_request,send,2}

      ,

      {couch_httpd,send_chunk,2}

      ,

      {couch_httpd_view,send_json_view_row,5}

      ,

      {couch_httpd_view,'-make_view_fold_fun/6-fun-0-',12}

      ,

      {couch_view,fold_fun,4}

      ,

      {couch_btree,stream_kv_node2,7}

      ,

      {couch_btree,stream_kp_node,7}

      ,

      {couch_btree,fold,5}

      ]

      This could lead someone to conclude couch has an error which is not correct. A simple [info] record recording the premature normal exit is probably more appropriate.

      FYI, the easiest way I found to repro this was to execute:

      % python -c "import urllib; urllib.urlopen('http://127.0.0.1:5984/your_db/_design/your_design_doc/_view/your_view?reduce=false').close()"

      which causes Python to open a connection and close it without reading anything. I'll attach a patch as suggested generally by jchris.

        Issue Links

          Activity

          Hide
          Mark Hammond added a comment -

          Patch which causes a simple:

          [info] [<0.66.0>] remote connection dropped prematurely

          record to be written. jchris also suggested I remove the error:badarg and error:function_clause handlers as these are picked up by the Tag:Error handler - so they have gone too.

          Show
          Mark Hammond added a comment - Patch which causes a simple: [info] [<0.66.0>] remote connection dropped prematurely record to be written. jchris also suggested I remove the error:badarg and error:function_clause handlers as these are picked up by the Tag:Error handler - so they have gone too.
          Hide
          Jan Lehnardt added a comment -

          Why do we get rid of the badarg & function_clause cases?

          Show
          Jan Lehnardt added a comment - Why do we get rid of the badarg & function_clause cases?
          Hide
          Mark Hammond added a comment -

          > Why do we get rid of the badarg & function_clause cases?

          jchris implies, but I didn't confirm, the Tag:Error handler will still match these.

          Show
          Mark Hammond added a comment - > Why do we get rid of the badarg & function_clause cases? jchris implies, but I didn't confirm, the Tag:Error handler will still match these.
          Hide
          Jan Lehnardt added a comment -

          It will match, but why don't we want to keep the special case. maybe the question boils down to why did we have them in the first place. Just wondering about the change.

          Show
          Jan Lehnardt added a comment - It will match, but why don't we want to keep the special case. maybe the question boils down to why did we have them in the first place. Just wondering about the change.
          Hide
          Adam Kocoloski added a comment -

          Thanks for fixing the

          {exit,normal}

          stuff, Mark. I don't have an opinion about whether to remove the explicit checks for badarg and function_clause errors.

          Show
          Adam Kocoloski added a comment - Thanks for fixing the {exit,normal} stuff, Mark. I don't have an opinion about whether to remove the explicit checks for badarg and function_clause errors.
          Hide
          Curt Arnold added a comment -

          Seems to be a duplicate (or at least similar) to COUCHDB-394.

          Show
          Curt Arnold added a comment - Seems to be a duplicate (or at least similar) to COUCHDB-394 .
          Hide
          Curt Arnold added a comment -

          Raised this issue on the mochiweb project site: http://code.google.com/p/mochiweb/issues/detail?id=39 in addition to the COUCHDB-394.

          Mochiweb discards the value returned from gen_tcp:send, so there is not any way to distinguish a premature close from some more significant problem.

          I definitely spent some time trying to figure out why I was getting 500 return codes in the logs on what appeared to be normal activities.

          Show
          Curt Arnold added a comment - Raised this issue on the mochiweb project site: http://code.google.com/p/mochiweb/issues/detail?id=39 in addition to the COUCHDB-394 . Mochiweb discards the value returned from gen_tcp:send, so there is not any way to distinguish a premature close from some more significant problem. I definitely spent some time trying to figure out why I was getting 500 return codes in the logs on what appeared to be normal activities.
          Hide
          Adam Kocoloski added a comment -

          Yes, that's a good point Curt. Just yesterday Jason needed to add some logging in MochiWeb to find out that a gen_tcp:recv was timing out. The log just said

          {exit,normal}

          there, too.

          Show
          Adam Kocoloski added a comment - Yes, that's a good point Curt. Just yesterday Jason needed to add some logging in MochiWeb to find out that a gen_tcp:recv was timing out. The log just said {exit,normal} there, too.
          Hide
          Chris Anderson added a comment -

          The special case matches were added one at a time, because those are two special cases where having a stacktrace in the logs can help with debugging on the ML and IRC.

          However, the general matcher now logs a stacktrace just the same as they did, so it's fine to let the general case handle them. (I guess we finally got few enough errors coming from deep within CouchDB that a stacktrace on any uncaught error works fine.)

          Removing the special case matchers just cleans up the code and does not change functionality.

          Show
          Chris Anderson added a comment - The special case matches were added one at a time, because those are two special cases where having a stacktrace in the logs can help with debugging on the ML and IRC. However, the general matcher now logs a stacktrace just the same as they did, so it's fine to let the general case handle them. (I guess we finally got few enough errors coming from deep within CouchDB that a stacktrace on any uncaught error works fine.) Removing the special case matchers just cleans up the code and does not change functionality.
          Hide
          Mark Hammond added a comment -

          Out of date - the trunk now completely ignores that case without even logging an info record.

          Show
          Mark Hammond added a comment - Out of date - the trunk now completely ignores that case without even logging an info record.

            People

            • Assignee:
              Unassigned
              Reporter:
              Mark Hammond
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development