CouchDB
  1. CouchDB
  2. COUCHDB-1395

Not well-formed JSON in changes feed filtered by view.

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Critical Critical
    • Resolution: Unresolved
    • Affects Version/s: 1.2, 1.3
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Environment:

      Apache CouchDB 1.3.0a-d59cdd7-git
      Apache CouchDB 1.2.0a-0d8ddc8-git

      Description

      CouchDB returns invalid JSON response for _changes feed if filter view have failed somehow: by code mistake, bug, os timeout etc.

      Steps to reproduce:
      1. Create db and fill it with some documents.
      2. Create buggy view that would make view server crush for some document. For javascript one segfault and os timeout errors are actual due to any exceptions raised from map function is silenced. You view could be fine however for normal usage.
      3. Request changes feed and apply this buggy view as filter.

      Story:
      View function had never proceed design documents and mostly they are created with that knowledge. But in changes feed they have to process every document in database and DDocs too, so they breaks their original logic and creates unexpectable situation. Another side of problem is about custom query servers that could prevent view processing on first occurred exception or due dynamic code execution nature.
      Certainly, it's all about invalid view function that generates exception for some document in some case, but should _changes feed return malformed data in this case or just notify about problem somehow and emit valid parseable JSON?

      Expected result:
      Valid JSON response and some message about document processing error.

      Actual result:

      • About to connect() to localhost port 5984 (#0)
      • Trying 127.0.0.1... connected
      • Connected to localhost (127.0.0.1) port 5984 (#0)
        > GET /filtered_view_bug/_changes?filter=_view&view=bug/crush_on_ddoc HTTP/1.1
        > User-Agent: curl/7.21.4 (x86_64-pc-linux-gnu) libcurl/7.21.4 GnuTLS/2.10.5 zlib/1.2.5
        > Host: localhost:5984
        > Accept: application/json
        >
        < HTTP/1.1 200 OK
        < Transfer-Encoding: chunked
        < Server: CouchDB/1.3.0a-d59cdd7-git (Erlang OTP/R14B04)
        < ETag: "3IV66Q7INUYEHYKVWADD0CA8A"
        < Date: Thu, 26 Jan 2012 19:30:23 GMT
        < Content-Type: application/json
        < Cache-Control: must-revalidate
        <
        Unknown macro: {"results"}

        ,

        Unknown macro: {"seq"}

        ,

        Unknown macro: {"seq"}
      • Received problem 2 in the chunky parser
      • Closing connection #0
        curl: (56) Received problem 2 in the chunky parser

      Last chunk arrives with "HTTP/1.1 500 Internal Server Error" instead of size value.

      1. filtered_view_bug.sh
        0.8 kB
        Alexander Shorin

        Issue Links

          Activity

          Hide
          Randall Leeds added a comment -

          These appear to be the same issue in which reporting errors after headers are sent is ugly.

          Show
          Randall Leeds added a comment - These appear to be the same issue in which reporting errors after headers are sent is ugly.
          Hide
          Benoit Chesneau added a comment -

          Randall: I will put a new version of the patch in the middle of the week.

          I'm not sure about the delayed response thing. Shouldn't we either, have a way to report back the errors we add at the end? Eventually closing the connection ? Like sending an

          {"error": ... }

           ? I agree that the right thing to do anyway is sending the corrects header first.

          Show
          Benoit Chesneau added a comment - Randall: I will put a new version of the patch in the middle of the week. I'm not sure about the delayed response thing. Shouldn't we either, have a way to report back the errors we add at the end? Eventually closing the connection ? Like sending an {"error": ... }  ? I agree that the right thing to do anyway is sending the corrects header first.
          Hide
          Randall Leeds added a comment -

          I would advise that we wait for Benoit to publish the work he's started on refactoring the httpd core. Benoit, any ETA on a proof of concept there?

          Show
          Randall Leeds added a comment - I would advise that we wait for Benoit to publish the work he's started on refactoring the httpd core. Benoit, any ETA on a proof of concept there?
          Hide
          Robert Newson added a comment -

          +1 on porting the delayed response work to couchdb.

          Show
          Robert Newson added a comment - +1 on porting the delayed response work to couchdb.
          Hide
          Paul Joseph Davis added a comment -

          Huh. We've dealt with this exact issue in BigCouch but most of our errors were related to errors between machines. Hadn't seen a report of it happening in CouchDB itself. Either way, its definitely something we should fix.

          The issue as Alexander quite well diagnosed is basically that we encounter an error after the response code and HTTP headers have already been sent to the client. The current code catches an error and then doesn't act appropriately (it just says, "Oh hai! You are a lovely exception! Off you go to the client!" but that just rewrites a 500 response into the middle of a chunked response which is obviously fail.

          So, the error is obvious, but the solution is not. Internally we argued quite extensively on the best way to do this. For BigCouch we ended up agreeing (after much persuasion) that the best solution is to just immediately close the socket (thereby creating invalid JSON (on purpose)) and log the error. The reasoning for this was that HTTP has no direct way of signaling an error after the request has started. It might be possible to send a X-CouchDB-Status trailer or something but that's just ridiculous cause no one would support such a thing and it would be quite surprising in the oddity.

          The only other obvious place to put the error is into the response body itself similar to what Alexander suggested. While it would be theoretically beneficial for a client to know information about what went wrong, I argued long and hard that it would end up causing more problems than it solved. First, the response has already started successfully, so the odds that the error is something the client can do anything about are minimal (ie, it was probably an ephemeral error from something else crashing). Secondly, if its valid JSON and and the client lib doesn't expect an error response, at best it'll throw some sort of error because of the mismatched JSON (ie, we send an

          {"error": "reason"}

          instead of the next changes row). But my worst fear here is that the client lib doesn't throw an error at all. It just says "oh hay, no moar changes!" and carries on. This I argued would be super bad in that an error occurred yet the user was unaware of the issue.

          Thus the only way to be super certain that a user would know that an error occurred was to close the connection leaving invalid JSON which would trigger an error deep in the stack that wouldn't be accidentally ignored.

          Not entirely sure what everyone else would expect but I figured I'd try and give as much background on the issue as possible.

          Also the fix is reasonable easy either way. We pass an HTTP response around in a couple places. To fix we just wrapped it with a special delayed response structure for long requests so that the first thing that tries to write a response to the client flushes the status and headers. Then when an exception is caught we have a boolean on whether to report the error or not (ie, is this response object a delayed response? Yes? Send error response instead, else kill connection).

          Show
          Paul Joseph Davis added a comment - Huh. We've dealt with this exact issue in BigCouch but most of our errors were related to errors between machines. Hadn't seen a report of it happening in CouchDB itself. Either way, its definitely something we should fix. The issue as Alexander quite well diagnosed is basically that we encounter an error after the response code and HTTP headers have already been sent to the client. The current code catches an error and then doesn't act appropriately (it just says, "Oh hai! You are a lovely exception! Off you go to the client!" but that just rewrites a 500 response into the middle of a chunked response which is obviously fail. So, the error is obvious, but the solution is not. Internally we argued quite extensively on the best way to do this. For BigCouch we ended up agreeing (after much persuasion) that the best solution is to just immediately close the socket (thereby creating invalid JSON (on purpose)) and log the error. The reasoning for this was that HTTP has no direct way of signaling an error after the request has started. It might be possible to send a X-CouchDB-Status trailer or something but that's just ridiculous cause no one would support such a thing and it would be quite surprising in the oddity. The only other obvious place to put the error is into the response body itself similar to what Alexander suggested. While it would be theoretically beneficial for a client to know information about what went wrong, I argued long and hard that it would end up causing more problems than it solved. First, the response has already started successfully, so the odds that the error is something the client can do anything about are minimal (ie, it was probably an ephemeral error from something else crashing). Secondly, if its valid JSON and and the client lib doesn't expect an error response, at best it'll throw some sort of error because of the mismatched JSON (ie, we send an {"error": "reason"} instead of the next changes row). But my worst fear here is that the client lib doesn't throw an error at all. It just says "oh hay, no moar changes!" and carries on. This I argued would be super bad in that an error occurred yet the user was unaware of the issue. Thus the only way to be super certain that a user would know that an error occurred was to close the connection leaving invalid JSON which would trigger an error deep in the stack that wouldn't be accidentally ignored. Not entirely sure what everyone else would expect but I figured I'd try and give as much background on the issue as possible. Also the fix is reasonable easy either way. We pass an HTTP response around in a couple places. To fix we just wrapped it with a special delayed response structure for long requests so that the first thing that tries to write a response to the client flushes the status and headers. Then when an exception is caught we have a boolean on whether to report the error or not (ie, is this response object a delayed response? Yes? Send error response instead, else kill connection).
          Hide
          Alexander Shorin added a comment -

          Simple script to test this issue.

          Show
          Alexander Shorin added a comment - Simple script to test this issue.

            People

            • Assignee:
              Unassigned
              Reporter:
              Alexander Shorin
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:

                Development