[Tarantool-patches] [PATCH 3/9] error: move code to struct error from ClientError
Serge Petrenko
sergepetrenko at tarantool.org
Mon Nov 8 18:15:29 MSK 2021
06.11.2021 02:56, Vladislav Shpilevoy пишет:
> All optional fields soon will be moved into error_payload. Code
> was optional too. But it is needed too often, the most used field.
> The patch moves it into struct error to make it more accessible.
>
> Also in future it should allow to drop the hack
> ClientError::get_errcode() which tries to return error code
> depending on error type. But could just store the code right away.
>
> As a consequence of the patch, errors which didn't have an error
> code at all before, such as LuajitError, now have it 0 in Lua.
>
> Part of #5568
Thanks for the patch! Please, find 2 minor comments below.
> ---
> src/box/error.cc | 15 +++++++--------
> src/box/error.h | 4 +---
> src/box/index.cc | 4 ++--
> src/box/mp_error.cc | 7 +++----
> src/lib/core/diag.c | 1 +
> src/lib/core/diag.h | 2 ++
> src/lua/error.lua | 3 ++-
> test/engine/func_index.result | 3 ++-
> test/unit/mp_error.cc | 2 +-
> 9 files changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/src/box/error.cc b/src/box/error.cc
> index 225b377b7..bc0d46601 100644
> --- a/src/box/error.cc
> +++ b/src/box/error.cc
> @@ -76,7 +76,7 @@ box_error_set(const char *file, unsigned line, uint32_t code,
> struct error *e = BuildClientError(file, line, ER_UNKNOWN);
> ClientError *client_error = type_cast(ClientError, e);
> if (client_error) {
> - client_error->m_errcode = code;
> + client_error->code = code;
> va_list ap;
> va_start(ap, fmt);
> error_vformat_msg(e, fmt, ap);
> @@ -94,7 +94,7 @@ box_error_new_va(const char *file, unsigned line, uint32_t code,
> struct error *e = BuildClientError(file, line, ER_UNKNOWN);
> ClientError *client_error = type_cast(ClientError, e);
> if (client_error != NULL) {
> - client_error->m_errcode = code;
> + client_error->code = code;
> error_vformat_msg(e, fmt, ap);
> }
> return e;
> @@ -170,7 +170,7 @@ ClientError::ClientError(const type_info *type, const char *file, unsigned line,
> uint32_t errcode)
> :Exception(type, file, line)
> {
> - m_errcode = errcode;
> + code = errcode;
> if (rmean_error)
> rmean_collect(rmean_error, RMEAN_ERROR, 1);
> }
> @@ -181,7 +181,7 @@ ClientError::ClientError(const char *file, unsigned line,
> {
> va_list ap;
> va_start(ap, errcode);
> - error_vformat_msg(this, tnt_errcode_desc(m_errcode), ap);
> + error_vformat_msg(this, tnt_errcode_desc(errcode), ap);
> va_end(ap);
> }
>
> @@ -194,7 +194,7 @@ BuildClientError(const char *file, unsigned line, uint32_t errcode, ...)
> va_start(ap, errcode);
> error_vformat_msg(e, tnt_errcode_desc(errcode), ap);
> va_end(ap);
> - e->m_errcode = errcode;
> + e->code = errcode;
> return e;
> } catch (OutOfMemory *e) {
> return e;
> @@ -204,8 +204,7 @@ BuildClientError(const char *file, unsigned line, uint32_t errcode, ...)
> void
> ClientError::log() const
> {
> - say_file_line(S_ERROR, file, line, errmsg, "%s",
> - tnt_errcode_str(m_errcode));
> + say_file_line(S_ERROR, file, line, errmsg, "%s", tnt_errcode_str(code));
> }
>
>
> @@ -296,7 +295,7 @@ AccessDeniedError::AccessDeniedError(const char *file, unsigned int line,
> bool run_trigers)
> :ClientError(&type_AccessDeniedError, file, line, ER_ACCESS_DENIED)
> {
> - error_format_msg(this, tnt_errcode_desc(m_errcode),
> + error_format_msg(this, tnt_errcode_desc(code),
> access_type, object_type, object_name, user_name);
>
> struct on_access_denied_ctx ctx = {access_type, object_type, object_name};
> diff --git a/src/box/error.h b/src/box/error.h
> index 338121dd9..30ab36a97 100644
> --- a/src/box/error.h
> +++ b/src/box/error.h
> @@ -208,14 +208,12 @@ public:
> int
> errcode() const
> {
> - return m_errcode;
> + return code;
> }
>
> ClientError(const char *file, unsigned line, uint32_t errcode, ...);
>
> static uint32_t get_errcode(const struct error *e);
> - /* client errno code */
> - int m_errcode;
> protected:
> ClientError(const type_info *type, const char *file, unsigned line,
> uint32_t errcode);
> diff --git a/src/box/index.cc b/src/box/index.cc
> index 0651868d3..be14827be 100644
> --- a/src/box/index.cc
> +++ b/src/box/index.cc
> @@ -47,8 +47,8 @@ UnsupportedIndexFeature::UnsupportedIndexFeature(const char *file,
> : ClientError(file, line, ER_UNKNOWN)
> {
> struct space *space = space_cache_find_xc(index_def->space_id);
> - m_errcode = ER_UNSUPPORTED_INDEX_FEATURE;
> - error_format_msg(this, tnt_errcode_desc(m_errcode), index_def->name,
> + code = ER_UNSUPPORTED_INDEX_FEATURE;
> + error_format_msg(this, tnt_errcode_desc(code), index_def->name,
> index_type_strs[index_def->type],
> space->def->name, space->def->engine_name, what);
> }
> diff --git a/src/box/mp_error.cc b/src/box/mp_error.cc
> index 059f9bc85..35c770400 100644
> --- a/src/box/mp_error.cc
> +++ b/src/box/mp_error.cc
> @@ -261,10 +261,8 @@ missing_fields:
> }
>
> if (strcmp(mp_error->type, "ClientError") == 0) {
> - ClientError *e = new ClientError(mp_error->file, mp_error->line,
> - ER_UNKNOWN);
> - e->m_errcode = mp_error->code;
> - err = e;
> + err = new ClientError(mp_error->file, mp_error->line,
> + ER_UNKNOWN);
> } else if (strcmp(mp_error->type, "CustomError") == 0) {
> if (mp_error->custom_type == NULL)
> goto missing_fields;
> @@ -320,6 +318,7 @@ missing_fields:
> err = new ClientError(mp_error->file, mp_error->line,
> ER_UNKNOWN);
> }
> + err->code = mp_error->code;
> err->saved_errno = mp_error->saved_errno;
> error_format_msg(err, "%s", mp_error->message);
> return err;
> diff --git a/src/lib/core/diag.c b/src/lib/core/diag.c
> index f872be6a7..b557ca667 100644
> --- a/src/lib/core/diag.c
> +++ b/src/lib/core/diag.c
> @@ -116,6 +116,7 @@ error_create(struct error *e,
> e->type = type;
> e->refs = 0;
> e->saved_errno = 0;
> + e->code = 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 d7d09f6fb..30b8d2f5d 100644
> --- a/src/lib/core/diag.h
> +++ b/src/lib/core/diag.h
> @@ -87,6 +87,8 @@ struct error {
> * to the standard library, then it is 0.
> */
> int saved_errno;
> + /** Error code. Shortest possible description of error's reason. */
> + int code;
> /** Line number. */
> unsigned line;
> /* Source file name. */
> diff --git a/src/lua/error.lua b/src/lua/error.lua
> index ebc784f07..9fb227df4 100644
> --- a/src/lua/error.lua
> +++ b/src/lua/error.lua
> @@ -18,6 +18,7 @@ struct error {
> const struct type_info *_type;
> int64_t _refs;
> int _saved_errno;
> + int code;
1. All the other fields are prefixed with "_" for some reason.
Let's do the same for "code", just to comply.
> /** Line number. */
> unsigned _line;
> /* Source file name. */
> @@ -158,7 +159,7 @@ local function error_unpack(err)
> if not ffi.istype('struct error', err) then
> error("Usage: error:unpack()")
> end
> - local result = {}
> + local result = {code = err.code}
2. Why not add 'code' to 'error_fields' ?
> for key, getter in pairs(error_fields) do
> result[key] = getter(err)
> end
> diff --git a/test/engine/func_index.result b/test/engine/func_index.result
> index b689c68ec..d3f44f7be 100644
> --- a/test/engine/func_index.result
> +++ b/test/engine/func_index.result
> @@ -292,7 +292,8 @@ e = e.prev
> | ...
> e:unpack()
> | ---
> - | - base_type: LuajitError
> + | - code: 0
> + | base_type: LuajitError
> | type: LuajitError
> | message: '[string "return function(tuple) local ..."]:1: attempt
> | to call global ''require'' (a nil value)'
> diff --git a/test/unit/mp_error.cc b/test/unit/mp_error.cc
> index e85d282e7..fe0cd49b9 100644
> --- a/test/unit/mp_error.cc
> +++ b/test/unit/mp_error.cc
> @@ -349,7 +349,7 @@ test_decode_unknown_type()
> const char *pos = buffer;
> struct error *unpacked = error_unpack(&pos, len);
> error_ref(unpacked);
> - err.code = 0;
> + err.code = 1;
> err.type = "ClientError";
> ok(error_is_eq_mp_error(unpacked, &err), "check SomeNewError");
> error_unref(unpacked);
--
Serge Petrenko
More information about the Tarantool-patches
mailing list