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

Lock related errors not setting Exception and Status fields correctly in the result

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.5.12
    • Component/s: library
    • Labels:
      None

      Description

      When using Couchbase Server 5.x, and you get a lock related error, the Status filed is set of "Failure" (-3) and the Exception field is set to null.

      You can reproduce with the following code:

      var cluster = new Cluster(new ClientConfiguration
      {
          Servers = new List<Uri> { new Uri("http://10.111.175.101") }
      });
       
      var authenticator = new PasswordAuthenticator("user", "password");
      cluster.Authenticate(authenticator);
      var bucket = cluster.OpenBucket("test");
       
      dynamic attempt1 = bucket.GetAndLock<dynamic>("12345", TimeSpan.FromSeconds(10));
      dynamic attempt2 = bucket.GetAndLock<dynamic>("12345", TimeSpan.FromSeconds(10));
      

      attempt2 above should be the "bad" result.

        Attachments

          Issue Links

          For Gerrit Dashboard: NCBC-1679
          # Subject Branch Project Status CR V

            Activity

            Hide
            jmorris Jeff Morris added a comment -

            I need to look deeper into the Error Map RFC, but the logic here doesn't seem right; if the server sends a status, the client should respect and return it back to the application not convert it to a generic "failure" status. Any additional error map information is just a bonus. 

            As for the exception field be null, its because there is no generated exception (in this case it would be the expected output given the input) - its simply the server status of the operation request. That being said, an exception which includes the status and any additional error map information could be returned. It does seem a bit redundant, tbh.

             

            Show
            jmorris Jeff Morris added a comment - I need to look deeper into the Error Map RFC, but the logic here doesn't seem right; if the server sends a status, the client should respect and return it back to the application not convert it to a generic "failure" status. Any additional error map information is just a bonus.  As for the exception field be null, its because there is no generated exception (in this case it would be the expected output given the input) - its simply the server status of the operation request. That being said, an exception which includes the status and any additional error map information could be returned. It does seem a bit redundant, tbh.  
            Hide
            jmorris Jeff Morris added a comment -
            Show
            jmorris Jeff Morris added a comment - Current response status from the server:  https://github.com/couchbase/kv_engine/blob/master/include/mcbp/protocol/status.h
            Hide
            david.saadeh David Saadeh added a comment -

            In 4.x, the response used to be Status: TemporaryFailure, Exception: TemporaryLockFailureException. Response from KV Engine is x0086 .... https://github.com/couchbase/kv_engine/blob/master/include/mcbp/protocol/status.h#L100

            In 5.x, the response from the KV Engine is x0009, which is Locked: https://github.com/couchbase/kv_engine/blob/master/include/mcbp/protocol/status.h#L56

            Show
            david.saadeh David Saadeh added a comment - In 4.x, the response used to be Status: TemporaryFailure, Exception: TemporaryLockFailureException. Response from KV Engine is x0086 .... https://github.com/couchbase/kv_engine/blob/master/include/mcbp/protocol/status.h#L100 In 5.x, the response from the KV Engine is x0009, which is Locked: https://github.com/couchbase/kv_engine/blob/master/include/mcbp/protocol/status.h#L56
            Hide
            jmorris Jeff Morris added a comment - - edited

            Yes, indeed it changed from Couchbase Server 4.X to 5.X where it was "fixed". The fix will be to reflect that the server returns the correct status from the server. The bug in the SDK is that a status of "Failure" is returned when the status is anything but "Success". 

            After the patch, for this specific case, for Couchbase Server 4.X and earlier, Temp_Failure will be returned as it always has when a key is locked. The exception field will also be set with a TemporaryLockException. For server 5.X and greater when error map is enabled, the response status will reflect what the server returns which will be "Locked" and the exception field will be set with a TemporaryLockException.

             

            For backwards compatibility the SDK must translate the locked response (cb 5.X) to 4.X temp_fail and then handle it the same. 

             

             

            Show
            jmorris Jeff Morris added a comment - - edited Yes, indeed it changed from Couchbase Server 4.X to 5.X where it was "fixed". The fix will be to reflect that the server returns the correct status from the server. The bug in the SDK is that a status of "Failure" is returned when the status is anything but "Success".   After the patch, for this specific case, for Couchbase Server 4.X and earlier, Temp_Failure will be returned as it always has when a key is locked. The exception field will also be set with a TemporaryLockException. For server 5.X and greater when error map is enabled, the response status will reflect what the server returns which will be "Locked" and the exception field will be set with a TemporaryLockException.   For backwards compatibility the SDK must translate the locked response (cb 5.X) to 4.X temp_fail and then handle it the same.     

              People

              • Assignee:
                jmorris Jeff Morris
                Reporter:
                david.saadeh David Saadeh
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Gerrit Reviews

                    PagerDuty

                    Error rendering 'com.pagerduty.jira-server-plugin:PagerDuty'. Please contact your Jira administrators.