Uploaded image for project: 'Couchbase .NET client library'
  1. Couchbase .NET client library
  2. NCBC-289

Does not return errors object on view operation

    XMLWordPrintable

Details

    • New Feature
    • Status: Resolved
    • Blocker
    • Resolution: Fixed
    • 1.2.7
    • 1.3.1
    • library

    Description

      The couchbase maunal shows the JSON for view responses:
      http://www.couchbase.com/docs/couchbase-manual-2.0/couchbase-views-writing-querying-errorcontrol.html

      The Java and python SDK have ability to access errors .net does not.

      Looking at CouchbaseViewHandler. It looks like its meant to be throwing an exception when it gets errors in the JSON. However that exception will never happen as there is a bug on lines 95 to 101 (code below). The two if statements counter each other. You can fix it via calling jsonReader.Read() after the 1st if statement however I believe that is not the correct solution. We should return an error object. I think the whole IEnumerator<T> TransformResults<T>(Func<JsonReader, T> rowTransformer, IDictionary<string, string> viewParams) needs looking at.

      if (jsonReader.TokenType == JsonToken.PropertyName
      && jsonReader.Depth == 1
      && ((string)jsonReader.Value) == "errors")
      {
      // we skip the deserialization if the array is null
      if (jsonReader.TokenType == Newtonsoft.Json.JsonToken.StartArray)
      {

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          Jeff is running with this change.

          ingenthr Matt Ingenthron added a comment - Jeff is running with this change.

          Jeff,

          From what I remember looking at the code it look like the error object would only get initialised once the JSON respond was full parsed.
          There were some operations that did not parsed the JSON to the end.

          Please let me know if the method I created in http://review.couchbase.org/#/c/29173/ will be changed.

          I know you been busy and this can wait until the December release.

          Thanks,
          Patrick

          pvarley Patrick Varley added a comment - Jeff, From what I remember looking at the code it look like the error object would only get initialised once the JSON respond was full parsed. There were some operations that did not parsed the JSON to the end. Please let me know if the method I created in http://review.couchbase.org/#/c/29173/ will be changed. I know you been busy and this can wait until the December release. Thanks, Patrick
          jmorris Jeff Morris added a comment -

          Patrick -

          I am mainly holding back on this only because we don't have valid unit tests to ensure that it works as expected. To make it easier to unit test, I am refactoring the code a bit so that any stream (not just the response from a web request) can run through that code. I started on this and then had to put it on the back burner for a bit, but will be jumping back to it as soon as 1.3.0 is released.

          If anything needs to be changed, I'll let you know!

          Thanks,

          Jeff

          jmorris Jeff Morris added a comment - Patrick - I am mainly holding back on this only because we don't have valid unit tests to ensure that it works as expected. To make it easier to unit test, I am refactoring the code a bit so that any stream (not just the response from a web request) can run through that code. I started on this and then had to put it on the back burner for a bit, but will be jumping back to it as soon as 1.3.0 is released. If anything needs to be changed, I'll let you know! Thanks, Jeff
          jmorris Jeff Morris added a comment -

          We decided to throw an exception when an error is detected for all view
          error cases. In the next version (2.x) of the client will make a decision
          on how we want the client to behave when an error is encountered when
          processing a view. This commit makes it consistent across all error cases
          and does not change the interface, which would likely impact users
          requiring them to change there code from handling exceptions to checking
          an errors property for failures.

          This commit also adds additional unit tests and refactors the
          CouchbaseViewHandler class so that we can pass streams into the
          ReadResponse method that contain text resembling errors returned from the
          server.

          jmorris Jeff Morris added a comment - We decided to throw an exception when an error is detected for all view error cases. In the next version (2.x) of the client will make a decision on how we want the client to behave when an error is encountered when processing a view. This commit makes it consistent across all error cases and does not change the interface, which would likely impact users requiring them to change there code from handling exceptions to checking an errors property for failures. This commit also adds additional unit tests and refactors the CouchbaseViewHandler class so that we can pass streams into the ReadResponse method that contain text resembling errors returned from the server.
          jmorris Jeff Morris added a comment -

          CR & Submitted via gerrit

          jmorris Jeff Morris added a comment - CR & Submitted via gerrit

          People

            jmorris Jeff Morris
            pvarley Patrick Varley
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Gerrit Reviews

                There are no open Gerrit changes

                PagerDuty