Re: [Tarantool-patches] [PATCH 9/9] box: enrich ER_READONLY with new details

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Thanks for the review!

>> diff --git a/changelogs/unreleased/gh-5568-readonly-reason.md b/changelogs/unreleased/gh-5568-readonly-reason.md
>> new file mode 100644
>> index 000000000..f90b53bdf
>> --- /dev/null
>> +++ b/changelogs/unreleased/gh-5568-readonly-reason.md
>> @@ -0,0 +1,24 @@
>> +## feature/core
>> +
>> +* Error objects with the code `box.error.READONLY` now have additional fields
>> +  explaining why the error happened (gh-5568).
>> +
>> +  For instance, if the error was due to `box.cfg{read_only = true}`, then the
>> +  error will have fields:
>> +  ```
>> +  {
>> +    reason = 'state',
>> +    state = 'read_only'
>> +  }
>> +  ```
>> +  Or if it was due to not being a leader with `box.cfg{election_mode = ...}` set
>> +  not to `off`, then:
>> +  ```
>> +  {
>> +    state: <box.info.election.state of this instance>,
>> +    leader_id: <box.info.id of the leader>,
>> +    leader_uuid: <box.info.uuid of the leader>,
>> +    reason: election,
>> +    term: <box.info.election.term of this instance>
>> +  }
>> +  ```
> 
> 1. Let's duplicate the additional error information in the error message.
> 
>    If it's hard to log every field, let's at least encode reason and state,
>    so that the user knows what's wrong by simply looking at the error message.

I added this in a new commit on top of the branch.

>> \ No newline at end of file

I forgot a new line, added one:

====================
@@ -21,4 +21,4 @@
     reason: election,
     term: <box.info.election.term of this instance>
   }
-  ```
\ No newline at end of file
+  ```
====================

>> diff --git a/test/replication-luatest/gh_5568_read_only_reason_test.lua b/test/replication-luatest/gh_5568_read_only_reason_test.lua
>> new file mode 100644
>> index 000000000..d4f7ff957
>> --- /dev/null
>> +++ b/test/replication-luatest/gh_5568_read_only_reason_test.lua

<...>

>> +
>> +--
>> +-- Read-only because does not own the limbo. Knows the owner.
>> +--
>> +g.test_read_only_reason_synchro = function(g)
>> +    g.master:exec(function()
>> +        box.schema.create_space('test', {is_sync = true}):create_index('pk')
> 
> 2. The space creation isn't necessary for this test.

Indeed, dropped:

====================
@@ -172,7 +172,6 @@ end
 --
 g.test_read_only_reason_synchro = function(g)
     g.master:exec(function()
-        box.schema.create_space('test', {is_sync = true}):create_index('pk')
         box.ctl.promote()
     end)
 
@@ -198,7 +197,6 @@ g.test_read_only_reason_synchro = function(g)
 
     -- Cleanup.
     g.master:exec(function()
-        box.space.test:drop()
         box.ctl.demote()
     end)
 end
====================

<...>

>> +
>> +    local ok, err = g.replica:exec(function()
>> +        local ok, err = pcall(box.schema.create_space, 'test')
>> +        return ok, err:unpack()
>> +    end)
>> +    t.assert(not ok, 'fail ddl')
>> +    t.assert(err.term, 'has term')
>> +    t.assert(not err.leader_uuid, 'has no leader uuid')
>> +    t.assert_covers(err, {
>> +        reason = 'election',
>> +        state = 'follower',
>> +        leader_id = leader_id,
>> +        code = box.error.READONLY,
>> +        type = 'ClientError'
>> +    }, 'reason is election, has leader but no uuid')
>> +end
>> +
>> +--
>> +-- Read-only because does not own the limbo. Knows the owner, but now its UUID.
>> +--
> 
> 3. Typo: now -> not.

Done:

====================
 --
--- Read-only because does not own the limbo. Knows the owner, but now its UUID.
+-- Read-only because does not own the limbo. Knows the owner, but not its UUID.
 --
 g.test_read_only_reason_synchro_no_uuid = function(g)
====================