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

    • Bug
    • Status: Resolved
    • Blocker
    • Resolution: Fixed
    • None
    • 2.5.12
    • library
    • None
    • 1

    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

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

          Activity

            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.

             

            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.  
            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

            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

            david.saadeh David Saadeh (Inactive) 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
            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. 

             

             

            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

              jmorris Jeff Morris
              david.saadeh David Saadeh (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews

                  PagerDuty