* [Tarantool-patches] [PATCH 1/3] access: fix invalid error type for not found user
2019-11-05 14:59 [Tarantool-patches] [PATCH 0/3] Replication status errno Vladislav Shpilevoy
@ 2019-11-05 14:59 ` Vladislav Shpilevoy
2019-11-05 18:15 ` Konstantin Osipov
2019-11-05 14:59 ` [Tarantool-patches] [PATCH 2/3] error: move errno into an error object Vladislav Shpilevoy
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Vladislav Shpilevoy @ 2019-11-05 14:59 UTC (permalink / raw)
To: tarantool-patches, kostja.osipov
Box.session.su() raised 'SystemError' when a user was not found
due to a too long user name. That was obviously wrong, because
SystemError is always something related to libraries (standard,
curl, etc), and it has an errno code.
Now a ClientError is raised.
---
src/box/schema.cc | 5 ++---
src/box/user.cc | 5 +++--
test/box/access.result | 3 ++-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/box/schema.cc b/src/box/schema.cc
index 9767207e0..fab7544f2 100644
--- a/src/box/schema.cc
+++ b/src/box/schema.cc
@@ -311,9 +311,8 @@ schema_find_id(uint32_t system_space_id, uint32_t index_id,
const char *name, uint32_t len, uint32_t *object_id)
{
if (len > BOX_NAME_MAX) {
- diag_set(SystemError,
- "name length %d is greater than BOX_NAME_MAX", len);
- return -1;
+ *object_id = BOX_ID_NIL;
+ return 0;
}
struct space *space = space_cache_find(system_space_id);
if (space == NULL)
diff --git a/src/box/user.cc b/src/box/user.cc
index 03b4b2e3b..15c40f90d 100644
--- a/src/box/user.cc
+++ b/src/box/user.cc
@@ -521,8 +521,9 @@ user_find_by_name(const char *name, uint32_t len)
if (schema_find_id(BOX_USER_ID, 2, name, len, &uid) != 0)
return NULL;
struct user *user = user_by_id(uid);
- if (user == NULL || user->def->type != SC_USER) {
- diag_set(ClientError, ER_NO_SUCH_USER, tt_cstr(name, len));
+ if (user == NULL || user->def->type != SC_USER || uid == BOX_ID_NIL) {
+ diag_set(ClientError, ER_NO_SUCH_USER,
+ tt_cstr(name, MIN(BOX_INVALID_NAME_MAX, len)));
return NULL;
}
return user;
diff --git a/test/box/access.result b/test/box/access.result
index dc339038d..05e2f4147 100644
--- a/test/box/access.result
+++ b/test/box/access.result
@@ -248,7 +248,8 @@ session.su('does not exist')
-- allowed name.
session.su(string.rep('a', 66000))
---
-- error: name length 66000 is greater than BOX_NAME_MAX
+- error: User 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' is
+ not found
...
--------------------------------------------------------------------------------
-- Check if identifiers obey the common constraints
--
2.21.0 (Apple Git-122.2)
^ permalink raw reply [flat|nested] 11+ messages in thread* [Tarantool-patches] [PATCH 2/3] error: move errno into an error object
2019-11-05 14:59 [Tarantool-patches] [PATCH 0/3] Replication status errno Vladislav Shpilevoy
2019-11-05 14:59 ` [Tarantool-patches] [PATCH 1/3] access: fix invalid error type for not found user Vladislav Shpilevoy
@ 2019-11-05 14:59 ` Vladislav Shpilevoy
2019-11-05 18:18 ` Konstantin Osipov
2019-11-05 14:59 ` [Tarantool-patches] [PATCH 3/3] replication: show errno in replication info Vladislav Shpilevoy
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Vladislav Shpilevoy @ 2019-11-05 14:59 UTC (permalink / raw)
To: tarantool-patches, kostja.osipov
The only error type having an errno as a part of it was
SystemError (and its descendants SocketError, TimedOut, OOM, ...).
That was used in logs (SystemError::log() method), and exposed to
Lua (if type was SystemError, an error object had 'errno' field).
But actually errno might be useful not only there. For example,
box.info.replication exposes the latest error message of
applier/relay as 'message' field of 'upstream/downstream' fields,
lacking errno description.
Before the patch it was impossible to obtain an errno code from C,
because it was necessary to check whether an error has SystemError
type, cast to SystemError class, and call SystemError::get_errno()
method.
Now errno is available as a part of struct error object (available
from C), and is not 0 for system errors.
Part of #4402
---
src/box/applier.cc | 2 +-
src/lib/core/diag.c | 1 +
src/lib/core/diag.h | 6 ++++++
src/lib/core/exception.cc | 22 ++++++++--------------
src/lib/core/exception.h | 5 -----
src/lua/error.lua | 10 ++++++++++
test/box/misc.result | 19 +++++++++++++++++++
test/box/misc.test.lua | 10 ++++++++++
8 files changed, 55 insertions(+), 20 deletions(-)
diff --git a/src/box/applier.cc b/src/box/applier.cc
index a04d13564..a287f4219 100644
--- a/src/box/applier.cc
+++ b/src/box/applier.cc
@@ -181,7 +181,7 @@ applier_writer_f(va_list ap)
* the master closed its end - we would only
* spam the log - so exit immediately.
*/
- if (e->get_errno() == EPIPE)
+ if (e->syserror == EPIPE)
break;
/*
* Do not exit, if there is a network error,
diff --git a/src/lib/core/diag.c b/src/lib/core/diag.c
index 248277e74..168f08225 100644
--- a/src/lib/core/diag.c
+++ b/src/lib/core/diag.c
@@ -44,6 +44,7 @@ error_create(struct error *e,
e->log = log;
e->type = type;
e->refs = 0;
+ e->syserror = 0;
if (file != NULL) {
snprintf(e->file, sizeof(e->file), "%s", file);
e->line = line;
diff --git a/src/lib/core/diag.h b/src/lib/core/diag.h
index fd3831e66..2f106d33f 100644
--- a/src/lib/core/diag.h
+++ b/src/lib/core/diag.h
@@ -72,6 +72,12 @@ struct error {
error_f log;
const struct type_info *type;
int refs;
+ /**
+ * Errno at the moment of the error
+ * creation. If the error is not related
+ * to the standard library, then it is 0.
+ */
+ int syserror;
/** Line number. */
unsigned line;
/* Source file name. */
diff --git a/src/lib/core/exception.cc b/src/lib/core/exception.cc
index a6999af43..6e725a36c 100644
--- a/src/lib/core/exception.cc
+++ b/src/lib/core/exception.cc
@@ -129,27 +129,21 @@ Exception::log() const
say_file_line(S_ERROR, file, line, errmsg, "%s", type->name);
}
-static const struct method_info systemerror_methods[] = {
- make_method(&type_SystemError, "errno", &SystemError::get_errno),
- METHODS_SENTINEL
-};
-
const struct type_info type_SystemError =
- make_type("SystemError", &type_Exception, systemerror_methods);
+ make_type("SystemError", &type_Exception);
SystemError::SystemError(const struct type_info *type,
const char *file, unsigned line)
- :Exception(type, file, line),
- m_errno(errno)
+ :Exception(type, file, line)
{
- /* nothing */
+ syserror = errno;
}
SystemError::SystemError(const char *file, unsigned line,
const char *format, ...)
- : Exception(&type_SystemError, file, line),
- m_errno(errno)
+ : Exception(&type_SystemError, file, line)
{
+ syserror = errno;
va_list ap;
va_start(ap, format);
error_vformat_msg(this, format, ap);
@@ -159,7 +153,7 @@ SystemError::SystemError(const char *file, unsigned line,
void
SystemError::log() const
{
- say_file_line(S_SYSERROR, file, line, strerror(m_errno),
+ say_file_line(S_SYSERROR, file, line, strerror(syserror),
"SystemError %s", errmsg);
}
@@ -192,7 +186,7 @@ OutOfMemory::OutOfMemory(const char *file, unsigned line,
const char *object)
: SystemError(&type_OutOfMemory, file, line)
{
- m_errno = ENOMEM;
+ syserror = ENOMEM;
error_format_msg(this, "Failed to allocate %u bytes in %s for %s",
(unsigned) amount, allocator, object);
}
@@ -203,7 +197,7 @@ const struct type_info type_TimedOut =
TimedOut::TimedOut(const char *file, unsigned line)
: SystemError(&type_TimedOut, file, line)
{
- m_errno = ETIMEDOUT;
+ syserror = ETIMEDOUT;
error_format_msg(this, "timed out");
}
diff --git a/src/lib/core/exception.h b/src/lib/core/exception.h
index a29281427..d6154eb32 100644
--- a/src/lib/core/exception.h
+++ b/src/lib/core/exception.h
@@ -86,17 +86,12 @@ class SystemError: public Exception {
public:
virtual void raise() { throw this; }
- int get_errno() const { return m_errno; }
-
virtual void log() const;
SystemError(const char *file, unsigned line,
const char *format, ...);
protected:
SystemError(const struct type_info *type, const char *file, unsigned line);
-protected:
- /* system errno */
- int m_errno;
};
extern const struct type_info type_SocketError;
diff --git a/src/lua/error.lua b/src/lua/error.lua
index 28fc0377d..575594273 100644
--- a/src/lua/error.lua
+++ b/src/lua/error.lua
@@ -17,6 +17,7 @@ struct error {
error_f _log;
const struct type_info *_type;
int _refs;
+ int _syserror;
/** Line number. */
unsigned _line;
/* Source file name. */
@@ -86,10 +87,19 @@ local function error_trace(err)
}
end
+local function error_errno(err)
+ local e = err._syserror
+ if e == 0 then
+ return nil
+ end
+ return e
+end
+
local error_fields = {
["type"] = error_type;
["message"] = error_message;
["trace"] = error_trace;
+ ["errno"] = error_errno;
}
local function error_unpack(err)
diff --git a/test/box/misc.result b/test/box/misc.result
index b2930515b..d2a20307a 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -196,6 +196,25 @@ box.error.new()
---
- error: 'Usage: box.error.new(code, args)'
...
+--
+-- System errors expose errno as a field.
+--
+_, err = require('fio').open('not_existing_file')
+---
+...
+type(err.errno)
+---
+- number
+...
+-- Errors not related to the standard library do
+-- not expose errno.
+err = box.error.new(box.error.PROC_LUA, "errno")
+---
+...
+type(err.errno)
+---
+- nil
+...
----------------
-- # box.stat
----------------
diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua
index cc223b2ef..c3e992a48 100644
--- a/test/box/misc.test.lua
+++ b/test/box/misc.test.lua
@@ -59,6 +59,16 @@ e = box.error.new(box.error.CREATE_SPACE, "space", "error")
e
box.error.new()
+--
+-- System errors expose errno as a field.
+--
+_, err = require('fio').open('not_existing_file')
+type(err.errno)
+-- Errors not related to the standard library do
+-- not expose errno.
+err = box.error.new(box.error.PROC_LUA, "errno")
+type(err.errno)
+
----------------
-- # box.stat
----------------
--
2.21.0 (Apple Git-122.2)
^ permalink raw reply [flat|nested] 11+ messages in thread* [Tarantool-patches] [PATCH 3/3] replication: show errno in replication info
2019-11-05 14:59 [Tarantool-patches] [PATCH 0/3] Replication status errno Vladislav Shpilevoy
2019-11-05 14:59 ` [Tarantool-patches] [PATCH 1/3] access: fix invalid error type for not found user Vladislav Shpilevoy
2019-11-05 14:59 ` [Tarantool-patches] [PATCH 2/3] error: move errno into an error object Vladislav Shpilevoy
@ 2019-11-05 14:59 ` Vladislav Shpilevoy
2019-11-05 18:18 ` Konstantin Osipov
2019-11-05 18:13 ` [Tarantool-patches] [PATCH 0/3] Replication status errno Konstantin Osipov
2019-11-21 17:38 ` Kirill Yukhin
4 siblings, 1 reply; 11+ messages in thread
From: Vladislav Shpilevoy @ 2019-11-05 14:59 UTC (permalink / raw)
To: tarantool-patches, kostja.osipov
Box.info.replication shows applier/relay's latest error message.
But it didn't include errno description for system errors, even
though it was included in the logs. Now box.info shows the errno
description as well, when possible.
Closes #4402
---
src/box/lua/info.c | 29 +++++----
test/replication/gh-4402-info-errno.result | 63 ++++++++++++++++++++
test/replication/gh-4402-info-errno.test.lua | 25 ++++++++
test/replication/suite.cfg | 1 +
4 files changed, 108 insertions(+), 10 deletions(-)
create mode 100644 test/replication/gh-4402-info-errno.result
create mode 100644 test/replication/gh-4402-info-errno.test.lua
diff --git a/src/box/lua/info.c b/src/box/lua/info.c
index 55382fd77..b4475959b 100644
--- a/src/box/lua/info.c
+++ b/src/box/lua/info.c
@@ -50,6 +50,7 @@
#include "box/box.h"
#include "lua/utils.h"
#include "fiber.h"
+#include "tt_static.h"
static void
lbox_pushvclock(struct lua_State *L, const struct vclock *vclock)
@@ -65,6 +66,20 @@ lbox_pushvclock(struct lua_State *L, const struct vclock *vclock)
luaL_setmaphint(L, -1); /* compact flow */
}
+static inline void
+lbox_push_replication_error_message(struct lua_State *L, struct error *e,
+ int idx)
+{
+ lua_pushstring(L, "message");
+ lua_pushstring(L, e->errmsg);
+ lua_settable(L, idx - 2);
+ if (e->syserror == 0)
+ return;
+ lua_pushstring(L, "system_message");
+ lua_pushstring(L, strerror(e->syserror));
+ lua_settable(L, idx - 2);
+}
+
static void
lbox_pushapplier(lua_State *L, struct applier *applier)
{
@@ -103,11 +118,8 @@ lbox_pushapplier(lua_State *L, struct applier *applier)
lua_settable(L, -3);
struct error *e = diag_last_error(&applier->reader->diag);
- if (e != NULL) {
- lua_pushstring(L, "message");
- lua_pushstring(L, e->errmsg);
- lua_settable(L, -3);
- }
+ if (e != NULL)
+ lbox_push_replication_error_message(L, e, -1);
}
}
@@ -135,11 +147,8 @@ lbox_pushrelay(lua_State *L, struct relay *relay)
lua_settable(L, -3);
struct error *e = diag_last_error(relay_get_diag(relay));
- if (e != NULL) {
- lua_pushstring(L, "message");
- lua_pushstring(L, e->errmsg);
- lua_settable(L, -3);
- }
+ if (e != NULL)
+ lbox_push_replication_error_message(L, e, -1);
break;
}
default: unreachable();
diff --git a/test/replication/gh-4402-info-errno.result b/test/replication/gh-4402-info-errno.result
new file mode 100644
index 000000000..661eea38b
--- /dev/null
+++ b/test/replication/gh-4402-info-errno.result
@@ -0,0 +1,63 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+
+--
+-- gh-4402: replication info table should contain not only
+-- Tarantool's error, but also a system error (errno's message)
+-- when possible.
+--
+
+box.schema.user.grant('guest', 'replication')
+ | ---
+ | ...
+
+test_run:cmd('create server replica with rpl_master=default, script="replication/replica.lua"')
+ | ---
+ | - true
+ | ...
+test_run:cmd('start server replica')
+ | ---
+ | - true
+ | ...
+i = box.info
+ | ---
+ | ...
+replica_id = i.id % 2 + 1
+ | ---
+ | ...
+d = i.replication[replica_id].downstream
+ | ---
+ | ...
+d ~= nil and d.status == 'follow' or i
+ | ---
+ | - true
+ | ...
+
+test_run:cmd('stop server replica')
+ | ---
+ | - true
+ | ...
+test_run:cmd("cleanup server replica")
+ | ---
+ | - true
+ | ...
+test_run:cmd("delete server replica")
+ | ---
+ | - true
+ | ...
+i = box.info
+ | ---
+ | ...
+d = i.replication[replica_id].downstream
+ | ---
+ | ...
+d ~= nil and d.system_message ~= nil and d.message ~= nil or i
+ | ---
+ | - true
+ | ...
+
+box.schema.user.revoke('guest', 'replication')
+ | ---
+ | ...
diff --git a/test/replication/gh-4402-info-errno.test.lua b/test/replication/gh-4402-info-errno.test.lua
new file mode 100644
index 000000000..1b2d9d814
--- /dev/null
+++ b/test/replication/gh-4402-info-errno.test.lua
@@ -0,0 +1,25 @@
+test_run = require('test_run').new()
+
+--
+-- gh-4402: replication info table should contain not only
+-- Tarantool's error, but also a system error (errno's message)
+-- when possible.
+--
+
+box.schema.user.grant('guest', 'replication')
+
+test_run:cmd('create server replica with rpl_master=default, script="replication/replica.lua"')
+test_run:cmd('start server replica')
+i = box.info
+replica_id = i.id % 2 + 1
+d = i.replication[replica_id].downstream
+d ~= nil and d.status == 'follow' or i
+
+test_run:cmd('stop server replica')
+test_run:cmd("cleanup server replica")
+test_run:cmd("delete server replica")
+i = box.info
+d = i.replication[replica_id].downstream
+d ~= nil and d.system_message ~= nil and d.message ~= nil or i
+
+box.schema.user.revoke('guest', 'replication')
diff --git a/test/replication/suite.cfg b/test/replication/suite.cfg
index eb25077d8..4abc93e20 100644
--- a/test/replication/suite.cfg
+++ b/test/replication/suite.cfg
@@ -11,6 +11,7 @@
"on_schema_init.test.lua": {},
"long_row_timeout.test.lua": {},
"join_without_snap.test.lua": {},
+ "gh-4402-info-errno.test.lua": {},
"*": {
"memtx": {"engine": "memtx"},
"vinyl": {"engine": "vinyl"}
--
2.21.0 (Apple Git-122.2)
^ permalink raw reply [flat|nested] 11+ messages in thread