From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 994514696C4 for ; Sat, 18 Apr 2020 23:39:57 +0300 (MSK) References: <3a69a4e67c4c9d74c5abebfac6d89824c6dcd7c1.1587223627.git.lvasiliev@tarantool.org> From: Vladislav Shpilevoy Message-ID: <22a19517-9864-68d2-c98e-c6121a56b398@tarantool.org> Date: Sat, 18 Apr 2020 22:39:52 +0200 MIME-Version: 1.0 In-Reply-To: <3a69a4e67c4c9d74c5abebfac6d89824c6dcd7c1.1587223627.git.lvasiliev@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH V5 6/6] error: add error MsgPack encoding List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Leonid Vasiliev , alexander.turenko@tarantool.org Cc: tarantool-patches@dev.tarantool.org Thanks for the patch! On 18/04/2020 17:29, Leonid Vasiliev wrote: > Co-authored-by: Vladislav Shpilevoy > > Closes #4398 > > @TarantoolBot document > Title: msgpack format for MP_ERROR > > For sent an error over IProto MP_ERROR(0x03) subtype > has been added to MsgPack MP_EXT. > Now, if session setting 'error_marshaling_enabled' > is true error will be encoded as: > ``` > +--------+----------+========+ > | MP_EXT | MP_ERROR | MP_MAP | > +--------+----------+========+ > ``` > > MP_ERROR: { > MP_ERROR_STACK: { > { > MP_ERROR_TYPE: , > MP_ERROR_FILE: , > MP_ERROR_LINE: , > MP_ERROR_REASON: , > MP_ERROR_ERRNO: , > MP_ERROR_FIELDS: { > : ..., > : ..., > ... > }, > }, > } > } > > MP_ERROR_STACK = 0x00 > > At the map passes all necessary error details (depending > from the error type) to create a copy of the error > on the client side. > The necessary fields: > MP_ERROR_TYPE = 0x00 - error type(string). > MP_ERROR_FILE = 0x01 - file name from trace. > MP_ERROR_LINE = 0x02 - line of the file from trace. > MP_ERROR_REASON = 0x03 - error message. > MP_ERROR_ERRNO = 0x04 - saved errno. > MP_ERROR_FIELDS = 0x05 - additional fields passes at MAP (MP_ERROR_FIELDS). > Now additional fields may content: > - ClientError code > - Custom type > - Access Denied object type > - Access Denied object name > - Access Denied access type I rewrote the commit message, force pushed. Also I added MP_ERROR_CODE to the root map. Because I realized a code is available for every error via public box_error_code() function in C. Also I renamed REASON to MESSAGE. Reason is a legacy name used only in Lua and only in error constructor. It is visible as .message after error creation. ==================== @TarantoolBot document Title: Error objects encoding in MessagePack Until now an error sent over IProto, or serialized into MessagePack was turned into a string consisting of the error message. As a result, all other error object attributes were lost, including type of the object. On client side seeing a string it was not possible to tell whether the string is a real string, or it is a serialized error. To deal with that the error objects encoding is reworked from the scratch. Now, when session setting `error_marshaling_enabled` is true, all fibers of that session will encode error objects as a new MP_EXT type - MP_ERROR (0x03). ``` +--------+----------+========+ | MP_EXT | MP_ERROR | MP_MAP | +--------+----------+========+ MP_ERROR: { MP_ERROR_STACK: [ { MP_ERROR_TYPE: , MP_ERROR_FILE: , MP_ERROR_LINE: , MP_ERROR_MESSAGE: , MP_ERROR_ERRNO: , MP_ERROR_CODE: , MP_ERROR_FIELDS: { : ..., : ..., ... }, ... }, ... ] } ``` On the top level there is a single key: `MP_ERROR_STACK = 0x00`. More keys can be added in future, and a client should ignore all unknown keys to keep compatibility with new versions. Every error in the stack is a map with the following keys: * `MP_ERROR_TYPE = 0x00` - error type. This is what is visible in `.base_type` field; * `MP_ERROR_FILE = 0x01` - file name from `.trace`; * `MP_ERROR_LINE = 0x02` - line from `.trace`; * `MP_ERROR_MESSAGE = 0x03` - error message from `.message`; * `MP_ERROR_ERRNO = 0x04` - errno saved at the moment of the error creation. Visible in `.errno`; * `MP_ERROR_CODE = 0x05` - error code. Visible in `.code` and in C function `box_error_code()`. * `MP_ERROR_FIELDS = 0x06` - additional fields depending on error type. For example, AccessDenied error type stores here fields `access_type`, `object_type`, `object_name`. Connector's code should ignore unknown keys met here, and be ready, that for some existing errors new fields can be added, old can be dropped. ==================== > --- > src/box/CMakeLists.txt | 1 + > src/box/lua/init.c | 56 ++++ > src/box/lua/tuple.c | 16 - > src/box/mp_error.cc | 552 +++++++++++++++++++++++++++++++++++ > src/box/mp_error.h | 60 ++++ > src/lib/core/mp_extension_types.h | 1 + > src/lua/error.c | 2 - > src/lua/error.h | 3 +- > src/lua/msgpack.c | 3 + > src/lua/utils.c | 8 +- > test/box-tap/extended_error.test.lua | 136 +++++++++ > 11 files changed, 817 insertions(+), 21 deletions(-) > create mode 100644 src/box/mp_error.cc > create mode 100644 src/box/mp_error.h > create mode 100755 test/box-tap/extended_error.test.lua > > diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt > index 6688303..81f6400 100644 > --- a/src/box/CMakeLists.txt > +++ b/src/box/CMakeLists.txt > @@ -136,6 +136,7 @@ add_library(box STATIC > wal.c > call.c > merger.c > + mp_error.cc I moved it to box_error library. ==================== diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt index 81f64001a..c82ceb56c 100644 --- a/src/box/CMakeLists.txt +++ b/src/box/CMakeLists.txt @@ -26,7 +26,7 @@ set_property(DIRECTORY PROPERTY ADDITIONAL_MAKE_CLEAN_FILES ${lua_sources}) include_directories(${ZSTD_INCLUDE_DIRS}) include_directories(${CMAKE_BINARY_DIR}/src/box/sql) -add_library(box_error STATIC error.cc errcode.c vclock.c) +add_library(box_error STATIC error.cc errcode.c vclock.c mp_error.cc) -target_link_libraries(box_error core stat) +target_link_libraries(box_error core stat mpstream) add_library(vclock STATIC vclock.c) target_link_libraries(vclock core) add_library(xrow STATIC xrow.c iproto_constants.c) target_link_libraries(xrow server core small vclock misc box_error - scramble mpstream ${MSGPUCK_LIBRARIES}) + scramble ${MSGPUCK_LIBRARIES}) @@ -136,7 +136,6 @@ add_library(box STATIC wal.c call.c merger.c - mp_error.cc ${lua_sources} lua/init.c lua/call.c ==================== I reviewed rest of the code and it looks mostly fine except things I mentioned above and which I fixed silently. See my diff below. It is already applied and force pushed. ==================== diff --git a/src/box/mp_error.cc b/src/box/mp_error.cc index 348b7a70b..86cb8dc23 100644 --- a/src/box/mp_error.cc +++ b/src/box/mp_error.cc @@ -39,20 +39,23 @@ * MP_ERROR format: * * MP_ERROR: { - * MP_ERROR_STACK: { + * MP_ERROR_STACK: [ * { - * MP_ERROR_TYPE: , - * MP_ERROR_FILE: , - * MP_ERROR_LINE: , - * MP_ERROR_REASON: , - * MP_ERROR_ERRNO: , + * MP_ERROR_TYPE: , + * MP_ERROR_FILE: , + * MP_ERROR_LINE: , + * MP_ERROR_MESSAGE: , + * MP_ERROR_ERRNO: , + * MP_ERROR_CODE: , * MP_ERROR_FIELDS: { * : ..., * : ..., * ... * }, + * ... * }, - * } + * ... + * ] * } */ @@ -64,24 +67,26 @@ enum { }; /** - * Keys used for error encode/decode to MP_ERROR. + * Keys of individual error in the stack. */ enum { - /* Error type */ + /** Error type. */ MP_ERROR_TYPE = 0x00, - /* File name from trace */ + /** File name from trace. */ MP_ERROR_FILE = 0x01, - /* Line from trace */ + /** Line from trace. */ MP_ERROR_LINE = 0x02, - /* Error message */ - MP_ERROR_REASON = 0x03, - /* Saved errno */ + /** Error message. */ + MP_ERROR_MESSAGE = 0x03, + /** Errno at the moment of error creation. */ MP_ERROR_ERRNO = 0x04, + /** Error code. */ + MP_ERROR_CODE = 0x05, /* - * Key uses for error type-specific fields which - * presents here as map: string key - value. + * Type-specific fields stored as a map + * {string key = value}. */ - MP_ERROR_FIELDS = 0x05 + MP_ERROR_FIELDS = 0x06 }; /** @@ -89,15 +94,15 @@ enum { * during decoding MP_ERROR. */ struct mp_error { - uint32_t error_code; + uint32_t code; uint32_t line; uint32_t saved_errno; - const char *error_type; + const char *type; const char *file; - const char *reason; + const char *message; const char *custom_type; - const char *ad_obj_type; - const char *ad_obj_name; + const char *ad_object_type; + const char *ad_object_name; const char *ad_access_type; }; @@ -110,21 +115,18 @@ mp_error_create(struct mp_error *mp_error) static uint32_t encode_error_size(const struct error *error) { - uint32_t errcode = 0; + uint32_t errcode = box_error_code(error); - bool is_client = false; bool is_custom = false; bool is_access_denied = false; - if (strcmp(error->type->name, "ClientError") == 0) { - is_client = true; - } else if (strcmp(error->type->name, "CustomError") == 0) { + if (strcmp(error->type->name, "CustomError") == 0) { is_custom = true; } else if (strcmp(error->type->name, "AccessDeniedError") == 0) { is_access_denied = true; } - uint32_t details_num = 5; + uint32_t details_num = 6; uint32_t data_size = 0; data_size += mp_sizeof_uint(MP_ERROR_TYPE); @@ -133,40 +135,29 @@ encode_error_size(const struct error *error) data_size += mp_sizeof_uint(error->line); data_size += mp_sizeof_uint(MP_ERROR_FILE); data_size += mp_sizeof_str(strlen(error->file)); - data_size += mp_sizeof_uint(MP_ERROR_REASON); + data_size += mp_sizeof_uint(MP_ERROR_MESSAGE); data_size += mp_sizeof_str(strlen(error->errmsg)); data_size += mp_sizeof_uint(MP_ERROR_ERRNO); data_size += mp_sizeof_uint(error->saved_errno); + data_size += mp_sizeof_uint(MP_ERROR_CODE); + data_size += mp_sizeof_uint(errcode); - if (is_client) { + if (is_access_denied) { ++details_num; data_size += mp_sizeof_uint(MP_ERROR_FIELDS); - errcode = box_error_code(error); - data_size += mp_sizeof_map(1); - data_size += mp_sizeof_str(strlen("code")); - data_size += mp_sizeof_uint(errcode); - } else if (is_access_denied) { - ++details_num; - data_size += mp_sizeof_uint(MP_ERROR_FIELDS); - errcode = box_error_code(error); - data_size += mp_sizeof_map(4); - data_size += mp_sizeof_str(strlen("code")); - data_size += mp_sizeof_uint(errcode); + data_size += mp_sizeof_map(3); AccessDeniedError *ad_err = type_cast(AccessDeniedError, error); - data_size += mp_sizeof_str(strlen("AD_OBJ_TYPE")); + data_size += mp_sizeof_str(strlen("object_type")); data_size += mp_sizeof_str(strlen(ad_err->object_type())); - data_size += mp_sizeof_str(strlen("AD_OBJ_NAME")); + data_size += mp_sizeof_str(strlen("object_name")); data_size += mp_sizeof_str(strlen(ad_err->object_name())); - data_size += mp_sizeof_str(strlen("AD_ACCESS_TYPE")); + data_size += mp_sizeof_str(strlen("access_type")); data_size += mp_sizeof_str(strlen(ad_err->access_type())); } else if (is_custom) { ++details_num; data_size += mp_sizeof_uint(MP_ERROR_FIELDS); - errcode = box_error_code(error); - data_size += mp_sizeof_map(2); - data_size += mp_sizeof_str(strlen("code")); - data_size += mp_sizeof_uint(errcode); - data_size += mp_sizeof_str(strlen("custom")); + data_size += mp_sizeof_map(1); + data_size += mp_sizeof_str(strlen("custom_type")); data_size += mp_sizeof_str(strlen(box_error_custom_type(error))); } @@ -179,23 +170,19 @@ encode_error_size(const struct error *error) static void encode_error(const struct error *error, char **data) { - uint32_t errcode = 0; + uint32_t errcode = box_error_code(error); - bool is_client = false; bool is_custom = false; bool is_access_denied = false; - if (strcmp(error->type->name, "ClientError") == 0) { - is_client = true; - } else if (strcmp(error->type->name, "CustomError") == 0) { + if (strcmp(error->type->name, "CustomError") == 0) { is_custom = true; } else if (strcmp(error->type->name, "AccessDeniedError") == 0) { is_access_denied = true; } - uint32_t details_num = 5; - - if (is_client || is_access_denied || is_custom) + uint32_t details_num = 6; + if (is_access_denied || is_custom) ++details_num; *data = mp_encode_map(*data, details_num); @@ -206,43 +193,34 @@ encode_error(const struct error *error, char **data) *data = mp_encode_uint(*data, error->line); *data = mp_encode_uint(*data, MP_ERROR_FILE); *data = mp_encode_str(*data, error->file, strlen(error->file)); - *data = mp_encode_uint(*data, MP_ERROR_REASON); + *data = mp_encode_uint(*data, MP_ERROR_MESSAGE); *data = mp_encode_str(*data, error->errmsg, strlen(error->errmsg)); *data = mp_encode_uint(*data, MP_ERROR_ERRNO); *data = mp_encode_uint(*data, error->saved_errno); + *data = mp_encode_uint(*data, MP_ERROR_CODE); + *data = mp_encode_uint(*data, errcode); - if (is_client) { - *data = mp_encode_uint(*data, MP_ERROR_FIELDS); - errcode = box_error_code(error); - *data = mp_encode_map(*data, 1); - *data = mp_encode_str(*data, "code",strlen("code")); - *data = mp_encode_uint(*data, errcode); - } else if (is_access_denied) { + if (is_access_denied) { *data = mp_encode_uint(*data, MP_ERROR_FIELDS); - errcode = box_error_code(error); - *data = mp_encode_map(*data, 4); - *data = mp_encode_str(*data, "code",strlen("code")); - *data = mp_encode_uint(*data, errcode); + *data = mp_encode_map(*data, 3); AccessDeniedError *ad_err = type_cast(AccessDeniedError, error); - *data = mp_encode_str(*data, "AD_OBJ_TYPE", - strlen("AD_OBJ_TYPE")); + *data = mp_encode_str(*data, "object_type", + strlen("object_type")); *data = mp_encode_str(*data, ad_err->object_type(), strlen(ad_err->object_type())); - *data = mp_encode_str(*data, "AD_OBJ_NAME", - strlen("AD_OBJ_NAME")); + *data = mp_encode_str(*data, "object_name", + strlen("object_name")); *data = mp_encode_str(*data, ad_err->object_name(), strlen(ad_err->object_name())); - *data = mp_encode_str(*data, "AD_ACCESS_TYPE", - strlen("AD_ACCESS_TYPE")); + *data = mp_encode_str(*data, "access_type", + strlen("access_type")); *data = mp_encode_str(*data, ad_err->access_type(), strlen(ad_err->access_type())); } else if (is_custom) { *data = mp_encode_uint(*data, MP_ERROR_FIELDS); - errcode = box_error_code(error); - *data = mp_encode_map(*data, 2); - *data = mp_encode_str(*data, "code",strlen("code")); - *data = mp_encode_uint(*data, errcode); - *data = mp_encode_str(*data, "custom", strlen("custom")); + *data = mp_encode_map(*data, 1); + *data = mp_encode_str(*data, "custom_type", + strlen("custom_type")); const char *custom = box_error_custom_type(error); *data = mp_encode_str(*data, custom, strlen(custom)); } @@ -252,75 +230,69 @@ static struct error * build_error_xc(struct mp_error *mp_error) { /* - * To create an error the "raw" constructor using + * To create an error the "raw" constructor is used * because OOM error must be thrown in OOM case. * Builders returns a pointer to the static OOM error * in OOM case. */ struct error *err = NULL; - if (strcmp(mp_error->error_type, "ClientError") == 0) { + if (strcmp(mp_error->type, "ClientError") == 0) { ClientError *e = new ClientError(mp_error->file, mp_error->line, ER_UNKNOWN); - e->m_errcode = mp_error->error_code; + e->m_errcode = mp_error->code; err = e; - } else if (strcmp(mp_error->error_type, "CustomError") == 0) { + } else if (strcmp(mp_error->type, "CustomError") == 0) { err = new CustomError(mp_error->file, mp_error->line, - mp_error->custom_type, - mp_error->error_code); - } else if (strcmp(mp_error->error_type, "AccessDeniedError") == 0) { + mp_error->custom_type, mp_error->code); + } else if (strcmp(mp_error->type, "AccessDeniedError") == 0) { err = new AccessDeniedError(mp_error->file, mp_error->line, mp_error->ad_access_type, - mp_error->ad_obj_type, - mp_error->ad_obj_name, "", false); - } else if (strcmp(mp_error->error_type, "XlogError") == 0) { + mp_error->ad_object_type, + mp_error->ad_object_name, "", + false); + } else if (strcmp(mp_error->type, "XlogError") == 0) { err = new XlogError(&type_XlogError, mp_error->file, mp_error->line); - error_format_msg(err, "%s", mp_error->reason); - } else if (strcmp(mp_error->error_type, "XlogGapError") == 0) { + } else if (strcmp(mp_error->type, "XlogGapError") == 0) { err = new XlogGapError(mp_error->file, mp_error->line, - mp_error->reason); - } else if (strcmp(mp_error->error_type, "SystemError") == 0) { + mp_error->message); + } else if (strcmp(mp_error->type, "SystemError") == 0) { err = new SystemError(mp_error->file, mp_error->line, - "%s", mp_error->reason); - } else if (strcmp(mp_error->error_type, "SocketError") == 0) { + "%s", mp_error->message); + } else if (strcmp(mp_error->type, "SocketError") == 0) { err = new SocketError(mp_error->file, mp_error->line, "", ""); - error_format_msg(err, "%s", mp_error->reason); - } else if (strcmp(mp_error->error_type, "OutOfMemory") == 0) { + error_format_msg(err, "%s", mp_error->message); + } else if (strcmp(mp_error->type, "OutOfMemory") == 0) { err = new OutOfMemory(mp_error->file, mp_error->line, 0, "", ""); - error_format_msg(err, "%s", mp_error->reason); - } else if (strcmp(mp_error->error_type, "TimedOut") == 0) { + } else if (strcmp(mp_error->type, "TimedOut") == 0) { err = new TimedOut(mp_error->file, mp_error->line); - } else if (strcmp(mp_error->error_type, "ChannelIsClosed") == 0) { + } else if (strcmp(mp_error->type, "ChannelIsClosed") == 0) { err = new ChannelIsClosed(mp_error->file, mp_error->line); - } else if (strcmp(mp_error->error_type, "FiberIsCancelled") == 0) { + } else if (strcmp(mp_error->type, "FiberIsCancelled") == 0) { err = new FiberIsCancelled(mp_error->file, mp_error->line); - } else if (strcmp(mp_error->error_type, "LuajitError") == 0) { + } else if (strcmp(mp_error->type, "LuajitError") == 0) { err = new LuajitError(mp_error->file, mp_error->line, - mp_error->reason); - } else if (strcmp(mp_error->error_type, "IllegalParams") == 0) { + mp_error->message); + } else if (strcmp(mp_error->type, "IllegalParams") == 0) { err = new IllegalParams(mp_error->file, mp_error->line, - "%s", mp_error->reason); - } else if (strcmp(mp_error->error_type, "CollationError") == 0) { + "%s", mp_error->message); + } else if (strcmp(mp_error->type, "CollationError") == 0) { err = new CollationError(mp_error->file, mp_error->line, - "%s", mp_error->reason); - } else if (strcmp(mp_error->error_type, "SwimError") == 0) { + "%s", mp_error->message); + } else if (strcmp(mp_error->type, "SwimError") == 0) { err = new SwimError(mp_error->file, mp_error->line, - "%s", mp_error->reason); - } else if (strcmp(mp_error->error_type, "CryptoError") == 0) { + "%s", mp_error->message); + } else if (strcmp(mp_error->type, "CryptoError") == 0) { err = new CryptoError(mp_error->file, mp_error->line, - "%s", mp_error->reason); + "%s", mp_error->message); } else { err = new ClientError(mp_error->file, mp_error->line, ER_UNKNOWN); } - - if (err) { - err->saved_errno = mp_error->saved_errno; - error_format_msg(err, "%s", mp_error->reason); - } - + err->saved_errno = mp_error->saved_errno; + error_format_msg(err, "%s", mp_error->message); return err; } @@ -337,6 +309,25 @@ region_strdup(struct region *region, const char *str, uint32_t len) return res; } +static inline const char * +decode_and_copy_str(struct region *region, const char **data) +{ + if (mp_typeof(**data) != MP_STR) { + diag_set(ClientError, ER_INVALID_MSGPACK, + "Invalid MP_ERROR MsgPack format"); + return NULL; + } + uint32_t str_len; + const char *str = mp_decode_str(data, &str_len); + return region_strdup(region, str, str_len);; +} + +static inline bool +str_nonterm_is_eq(const char *l, const char *r, uint32_t r_len) +{ + return r_len == strlen(l) && memcmp(l, r, r_len) == 0; +} + static int decode_additional_fields(const char **data, struct region *region, struct mp_error *mp_err) @@ -344,47 +335,28 @@ decode_additional_fields(const char **data, struct region *region, uint32_t map_sz = mp_decode_map(data); const char *key; uint32_t key_len; - const char *str; - uint32_t str_len; for (uint32_t i = 0; i < map_sz; ++i) { if (mp_typeof(**data) != MP_STR) return -1; key = mp_decode_str(data, &key_len); - - if (strncmp(key, "code", key_len) == 0) { - if (mp_typeof(**data) != MP_UINT) - return -1; - mp_err->error_code = mp_decode_uint(data); - } else if (strncmp(key, "AD_OBJ_TYPE", key_len) == 0) { - if (mp_typeof(**data) != MP_STR) + if (str_nonterm_is_eq("object_type", key, key_len)) { + mp_err->ad_object_type = + decode_and_copy_str(region, data); + if (mp_err->ad_object_type == NULL) return -1; - str = mp_decode_str(data, &str_len); - mp_err->ad_obj_type = region_strdup(region, str, - str_len); - if (mp_err->ad_obj_type == NULL) + } else if (str_nonterm_is_eq("object_name", key, key_len)) { + mp_err->ad_object_name = + decode_and_copy_str(region, data); + if (mp_err->ad_object_name == NULL) return -1; - } else if (strncmp(key, "AD_OBJ_NAME", key_len) == 0) { - if (mp_typeof(**data) != MP_STR) - return -1; - str = mp_decode_str(data, &str_len); - mp_err->ad_obj_name = region_strdup(region, str, - str_len); - if (mp_err->ad_obj_name == NULL) - return -1; - } else if (strncmp(key, "AD_ACCESS_TYPE", key_len) == 0) { - if (mp_typeof(**data) != MP_STR) - return -1; - str = mp_decode_str(data, &str_len); - mp_err->ad_access_type = region_strdup(region, str, - str_len); + } else if (str_nonterm_is_eq("access_type", key, key_len)) { + mp_err->ad_access_type = + decode_and_copy_str(region, data); if (mp_err->ad_access_type == NULL) return -1; - } else if (strncmp(key, "custom", key_len) == 0) { - if (mp_typeof(**data) != MP_STR) - return -1; - str = mp_decode_str(data, &str_len); - mp_err->custom_type = region_strdup(region, str, - str_len); + } else if (str_nonterm_is_eq("custom_type", key, key_len)) { + mp_err->custom_type = + decode_and_copy_str(region, data); if (mp_err->custom_type == NULL) return -1; } else { @@ -397,43 +369,30 @@ decode_additional_fields(const char **data, struct region *region, static struct error * decode_error(const char **data) { - if (mp_typeof(**data) != MP_MAP) { - diag_set(ClientError, ER_INVALID_MSGPACK, - "Invalid MP_ERROR format"); - return NULL; - } - struct mp_error mp_err; mp_error_create(&mp_err); struct region *region = &fiber()->gc; uint32_t region_svp = region_used(region); - uint32_t map_size = mp_decode_map(data); - struct error *err = NULL; + uint32_t map_size; + + if (mp_typeof(**data) != MP_MAP) + goto error; + + map_size = mp_decode_map(data); for (uint32_t i = 0; i < map_size; ++i) { - if (mp_typeof(**data) != MP_UINT) { - diag_set(ClientError, ER_INVALID_MSGPACK, - "Invalid MP_ERROR MsgPack format"); - goto finish; - } + if (mp_typeof(**data) != MP_UINT) + goto error; uint64_t key = mp_decode_uint(data); - const char *str; - uint32_t str_len; switch(key) { case MP_ERROR_TYPE: - if (mp_typeof(**data) != MP_STR) - goto error; - str = mp_decode_str(data, &str_len); - mp_err.error_type = region_strdup(region, str, str_len); - if (mp_err.error_type == NULL) + mp_err.type = decode_and_copy_str(region, data); + if (mp_err.type == NULL) goto finish; break; case MP_ERROR_FILE: - if (mp_typeof(**data) != MP_STR) - goto error; - str = mp_decode_str(data, &str_len); - mp_err.file = region_strdup(region, str, str_len); + mp_err.file = decode_and_copy_str(region, data); if (mp_err.file == NULL) goto finish; break; @@ -442,12 +401,9 @@ decode_error(const char **data) goto error; mp_err.line = mp_decode_uint(data); break; - case MP_ERROR_REASON: - if (mp_typeof(**data) != MP_STR) - goto error; - str = mp_decode_str(data, &str_len); - mp_err.reason = region_strdup(region, str, str_len); - if (mp_err.reason == NULL) + case MP_ERROR_MESSAGE: + mp_err.message = decode_and_copy_str(region, data); + if (mp_err.message == NULL) goto finish; break; case MP_ERROR_ERRNO: @@ -455,10 +411,15 @@ decode_error(const char **data) goto error; mp_err.saved_errno = mp_decode_uint(data); break; - case MP_ERROR_FIELDS: - if (decode_additional_fields(data, region, &mp_err) != 0) { + case MP_ERROR_CODE: + if (mp_typeof(**data) != MP_UINT) goto error; - } + mp_err.code = mp_decode_uint(data); + break; + case MP_ERROR_FIELDS: + if (decode_additional_fields(data, region, + &mp_err) != 0) + goto finish; break; default: mp_next(data); @@ -468,7 +429,7 @@ decode_error(const char **data) try { err = build_error_xc(&mp_err); } catch (OutOfMemory *e) { - diag_set_error(&fiber()->diag, e); + assert(err == NULL && !diag_is_empty(diag_get())); } finish: region_truncate(region, region_svp); @@ -522,8 +483,7 @@ error_unpack(const char **data, uint32_t len) } uint64_t key = mp_decode_uint(data); switch(key) { - case MP_ERROR_STACK: - { + case MP_ERROR_STACK: { uint32_t stack_sz = mp_decode_array(data); struct error *effect = NULL; for (uint32_t i = 0; i < stack_sz; i++) { diff --git a/src/lua/error.h b/src/lua/error.h index 4e4dc048b..efaaa63fd 100644 --- a/src/lua/error.h +++ b/src/lua/error.h @@ -37,6 +37,7 @@ extern "C" { #endif /* defined(__cplusplus) */ + /** \cond public */ struct error; diff --git a/src/lua/msgpack.c b/src/lua/msgpack.c index 91560eff0..f2ae2d8c3 100644 --- a/src/lua/msgpack.c +++ b/src/lua/msgpack.c @@ -239,7 +239,6 @@ luamp_encode(struct lua_State *L, struct luaL_serializer *cfg, return top_type; } - void luamp_decode(struct lua_State *L, struct luaL_serializer *cfg, const char **data)