Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alex Khatskevich <avkhatskevich@tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>,
	tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH 2/3] Refactor error reporting system
Date: Thu, 7 Jun 2018 19:13:14 +0300	[thread overview]
Message-ID: <e672436b-2df9-192e-1949-25e23d1b08bf@tarantool.org> (raw)
In-Reply-To: <4ce919e8-117f-fee4-0adb-cd275de93d06@tarantool.org>


>> +-- * name -- Key by which an error code can be retrieved from
>> +--   exprted `code` dictionary.
> 1. exprted -> ?
Fixed.
-- * name -- Key by which an error code can be retrieved from
--   the expoted by the module `code` dictionary.
>> +-- * msg -- Error message which can cantain parameters described
> 2. cantain -> contain. And it must contain. Not can.
fixed
-- * msg -- Error message which can use `args` using
--   `string.format` notation.
> 3. You have found that uuid is always nil for NO_ROUTE_TO_BUCKET.
> Why do you need it in args?
Fixed
> 4. Garbage diff. And this empty line was put here deliberately.
> The comment refers to both box_error and vshard_error.
fixed.
> 5. vstard -> vshard.
Fixed
> 6. arg -> args. Error objects above has args attribute.
fixed
> 7. Please, start a sentence with capital letter and finish with dot.
Ok.
> 8. Why? Either the pcall returns an error and you fail in the assert,
> or simple unprotected string.format() fails. No difference. Just to
> simple string.format().
fixed.
> 9. You do not need local msg here.
fixed
>> +    else
>> +        msg = string.format('%s: %s', format.name, 
>> json.encode(tostring(ret)))
>
> 10. Why do you need encode(tostring())? Ret already has __tostring = 
> json.encode,
> so just do this:
>
>     string.format('%s: %s', format.name, ret)
>
> Diff is below. I did not run the tests. Please, apply and fix the 
> tests if
> needed.
>
> @@ -141,18 +141,14 @@ local function vshard_error(code, ...)
>      for i = 1, #args do
>          ret[args[i]] = select(i, ...)
>      end
> -    local msg
>      if format.msg then
> -        local ok
> -        ok, msg = pcall(string.format, format.msg, ...)
> -        assert(ok)
> +        ret.message = string.format(format.msg, ...)
>      else
> -        msg = string.format('%s: %s', format.name, 
> json.encode(tostring(ret)))
> +        ret.message = string.format('%s: %s', format.name, ret)
>      end
>      ret.type = 'ShardingError'
>      ret.code = code
>      ret.name = format.name
> -    ret.message = msg
>      return ret
>  end
fixed
> 11. Unreachable uuid is always nil as we have found.
Fixed
>>       end
>>         return nil, err
>> @@ -719,14 +717,12 @@ local function bucket_send(bucket_id, destination)
>>       end
>>       local replicaset = M.replicasets[destination]
>>       if replicaset == nil then
>> -        return nil, lerror.vshard(lerror.code.NO_SUCH_REPLICASET,
>> -                                  {replicaset_uuid = destination})
>> +        return nil, lerror.vshard(lerror.code.NO_SUCH_REPLICASET, 
>> destination)
>>       end
>
> 12. NO_SUCH_REPLICASET has two arguments. And you have assert(#args >= 
> args_passed_cnt),
> that has not been failed. It means, that bucket_send has no a test on 
> sending to
> non-existing replicaset. Please,
>     * add a test on bucket_send to non-existing replicaset;
>     * add unit tests on error building.
New diff:

commit af0bfccfbba87a8dcccd71f0dd7d2f8b6c2317cf
Author: AKhatskevich <avkhatskevich@tarantool.org>
Date:   Tue Jun 5 17:05:15 2018 +0300

     Refactor error reporting system

     This commit applies stricter rules on error reporting:
      * error may has only arguments specified in `args` field of
        `error_message_template`
      * arguments may be used in error message
      * arguments are saved in error object by names from `args`

     Extra:
      * testcase for NO_SUCH_REPLICASET added

     Part of #100

diff --git a/test/misc/check_uuid_on_connect.result 
b/test/misc/check_uuid_on_connect.result
index d54c367..6ac2353 100644
--- a/test/misc/check_uuid_on_connect.result
+++ b/test/misc/check_uuid_on_connect.result
@@ -182,10 +182,12 @@ test_run:switch('bad_uuid_router')
  vshard.router.bucket_discovery(1)
  ---
  - null
-- type: ShardingError
-  unreachable_uuid: ac522f65-aa94-4134-9f64-51ee384f1a54
+- bucket_id: 1
    code: 8
-  bucket_id: 1
+  unreachable_uuid: ac522f65-aa94-4134-9f64-51ee384f1a54
+  name: UNREACHABLE_REPLICASET
+  message: There is no active replicas in replicaset 
ac522f65-aa94-4134-9f64-51ee384f1a54
+  type: ShardingError
  ...
  -- Ok to work with correct replicasets.
  vshard.router.bucket_discovery(2).uuid
diff --git a/test/router/reroute_wrong_bucket.result 
b/test/router/reroute_wrong_bucket.result
index eda3bab..f8021f2 100644
--- a/test/router/reroute_wrong_bucket.result
+++ b/test/router/reroute_wrong_bucket.result
@@ -109,9 +109,11 @@ test_run:switch('router_1')
  vshard.router.call(100, 'read', 'customer_lookup', {1}, {timeout = 1})
  ---
  - null
-- type: ShardingError
-  bucket_id: 100
+- bucket_id: 100
    code: 9
+  type: ShardingError
+  message: 'NO_ROUTE_TO_BUCKET: {"bucket_id":100}'
+  name: NO_ROUTE_TO_BUCKET
  ...
  -- Wait reconfiguration durigin timeout, if a replicaset was not
  -- found by bucket.destination from WRONG_BUCKET or
@@ -188,7 +190,8 @@ test_run:grep_log('router_1', 'please update 
configuration')
  ...
  err
  ---
-- {'type': 'ShardingError', 'bucket_id': 100, 'code': 1, 'destination': 
'ac522f65-aa94-4134-9f64-51ee384f1a54'}
+- {'bucket_id': 100, 'code': 1, 'type': 'ShardingError', 'destination': 
'ac522f65-aa94-4134-9f64-51ee384f1a54',
+  'name': 'WRONG_BUCKET', 'message': 'WRONG_BUCKET: 
{"bucket_id":100,"destination":"ac522f65-aa94-4134-9f64-51ee384f1a54"}'}
  ...
  --
  -- Now try again, but update configuration during call(). It must
diff --git a/test/router/router.result b/test/router/router.result
index 5f3ceca..5d99255 100644
--- a/test/router/router.result
+++ b/test/router/router.result
@@ -195,15 +195,19 @@ vshard.router.bucket_count()
  util.check_error(vshard.router.call, 1, 'read', 'echo', {123})
  ---
  - null
-- type: ShardingError
-  bucket_id: 1
+- bucket_id: 1
    code: 9
+  type: ShardingError
+  message: 'NO_ROUTE_TO_BUCKET: {"bucket_id":1}'
+  name: NO_ROUTE_TO_BUCKET
  ...
  replicaset, err = vshard.router.bucket_discovery(1); return err == nil 
or err
  ---
-- type: ShardingError
-  bucket_id: 1
+- bucket_id: 1
    code: 9
+  type: ShardingError
+  message: 'NO_ROUTE_TO_BUCKET: {"bucket_id":1}'
+  name: NO_ROUTE_TO_BUCKET
  ...
  vshard.router.bootstrap()
  ---
@@ -214,8 +218,9 @@ vshard.router.bootstrap()
  ---
  - null
  - type: ShardingError
-  code: 10
+  name: NON_EMPTY
    message: Cluster is already bootstrapped
+  code: 10
  ...
  --
  -- gh-108: negative bucket count on discovery.
@@ -312,9 +317,11 @@ util.check_error(vshard.router.call, 0, 'read', 
'echo', {123})
  ...
  replicaset, err = vshard.router.bucket_discovery(0); return err == nil 
or err
  ---
-- type: ShardingError
-  bucket_id: 0
+- bucket_id: 0
    code: 9
+  type: ShardingError
+  message: 'NO_ROUTE_TO_BUCKET: {"bucket_id":0}'
+  name: NO_ROUTE_TO_BUCKET
  ...
  replicaset, err = vshard.router.bucket_discovery(1); return err == nil 
or err
  ---
@@ -353,7 +360,8 @@ vshard.router.call(1, 'read', 'echo', {123})
  util.check_error(vshard.router.call, 1, 'write', 'echo', {123})
  ---
  - null
-- {'type': 'ShardingError', 'bucket_id': 1, 'code': 7, 'destination': 
'<replicaset_1>'}
+- {'bucket_id': 1, 'code': 7, 'type': 'ShardingError', 'destination': 
'<replicaset_1>',
+  'name': 'TRANSFER_IS_IN_PROGRESS', 'message': 
'TRANSFER_IS_IN_PROGRESS: {"bucket_id":1,"destination":"<replicaset_1>"}'}
  ...
  test_run:cmd('switch storage_2_a')
  ---
@@ -397,10 +405,12 @@ test_run:cmd('stop server storage_2_a')
  util.check_error(vshard.router.call, 1, 'read', 'echo', {123})
  ---
  - null
-- type: ShardingError
-  unreachable_uuid: <replicaset_2>
+- bucket_id: 1
    code: 8
-  bucket_id: 1
+  unreachable_uuid: <replicaset_2>
+  name: UNREACHABLE_REPLICASET
+  message: There is no active replicas in replicaset <replicaset_2>
+  type: ShardingError
  ...
  vshard.router.buckets_info(0, 3)
  ---
@@ -420,9 +430,11 @@ test_run:cmd('start server storage_2_a')
  vshard.router.route(vshard.consts.DEFAULT_BUCKET_COUNT + 100)
  ---
  - null
-- type: ShardingError
-  bucket_id: 3100
+- bucket_id: 3100
    code: 9
+  type: ShardingError
+  message: 'NO_ROUTE_TO_BUCKET: {"bucket_id":3100}'
+  name: NO_ROUTE_TO_BUCKET
  ...
  util.check_error(vshard.router.route, 'asdfg')
  ---
@@ -956,7 +968,8 @@ test_run:cmd("switch router_1")
  util.check_error(vshard.router.call, 1, 'write', 'echo', { 'hello 
world' })
  ---
  - null
-- {'type': 'ShardingError', 'code': 2, 'bucket_id': 1}
+- {'bucket_id': 1, 'code': 2, 'type': 'ShardingError', 'message': 
'NON_MASTER: {"bucket_id":1}',
+  '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 e93ad24..51492a2 100644
--- a/test/storage/storage.result
+++ b/test/storage/storage.result
@@ -594,9 +594,11 @@ vshard.storage.call(1, 'read', 'customer_lookup', {1})
  vshard.storage.call(100500, 'read', 'customer_lookup', {1})
  ---
  - null
-- type: ShardingError
-  bucket_id: 100500
+- bucket_id: 100500
    code: 1
+  type: ShardingError
+  message: 'WRONG_BUCKET: {"bucket_id":100500}'
+  name: WRONG_BUCKET
  ...
  --
  -- Test not existing space in bucket data.
@@ -614,6 +616,17 @@ vshard.storage.bucket_recv(100, 'from_uuid', 
{{1000, {{1}}}})
  --
  -- Bucket transfer
  --
+-- Transfer to unknown replicaset.
+vshard.storage.bucket_send(1, 'unknown uuid')
+---
+- null
+- code: 4
+  type: ShardingError
+  name: NO_SUCH_REPLICASET
+  replicaset_uuid: unknown uuid
+  message: 'NO_SUCH_REPLICASET: {"replicaset_uuid":"unknown uuid"}'
+...
+-- Successful transfer.
  vshard.storage.bucket_send(1, replicaset2_uuid)
  ---
  - true
diff --git a/test/storage/storage.test.lua b/test/storage/storage.test.lua
index 948decc..f4bbf0e 100644
--- a/test/storage/storage.test.lua
+++ b/test/storage/storage.test.lua
@@ -137,6 +137,11 @@ vshard.storage.bucket_recv(100, 'from_uuid', 
{{1000, {{1}}}})
  --
  -- Bucket transfer
  --
+
+-- Transfer to unknown replicaset.
+vshard.storage.bucket_send(1, 'unknown uuid')
+
+-- Successful transfer.
  vshard.storage.bucket_send(1, replicaset2_uuid)
  _ = test_run:cmd("switch storage_2_a")
  vshard.storage.buckets_info()
diff --git a/test/unit/error.result b/test/unit/error.result
index a46fb0d..74571ca 100644
--- a/test/unit/error.result
+++ b/test/unit/error.result
@@ -20,13 +20,13 @@ tostring(box_error)
  ---
  - '{"type":"ClientError","code":78,"message":"Timeout 
exceeded","trace":[{"file":"[C]","line":4294967295}]}'
  ...
-vshard_error = lerror.vshard(lerror.code.UNREACHABLE_MASTER, {uuid = 
'uuid', reason = 'reason'}, 'message 1 2 3')
+vshard_error = lerror.vshard(lerror.code.UNREACHABLE_MASTER, 'uuid', 
'reason')
  ---
  ...
  tostring(vshard_error)
  ---
-- 
'{"reason":"reason","code":11,"type":"ShardingError","uuid":"uuid","message":"message
-  1 2 3"}'
+- 
'{"reason":"reason","code":11,"type":"ShardingError","name":"UNREACHABLE_MASTER","uuid":"uuid","message":"Master
+  of replicaset uuid is unreachable: reason"}'
  ...
  log = require('log')
  ---
@@ -38,6 +38,42 @@ test_run:grep_log('default', 
'"reason":"reason","code":11,"type":"ShardingError"
  ---
  - '"reason":"reason","code":11,"type":"ShardingError"'
  ...
+--
+-- Part of gh-100: check `error.vshard`.
+--
+lerror.vshard(lerror.code.WRONG_BUCKET, 'arg1', 'arg2')
+---
+- bucket_id: arg1
+  code: 1
+  type: ShardingError
+  name: WRONG_BUCKET
+  message: 'WRONG_BUCKET: {"bucket_id":"arg1","destination":"arg2"}'
+  destination: arg2
+...
+-- Pass less args than msg requires.
+ok, res = pcall(lerror.vshard, lerror.code.MISSING_MASTER)
+---
+...
+res:match('bad argument #2.*')
+---
+- 'bad argument #2 to ''format'' (no value)'
+...
+-- Pass more args than `args` field contains.
+ok, res = pcall(lerror.vshard, lerror.code.MISSING_MASTER, 'arg1', 'arg2')
+---
+...
+res:match('Wrong number of.*')
+---
+- Wrong number of arguments passed to MISSING_MASTER error
+...
+-- Pass wrong format code.
+ok, res = pcall(lerror.vshard, 'Wrong format code', 'arg1', 'arg2')
+---
+...
+res:match('Error message format not found.')
+---
+- Error message format not found.
+...
  function raise_lua_err() assert(false) end
  ---
  ...
diff --git a/test/unit/error.test.lua b/test/unit/error.test.lua
index 4b5c20f..7496982 100644
--- a/test/unit/error.test.lua
+++ b/test/unit/error.test.lua
@@ -9,13 +9,27 @@ ok, err = pcall(box.error, box.error.TIMEOUT)
  box_error = lerror.box(err)
  tostring(box_error)

-vshard_error = lerror.vshard(lerror.code.UNREACHABLE_MASTER, {uuid = 
'uuid', reason = 'reason'}, 'message 1 2 3')
+vshard_error = lerror.vshard(lerror.code.UNREACHABLE_MASTER, 'uuid', 
'reason')
  tostring(vshard_error)

  log = require('log')
  log.info('Log error: %s', vshard_error)
  test_run:grep_log('default', 
'"reason":"reason","code":11,"type":"ShardingError"')

+--
+-- Part of gh-100: check `error.vshard`.
+--
+lerror.vshard(lerror.code.WRONG_BUCKET, 'arg1', 'arg2')
+-- Pass less args than msg requires.
+ok, res = pcall(lerror.vshard, lerror.code.MISSING_MASTER)
+res:match('bad argument #2.*')
+-- Pass more args than `args` field contains.
+ok, res = pcall(lerror.vshard, lerror.code.MISSING_MASTER, 'arg1', 'arg2')
+res:match('Wrong number of.*')
+-- Pass wrong format code.
+ok, res = pcall(lerror.vshard, 'Wrong format code', 'arg1', 'arg2')
+res:match('Error message format not found.')
+
  function raise_lua_err() assert(false) end
  ok, err = pcall(raise_lua_err)
  lerror.make(err)
diff --git a/vshard/error.lua b/vshard/error.lua
index 8d26d11..59e4fac 100644
--- a/vshard/error.lua
+++ b/vshard/error.lua
@@ -1,6 +1,111 @@
  local ffi = require('ffi')
  local json = require('json')

+--
+-- Error messages description.
+-- * name -- Key by which an error code can be retrieved from
+--   the expoted by the module `code` dictionary.
+-- * msg -- Error message which can use `args` using
+--   `string.format` notation.
+-- * args -- Names of arguments passed while constructing an
+--   error. After constructed, an error-object contains provided
+--   arguments by the names specified in the field.
+--
+local error_message_template = {
+    [1] = {
+        name = 'WRONG_BUCKET',
+        args = {'bucket_id', 'destination'}
+    },
+    [2] = {
+        name = 'NON_MASTER',
+        args = {'bucket_id', 'destination'}
+    },
+    [3] = {
+        name = 'BUCKET_ALREADY_EXISTS',
+        args = {'bucket_id'}
+    },
+    [4] = {
+        name = 'NO_SUCH_REPLICASET',
+        args = {'replicaset_uuid'}
+    },
+    [5] = {
+        name = 'MOVE_TO_SELF',
+        args = {'replicaset_uuid', 'bucket_id'}
+    },
+    [6] = {
+         name = 'MISSING_MASTER',
+         msg = 'Master is not configured for replicaset %s',
+         args = {'replicaset_uuid'}
+    },
+    [7] = {
+        name = 'TRANSFER_IS_IN_PROGRESS',
+        args = {'bucket_id', 'destination'}
+    },
+    [8] = {
+        name = 'UNREACHABLE_REPLICASET',
+        msg = 'There is no active replicas in replicaset %s',
+        args = {'unreachable_uuid', 'bucket_id'}
+    },
+    [9] = {
+        name = 'NO_ROUTE_TO_BUCKET',
+        args = {'bucket_id'}
+    },
+    [10] = {
+        name = 'NON_EMPTY',
+        msg = 'Cluster is already bootstrapped'
+    },
+    [11] = {
+        name = 'UNREACHABLE_MASTER',
+        msg = 'Master of replicaset %s is unreachable: %s',
+        args = {'uuid', 'reason'}
+    },
+    [12] = {
+        name = 'OUT_OF_SYNC',
+        msg = 'Replica is out of sync'
+    },
+    [13] = {
+        name = 'HIGH_REPLICATION_LAG',
+        msg = 'High replication lag: %f',
+        args = {'lag'}
+    },
+    [14] = {
+        name = 'UNREACHABLE_REPLICA',
+        msg = "Replica %s isn't active",
+        args = {'unreachable_uuid'}
+    },
+    [15] = {
+        name = 'LOW_REDUNDANCY',
+        msg = 'Only one replica is active'
+    },
+    [16] = {
+        name = 'INVALID_REBALANCING',
+        msg = 'Sending and receiving buckets at same time is not allowed'
+    },
+    [17] = {
+        name = 'SUBOPTIMAL_REPLICA',
+        msg = 'A current read replica in replicaset %s is not optimal'
+    },
+    [18] = {
+        name = 'UNKNOWN_BUCKETS',
+        msg = '%d buckets are not discovered',
+        arg = {'not_discovered_cnt'}
+    },
+    [19] = {
+        name = 'REPLICASET_IS_LOCKED',
+        msg = 'Replicaset is locked'
+    }
+}
+
+--
+-- User-visible error_name -> error_number dictionary.
+--
+local error_code = {}
+for code, err in pairs(error_message_template) do
+    assert(type(code) == 'number')
+    assert(error_code[err.name] == nil, "Dublicate error name")
+    error_code[err.name] = code
+end
+
  --
  -- There are 2 error types:
  -- * box_error - it is created on tarantool errors: client error,
@@ -16,14 +121,35 @@ local function box_error(original_error)
      return setmetatable(original_error:unpack(), {__tostring = 
json.encode})
  end

-local function vshard_error(code, args, msg)
-    local ret = setmetatable({type = 'ShardingError', code = code,
-                              message = msg}, {__tostring = json.encode})
-    if args then
-        for k, v in pairs(args) do
-            ret[k] = v
-        end
+--
+-- Construct an vshard error.
+-- @param code Number, one of error_code constants.
+-- @param ... Arguments from `error_message_template` `args`
+--        field. Caller have to pass at least as many arguments
+--        as `msg` field requires.
+-- @retval ShardingError object.
+--
+local function vshard_error(code, ...)
+    local format = error_message_template[code]
+    assert(format, 'Error message format not found.')
+    local args_passed_cnt = select('#', ...)
+    local args = format.args or {}
+    assert(#args >= args_passed_cnt,
+           string.format('Wrong number of arguments passed to %s error',
+                         format.name))
+    local ret = setmetatable({}, {__tostring = json.encode})
+    -- Save error arguments.
+    for i = 1, #args do
+        ret[args[i]] = select(i, ...)
      end
+    if format.msg then
+        ret.message = string.format(format.msg, ...)
+    else
+        ret.message = string.format('%s: %s', format.name, ret)
+    end
+    ret.type = 'ShardingError'
+    ret.code = code
+    ret.name = format.name
      return ret
  end

@@ -45,70 +171,6 @@ local function make_error(e)
      end
  end

-local error_code = {
-    WRONG_BUCKET = 1,
-    NON_MASTER = 2,
-    BUCKET_ALREADY_EXISTS = 3,
-    NO_SUCH_REPLICASET = 4,
-    MOVE_TO_SELF = 5,
-    MISSING_MASTER = 6,
-    TRANSFER_IS_IN_PROGRESS = 7,
-    UNREACHABLE_REPLICASET = 8,
-    NO_ROUTE_TO_BUCKET = 9,
-    NON_EMPTY = 10,
-    UNREACHABLE_MASTER = 11,
-    OUT_OF_SYNC = 12,
-    HIGH_REPLICATION_LAG = 13,
-    UNREACHABLE_REPLICA = 14,
-    LOW_REDUNDANCY = 15,
-    INVALID_REBALANCING = 16,
-    SUBOPTIMAL_REPLICA = 17,
-    UNKNOWN_BUCKETS = 18,
-    REPLICASET_IS_LOCKED = 19,
-}
-
-local error_message_template = {
-    [error_code.MISSING_MASTER] = {
-         name = 'MISSING_MASTER',
-         msg = 'Master is not configured for replicaset %s'
-    },
-    [error_code.UNREACHABLE_MASTER] = {
-        name = 'UNREACHABLE_MASTER',
-        msg = 'Master of replicaset %s is unreachable: %s'
-    },
-    [error_code.OUT_OF_SYNC] = {
-        name = 'OUT_OF_SYNC',
-        msg = 'Replica is out of sync'
-    },
-    [error_code.HIGH_REPLICATION_LAG] = {
-        name = 'HIGH_REPLICATION_LAG',
-        msg = 'High replication lag: %f'
-    },
-    [error_code.UNREACHABLE_REPLICA] = {
-        name = 'UNREACHABLE_REPLICA',
-        msg = "Replica %s isn't active"
-    },
-    [error_code.UNREACHABLE_REPLICASET] = {
-        name = 'UNREACHABLE_REPLICASET',
-        msg = 'There is no active replicas in replicaset %s'
-    },
-    [error_code.LOW_REDUNDANCY] = {
-        name = 'LOW_REDUNDANCY',
-        msg = 'Only one replica is active'
-    },
-    [error_code.INVALID_REBALANCING] = {
-        name = 'INVALID_REBALANCING',
-        msg = 'Sending and receiving buckets at same time is not allowed'
-    },
-    [error_code.SUBOPTIMAL_REPLICA] = {
-        name = 'SUBOPTIMAL_REPLICA',
-        msg = 'A current read replica in replicaset %s is not optimal'
-    },
-    [error_code.UNKNOWN_BUCKETS] = {
-        name = 'UNKNOWN_BUCKETS',
-        msg = '%d buckets are not discovered',
-    }
-}

  local function make_alert(code, ...)
      local format = error_message_template[code]
diff --git a/vshard/replicaset.lua b/vshard/replicaset.lua
index 7fcf2df..99f59aa 100644
--- a/vshard/replicaset.lua
+++ b/vshard/replicaset.lua
@@ -114,8 +114,8 @@ end
  local function replicaset_connect_master(replicaset)
      local master = replicaset.master
      if master == nil then
-        return nil, lerror.vshard(lerror.code.MASTER_IS_MISSING,
-                                  {replicaset_uuid = replicaset.uuid})
+        return nil, lerror.vshard(lerror.code.MISSING_MASTER,
+                                  replicaset.uuid)
      end
      return replicaset_connect_to_replica(replicaset, master)
  end
diff --git a/vshard/router/init.lua b/vshard/router/init.lua
index 42e9c86..21093e5 100644
--- a/vshard/router/init.lua
+++ b/vshard/router/init.lua
@@ -85,8 +85,7 @@ local function bucket_discovery(bucket_id)
          if last_err.type == 'ClientError' and
             last_err.code == box.error.NO_CONNECTION then
              err = lerror.vshard(lerror.code.UNREACHABLE_REPLICASET,
-                                {bucket_id = bucket_id,
-                                 unreachable_uuid = unreachable_uuid})
+                                unreachable_uuid, bucket_id)
          else
              err = lerror.make(last_err)
          end
@@ -97,9 +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 = bucket_id,
-                             unreachable_uuid = unreachable_uuid})
+        err = lerror.vshard(lerror.code.NO_ROUTE_TO_BUCKET, bucket_id)
      end

      return nil, err
@@ -519,8 +516,7 @@ local function cluster_bootstrap()
              return nil, err
          end
          if count > 0 then
-            return nil, lerror.vshard(lerror.code.NON_EMPTY, {},
-                                      'Cluster is already bootstrapped')
+            return nil, lerror.vshard(lerror.code.NON_EMPTY)
          end
      end
      lreplicaset.calculate_etalon_balance(M.replicasets, 
M.total_bucket_count)
diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
index d5e7b57..2d38ff9 100644
--- a/vshard/storage/init.lua
+++ b/vshard/storage/init.lua
@@ -452,8 +452,7 @@ local function bucket_check_state(bucket_id, mode)
             mode == 'write' and bucket_is_writable(bucket))
  ::finish::
      return bucket, errcode and
-           lerror.vshard(errcode, {bucket_id = bucket_id,
-                                   destination = bucket and 
bucket.destination})
+           lerror.vshard(errcode, bucket_id, bucket and bucket.destination)
  end

  --
@@ -514,8 +513,7 @@ local function bucket_recv(bucket_id, from, data)

      local bucket = box.space._bucket:get({bucket_id})
      if bucket ~= nil then
-        return nil, lerror.vshard(lerror.code.BUCKET_ALREADY_EXISTS,
-                                  {bucket_id = bucket_id})
+        return nil, lerror.vshard(lerror.code.BUCKET_ALREADY_EXISTS, 
bucket_id)
      end

      box.begin()
@@ -719,14 +717,12 @@ local function bucket_send(bucket_id, destination)
      end
      local replicaset = M.replicasets[destination]
      if replicaset == nil then
-        return nil, lerror.vshard(lerror.code.NO_SUCH_REPLICASET,
-                                  {replicaset_uuid = destination})
+        return nil, lerror.vshard(lerror.code.NO_SUCH_REPLICASET, 
destination)
      end

      if destination == box.info.cluster.uuid then
-        return nil, lerror.vshard(lerror.code.MOVE_TO_SELF,
-                                  {bucket_id = bucket_id,
-                                   replicaset_uuid = replicaset_uuid})
+        return nil, lerror.vshard(lerror.code.MOVE_TO_SELF, 
replicaset_uuid,
+                                  bucket_id)
      end

      local data = bucket_collect_internal(bucket_id)
@@ -1260,8 +1256,7 @@ end
  --
  local function rebalancer_apply_routes(routes)
      if is_this_replicaset_locked() then
-        return false, lerror.vshard(lerror.code.REPLICASET_IS_LOCKED, nil,
-                                    "Replicaset is locked");
+        return false, lerror.vshard(lerror.code.REPLICASET_IS_LOCKED);
      end
      assert(not rebalancing_is_in_progress())
      -- Can not apply routes here because of gh-946 in tarantool

  reply	other threads:[~2018-06-07 16:13 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-06 15:57 [tarantool-patches] [PATCH 0/3][vshard] Add error messages AKhatskevich
2018-06-06 15:57 ` [tarantool-patches] [PATCH 1/3] Fix test on bucket_transferring error AKhatskevich
2018-06-06 15:57 ` [tarantool-patches] [PATCH 2/3] Refactor error reporting system AKhatskevich
2018-06-07 12:22   ` [tarantool-patches] " Vladislav Shpilevoy
2018-06-07 16:13     ` Alex Khatskevich [this message]
2018-06-06 15:57 ` [tarantool-patches] [PATCH 3/3] Add error messages AKhatskevich
2018-06-07 12:22   ` [tarantool-patches] " Vladislav Shpilevoy
2018-06-07 16:12     ` Alex Khatskevich
2018-06-07 18:44       ` Vladislav Shpilevoy
2018-06-07 12:22 ` [tarantool-patches] Re: [PATCH 0/3][vshard] " Vladislav Shpilevoy
2018-06-07 13:16   ` Alex Khatskevich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e672436b-2df9-192e-1949-25e23d1b08bf@tarantool.org \
    --to=avkhatskevich@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='[tarantool-patches] Re: [PATCH 2/3] Refactor error reporting system' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox