[tarantool-patches] [PATCH 3/3] Add error messages

[AKhatskevich <avkhatskevich@tarantool.org>]

Add error message to each `error_message_template`.
This change required to change arguments of some errors to make
them more informative.

Closes #100
---
 test/router/reroute_wrong_bucket.result |  7 ++++---
 test/router/router.result               | 16 +++++++++-------
 test/storage/storage.result             |  3 ++-
 vshard/error.lua                        | 15 +++++++++++----
 vshard/router/init.lua                  |  3 +--
 vshard/storage/init.lua                 | 28 ++++++++++++++++++++--------
 6 files changed, 47 insertions(+), 25 deletions(-)

diff --git a/test/router/reroute_wrong_bucket.result b/test/router/reroute_wrong_bucket.result
index c7b980c..75a1c5a 100644
--- a/test/router/reroute_wrong_bucket.result
+++ b/test/router/reroute_wrong_bucket.result
@@ -113,7 +113,7 @@ vshard.router.call(100, 'read', 'customer_lookup', {1}, {timeout = 1})
   code: 9
   type: ShardingError
   name: NO_ROUTE_TO_BUCKET
-  message: 'NO_ROUTE_TO_BUCKET: "{\"bucket_id\":100}"'
+  message: Bucket 100 cannot be found. Is rebalancing in progress?
 ...
 -- Wait reconfiguration durigin timeout, if a replicaset was not
 -- found by bucket.destination from WRONG_BUCKET or
@@ -190,8 +190,9 @@ test_run:grep_log('router_1', 'please update configuration')
 ...
 err
 ---
-- {'bucket_id': 100, 'code': 1, 'type': 'ShardingError', 'destination': 'ac522f65-aa94-4134-9f64-51ee384f1a54',
-  'message': 'WRONG_BUCKET: "{\"bucket_id\":100,\"destination\":\"ac522f65-aa94-4134-9f64-51ee384f1a54\"}"',
+- {'bucket_id': 100, 'reason': 'bucket moving to ac522f65-aa94-4134-9f64-51ee384f1a54',
+  'code': 1, 'type': 'ShardingError', 'destination': 'ac522f65-aa94-4134-9f64-51ee384f1a54',
+  'message': 'Cannot perform action with bucket 100, reason: bucket moving to ac522f65-aa94-4134-9f64-51ee384f1a54',
   'name': 'WRONG_BUCKET'}
 ...
 --
diff --git a/test/router/router.result b/test/router/router.result
index 6955b59..dab71f2 100644
--- a/test/router/router.result
+++ b/test/router/router.result
@@ -199,7 +199,7 @@ util.check_error(vshard.router.call, 1, 'read', 'echo', {123})
   code: 9
   type: ShardingError
   name: NO_ROUTE_TO_BUCKET
-  message: 'NO_ROUTE_TO_BUCKET: "{\"bucket_id\":1}"'
+  message: Bucket 1 cannot be found. Is rebalancing in progress?
 ...
 replicaset, err = vshard.router.bucket_discovery(1); return err == nil or err
 ---
@@ -207,7 +207,7 @@ replicaset, err = vshard.router.bucket_discovery(1); return err == nil or err
   code: 9
   type: ShardingError
   name: NO_ROUTE_TO_BUCKET
-  message: 'NO_ROUTE_TO_BUCKET: "{\"bucket_id\":1}"'
+  message: Bucket 1 cannot be found. Is rebalancing in progress?
 ...
 vshard.router.bootstrap()
 ---
@@ -321,7 +321,7 @@ replicaset, err = vshard.router.bucket_discovery(0); return err == nil or err
   code: 9
   type: ShardingError
   name: NO_ROUTE_TO_BUCKET
-  message: 'NO_ROUTE_TO_BUCKET: "{\"bucket_id\":0}"'
+  message: Bucket 0 cannot be found. Is rebalancing in progress?
 ...
 replicaset, err = vshard.router.bucket_discovery(1); return err == nil or err
 ---
@@ -361,7 +361,7 @@ util.check_error(vshard.router.call, 1, 'write', 'echo', {123})
 ---
 - null
 - {'bucket_id': 1, 'code': 7, 'type': 'ShardingError', 'destination': '<replicaset_1>',
-  'message': 'TRANSFER_IS_IN_PROGRESS: "{\"bucket_id\":1,\"destination\":\"<replicaset_1>\"}"',
+  'message': 'Bucket 1 is transferring to replicaset <replicaset_1>',
   'name': 'TRANSFER_IS_IN_PROGRESS'}
 ...
 test_run:cmd('switch storage_2_a')
@@ -435,7 +435,7 @@ vshard.router.route(vshard.consts.DEFAULT_BUCKET_COUNT + 100)
   code: 9
   type: ShardingError
   name: NO_ROUTE_TO_BUCKET
-  message: 'NO_ROUTE_TO_BUCKET: "{\"bucket_id\":3100}"'
+  message: Bucket 3100 cannot be found. Is rebalancing in progress?
 ...
 util.check_error(vshard.router.route, 'asdfg')
 ---
@@ -969,8 +969,10 @@ test_run:cmd("switch router_1")
 util.check_error(vshard.router.call, 1, 'write', 'echo', { 'hello world' })
 ---
 - null
-- {'bucket_id': 1, 'code': 2, 'type': 'ShardingError', 'name': 'NON_MASTER', 'message': 'NON_MASTER:
-    "{\"bucket_id\":1}"'}
+- {'code': 2, 'replica_uuid': '1e02ae8a-afc0-4e91-ba34-843a356b8ed7', 'type': 'ShardingError',
+  'replicaset_uuid': '<replicaset_2>', 'message': 'Replica 1e02ae8a-afc0-4e91-ba34-843a356b8ed7
+    is not a master for replicaset <replicaset_2> anymore',
+  'name': 'NON_MASTER'}
 ...
 -- Reconfigure router and test that the WRITE request does work
 vshard.router.cfg(cfg)
diff --git a/test/storage/storage.result b/test/storage/storage.result
index aee9477..1258c9b 100644
--- a/test/storage/storage.result
+++ b/test/storage/storage.result
@@ -595,10 +595,11 @@ vshard.storage.call(100500, 'read', 'customer_lookup', {1})
 ---
 - null
 - bucket_id: 100500
+  reason: Not found
   code: 1
   type: ShardingError
   name: WRONG_BUCKET
-  message: 'WRONG_BUCKET: "{\"bucket_id\":100500}"'
+  message: 'Cannot perform action with bucket 100500, reason: Not found'
 ...
 --
 -- Test not existing space in bucket data.
diff --git a/vshard/error.lua b/vshard/error.lua
index 1271d16..8785695 100644
--- a/vshard/error.lua
+++ b/vshard/error.lua
@@ -14,23 +14,28 @@ local json = require('json')
 local error_message_template = {
     [1] = {
         name = 'WRONG_BUCKET',
-        args = {'bucket_id', 'destination'}
+        msg = 'Cannot perform action with bucket %d, reason: %s',
+        args = {'bucket_id', 'reason', 'destination'}
     },
     [2] = {
         name = 'NON_MASTER',
-        args = {'bucket_id', 'destination'}
+        msg = 'Replica %s is not a master for replicaset %s anymore',
+        args = {'replica_uuid', 'replicaset_uuid'}
     },
     [3] = {
         name = 'BUCKET_ALREADY_EXISTS',
+        msg = 'Bucket %d already exists',
         args = {'bucket_id'}
     },
     [4] = {
         name = 'NO_SUCH_REPLICASET',
+        msg = 'Replicaset %s not found',
         args = {'replicaset_uuid'}
     },
     [5] = {
         name = 'MOVE_TO_SELF',
-        args = {'replicaset_uuid', 'bucket_id'}
+        msg = 'Cannot move: bucket %d is already on replicaset %s',
+        args = {'bucket_id', 'replicaset_uuid'}
     },
     [6] = {
          name = 'MISSING_MASTER',
@@ -39,6 +44,7 @@ local error_message_template = {
     },
     [7] = {
         name = 'TRANSFER_IS_IN_PROGRESS',
+        msg = 'Bucket %d is transferring to replicaset %s',
         args = {'bucket_id', 'destination'}
     },
     [8] = {
@@ -48,7 +54,8 @@ local error_message_template = {
     },
     [9] = {
         name = 'NO_ROUTE_TO_BUCKET',
-        args = {'bucket_id', 'unreachable_uuid'}
+        msg = 'Bucket %d cannot be found. Is rebalancing in progress?',
+        args = {'bucket_id'}
     },
     [10] = {
         name = 'NON_EMPTY',
diff --git a/vshard/router/init.lua b/vshard/router/init.lua
index 95ee8b3..21093e5 100644
--- a/vshard/router/init.lua
+++ b/vshard/router/init.lua
@@ -96,8 +96,7 @@ local function bucket_discovery(bucket_id)
         -- bucket was found to be RECEIVING on one replicaset, and
         -- was not found on other replicasets (it was sent during
         -- discovery).
-        err = lerror.vshard(lerror.code.NO_ROUTE_TO_BUCKET, bucket_id,
-                            unreachable_uuid)
+        err = lerror.vshard(lerror.code.NO_ROUTE_TO_BUCKET, bucket_id)
     end
 
     return nil, err
diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
index 2d38ff9..cd02657 100644
--- a/vshard/storage/init.lua
+++ b/vshard/storage/init.lua
@@ -428,31 +428,43 @@ local function bucket_check_state(bucket_id, mode)
     assert(mode == 'read' or mode == 'write')
     local bucket = box.space._bucket:get({bucket_id})
     local errcode = nil
+    local reason = nil
     if not bucket then
         errcode = lerror.code.WRONG_BUCKET
-        goto finish
+        reason = 'Not found'
+        goto wrong_bucket
     elseif mode == 'read' then
         if not bucket_is_readable(bucket) then
             errcode = lerror.code.WRONG_BUCKET
-            goto finish
+            reason = 'read prohibited'
+            goto wrong_bucket
         end
     elseif not bucket_is_writable(bucket) then
         if bucket_is_transfer_in_progress(bucket) then
             errcode = lerror.code.TRANSFER_IS_IN_PROGRESS
+            return bucket, errcode and
+                   lerror.vshard(errcode, bucket_id, bucket.destination)
         else
             errcode = lerror.code.WRONG_BUCKET
+            reason = 'write prohibited'
+            goto wrong_bucket
         end
-        goto finish
     elseif M.this_replicaset.master ~= M.this_replica then
         errcode = lerror.code.NON_MASTER
-        goto finish
+        return bucket, errcode and
+               lerror.vshard(errcode, M.this_replica.uuid,
+                             M.this_replicaset.uuid)
     end
     assert(not errcode)
     assert(mode == 'read' and bucket_is_readable(bucket) or
            mode == 'write' and bucket_is_writable(bucket))
-::finish::
+::wrong_bucket::
+    if bucket and bucket.destination then
+        reason = "bucket moving to " .. bucket.destination
+    end
     return bucket, errcode and
-           lerror.vshard(errcode, bucket_id, bucket and bucket.destination)
+           lerror.vshard(errcode, bucket_id, reason,
+                         bucket and bucket.destination)
 end
 
 --
@@ -721,8 +733,8 @@ local function bucket_send(bucket_id, destination)
     end
 
     if destination == box.info.cluster.uuid then
-        return nil, lerror.vshard(lerror.code.MOVE_TO_SELF, replicaset_uuid,
-                                  bucket_id)
+        return nil, lerror.vshard(lerror.code.MOVE_TO_SELF, bucket_id,
+                                  replicaset_uuid)
     end
 
     local data = bucket_collect_internal(bucket_id)
-- 
2.14.1