CouchDB
  1. CouchDB
  2. COUCHDB-1330

provides() does not supports returning a status code or headers

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 1.1.1
    • Fix Version/s: None
    • Component/s: JavaScript View Server
    • Labels:
      None
    • Environment:

      Iris Couch

    • Skill Level:
      New Contributors Level (Easy)

      Description

      An Iris Couch user lost the ability to redirect from a provides() callback in a show function: http://getsatisfaction.com/iriscouch/topics/_provides_method_not_returning_code_and_headers

      This show function no longer works:

      function (doc, req) {
      provides('html', function () {
      return {
      headers:

      { Location: 'http://www.iriscouch.com' }

      ,
      code: 302,
      body: 'Redirecting to IrisCouch website...'
      };
      });
      }

      This looks like a regression from 1.1.0; although there are no unit tests so perhaps strictly speaking it was unsupported functionality. Git bisect places the error at the patch to COUCHDB-1272.

        Activity

        Jason Smith created issue -
        Hide
        Jason Smith added a comment -

        Patch with fix and unit tests

        Show
        Jason Smith added a comment - Patch with fix and unit tests
        Jason Smith made changes -
        Field Original Value New Value
        Attachment A_0001-Support-provides-callbacks-indicating-status-code-an.patch [ 12502087 ]
        Hide
        Jason Smith added a comment -

        Work is also in GitHub, https://github.com/jhs/couchdb/tree/COUCHDB-1330, note this is a patch against the 1.1.1 tag.

        Show
        Jason Smith added a comment - Work is also in GitHub, https://github.com/jhs/couchdb/tree/COUCHDB-1330 , note this is a patch against the 1.1.1 tag.
        Hide
        Nathan Vander Wilt added a comment -

        This regression was introduced by https://github.com/apache/couchdb/commit/92f70219ce5ba487e4eb65dea7d16a9168a8547f#share/server/render.js for COUCHDB-1272 and also breaks returning "json" instead of "body". Jason's patch looks good to me for fixing it, and I'd love to see it applied soon.

        Show
        Nathan Vander Wilt added a comment - This regression was introduced by https://github.com/apache/couchdb/commit/92f70219ce5ba487e4eb65dea7d16a9168a8547f#share/server/render.js for COUCHDB-1272 and also breaks returning "json" instead of "body". Jason's patch looks good to me for fixing it, and I'd love to see it applied soon.
        Hide
        Nathan Vander Wilt added a comment -

        Just noticed one thing that might warrant further review by someone who understands the stack a little better:

        Jason's patch will always set "provided_resp.body", potentially to an empty string. Could the presence of that (albeit empty) string conflict with provided_resp.json field at the Erlang level?

        Show
        Nathan Vander Wilt added a comment - Just noticed one thing that might warrant further review by someone who understands the stack a little better: Jason's patch will always set "provided_resp.body", potentially to an empty string. Could the presence of that (albeit empty) string conflict with provided_resp.json field at the Erlang level?
        Hide
        Alexander Shorin added a comment -

        Oops, it looks like I'd break some things...My position was that provides functions are operates with data level, providing specific handler for requested content-type, while headers/status code is metadata that should be determent early.

        However, @Nathan right, there could be

        {"json": ...}

        body definition and also

        {"base64": ...}

        that processed behind of scene at CouchDB side. Mixing them better to count as unexpectable result due to it depends on key order in json response from query server.

        However, there is possible case for data collision:

        function (doc, req) {
        provides('html', function () {
        return {
        headers:

        { Location: 'http://www.iriscouch.com' }

        ,
        code: 302,
        json:

        {status: 'redirecting', message: 'We will take you with us to IrisCouch!'}

        };
        });
        return {
        code: 404,
        headers:

        {"X-Couch-Reponse": "Relax!"}

        json:

        {error: 'not_a_chance', reason: "I'm tired, leave me alone!"}

        }
        }

        What is expected output? 404 and error message or 302 and redirect? Should headers be merged or provided_resp one only wins? IMHO, provided_resp should override all resp data, except in case of chunked response when resp.body = resp.chunks + resp.body + provided_resp.chunks + provided_resp.body.

        If nobody stand against those terms I'll update patch to follow described behavior.

        Show
        Alexander Shorin added a comment - Oops, it looks like I'd break some things...My position was that provides functions are operates with data level, providing specific handler for requested content-type, while headers/status code is metadata that should be determent early. However, @Nathan right, there could be {"json": ...} body definition and also {"base64": ...} that processed behind of scene at CouchDB side. Mixing them better to count as unexpectable result due to it depends on key order in json response from query server. However, there is possible case for data collision: function (doc, req) { provides('html', function () { return { headers: { Location: 'http://www.iriscouch.com' } , code: 302, json: {status: 'redirecting', message: 'We will take you with us to IrisCouch!'} }; }); return { code: 404, headers: {"X-Couch-Reponse": "Relax!"} json: {error: 'not_a_chance', reason: "I'm tired, leave me alone!"} } } What is expected output? 404 and error message or 302 and redirect? Should headers be merged or provided_resp one only wins? IMHO, provided_resp should override all resp data, except in case of chunked response when resp.body = resp.chunks + resp.body + provided_resp.chunks + provided_resp.body. If nobody stand against those terms I'll update patch to follow described behavior.
        Hide
        Alexander Shorin added a comment -

        I've ported fix from Python query server.

        Current behavior: provides response could override everything from original one, but result body is still been merged by rules that I'd mentioned before:
        > resp.body = resp.chunks + resp.body + provided_resp.chunks + provided_resp.body.

        Show
        Alexander Shorin added a comment - I've ported fix from Python query server. Current behavior: provides response could override everything from original one, but result body is still been merged by rules that I'd mentioned before: > resp.body = resp.chunks + resp.body + provided_resp.chunks + provided_resp.body.
        Alexander Shorin made changes -
        Attachment couchdb-1330.patch [ 12517863 ]

          People

          • Assignee:
            Unassigned
            Reporter:
            Jason Smith
          • Votes:
            2 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Time Tracking

              Estimated:
              Original Estimate - 4h
              4h
              Remaining:
              Remaining Estimate - 4h
              4h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development