core: increase error logging verbosity from C#11716
Merged
locker merged 2 commits intotarantool:masterfrom Aug 11, 2025
Merged
Conversation
locker
reviewed
Jul 25, 2025
changelogs/unreleased/gh-10116-increase-error-logging-verbosity-C.md
Outdated
Show resolved
Hide resolved
78ea954 to
41289f1
Compare
41289f1 to
c61ba20
Compare
locker
reviewed
Jul 30, 2025
Member
|
@nshy ASAN workflows fail. PTAL. |
Contributor
Author
Fixed. diffdiff --git a/src/lua/error.lua b/src/lua/error.lua
index 5adf83c5e8..130943751d 100644
--- a/src/lua/error.lua
+++ b/src/lua/error.lua
@@ -269,6 +269,7 @@ local function error_to_string(err)
end
local ibuf = buffer.internal.cord_ibuf_take()
+ ibuf:reserve(ibuf:capacity())
local len = ffi.C.error_to_string(err, ibuf.wpos, ibuf:capacity())
if len + 1 > ibuf:capacity() then
ibuf:reserve(len + 1)
diff --git a/test/unit/error.c b/test/unit/error.c
index 44a90f6a1c..e597873144 100644
--- a/test/unit/error.c
+++ b/test/unit/error.c
@@ -672,15 +672,18 @@ test_logging(void)
/* Some of basic errors. */
e = BuildTimedOut("filename", 303);
+ error_ref(e);
error_log(e);
fail_unless(fgets(buf, size, f) != NULL);
ok(strstr(buf,
"E> timed out "
"{\"type\":\"TimedOut\",\"errno\":110,"
"\"trace\":[{\"file\":\"filename\",\"line\":303}]}") != NULL);
+ error_unref(e);
/* ClientError plus payload plus some escaping. */
e = BuildClientError("\"filename\"", 707, ER_READONLY);
+ error_ref(e);
/* Check json escaping payload name and data. */
error_set_str(e, "\"f1\"", "\"p1\"");
error_log(e);
@@ -692,13 +695,15 @@ test_logging(void)
"\"\\\"f1\\\"\":\"\\\"p1\\\"\","
"\"trace\":[{\"file\":\"\\\"filename\\\"\",\"line\":707}]}"
) != NULL);
+ error_unref(e);
/* CustomError plus some unusual branches. */
/* 1. Check json escaping custom type. */
/* 2. Check no trace. */
/* 3. Check zero code. */
e = BuildCustomError("", 505, "\"typo\"", 0);
- e->errmsg = "some message";
+ error_ref(e);
+ strcpy(e->errmsg_buf, "some message");
/* 4. Check errno. */
e->saved_errno = 11;
error_log(e);
@@ -707,14 +712,16 @@ test_logging(void)
"E> some message "
"{\"type\":\"\\\"typo\\\"\",\"errno\":11,"
"\"trace\":[{}]}") != NULL);
+ error_unref(e);
/* Check stacked error. */
struct error *e1 = BuildCustomError("", 101, "level_1", 0);
- e1->errmsg = "message 1";
+ error_ref(e1);
+ strcpy(e1->errmsg_buf, "message 1");
struct error *e2 = BuildCustomError("", 202, "level_2", 0);
- e2->errmsg = "message 2";
+ strcpy(e2->errmsg_buf, "message 2");
struct error *e3 = BuildCustomError("", 303, "level_3", 0);
- e3->errmsg = "message 3";
+ strcpy(e3->errmsg_buf, "message 3");
error_set_prev(e1, e2);
error_set_prev(e2, e3);
error_log(e2);
@@ -728,6 +735,7 @@ test_logging(void)
"Caused by: message 3 "
"{\"type\":\"level_3\","
"\"trace\":[{}]}") != 0);
+ error_unref(e1);
fclose(f);
say_logger_free(); |
c61ba20 to
e4a346d
Compare
Contributor
|
Sorry, reassigned by a mistake. |
locker
approved these changes
Aug 1, 2025
locker
approved these changes
Aug 1, 2025
Totktonada
reviewed
Aug 4, 2025
Totktonada
reviewed
Aug 4, 2025
Totktonada
reviewed
Aug 4, 2025
xuniq
approved these changes
Aug 5, 2025
It will give cleaner diagnostic for user. Currently they have 'LuajitError: ' prefix and 'fatal error, exiting the event loop' suffix. Also we are going to print error trace in technical format on panic. It would clutter diagnostic even more. Part of tarantool#10116 NO_CHANGELOG=minor changes NO_DOC=minor changes
e4a346d to
cfe1400
Compare
Totktonada
approved these changes
Aug 11, 2025
When compat option `box_error_serialize_verbose` is set to `new` then `tostring` of Lua box error object prints error with increased verbosity. This affects logging error from Lua. Unfortunately it does not affect logging box error from C. Let's move `tostring` logic to C code so we can align logging from Lua and C. Note that: - error is logged from C verbosly unconditionally (`box_error_serialize_verbose` is not taken into account) - details that printed before ':' are removed (error code for `ClientError` for example) While at it we did next changes in `tostring()`: - frames are printed from effect to cause - prefix `Caused by: ` is added for cause frames Note that `say_logger_free()` it patched so that logger can be reinitialized in `unit/fiber.cc` test. Closes tarantool#10116 Follow-up tarantool#9105 NO_DOC=minor change
cfe1400 to
01f8489
Compare
Buristan
added a commit
to Buristan/tarantool
that referenced
this pull request
Aug 18, 2025
This patch is a follow-up to the commit 2120de9 ("core: increase error logging verbosity from C"). The test `test_increased_error_string_conversion_verbosity` extended in this commit assumes that the repository is cloned in the directory "tarantool", so the path to the test file ends like the following: | ..l/test/box-luatest/error_subsystem_improvements_test.lua In the case when the last letter in the directory name is different, the test fails. This patch changes strict comparison of strings to pattern matching to accept any character in the directory name. Follows-up tarantool#11716 NO_DOC=test NO_CHANGELOG=test
Buristan
added a commit
to Buristan/tarantool
that referenced
this pull request
Aug 18, 2025
This patch is a follow-up to the commit 2120de9 ("core: increase error logging verbosity from C"). The test `test_increased_error_string_conversion_verbosity` extended in this commit assumes that the repository is cloned in the directory "tarantool", so the path to the test file ends like the following: | ...l/test/box-luatest/error_subsystem_improvements_test.lua In the case when the last letter in the directory name is different, the test fails. This patch chomps the last character in the directory name to avoid failures. Follows-up tarantool#11716 NO_DOC=test NO_CHANGELOG=test
Buristan
added a commit
that referenced
this pull request
Aug 18, 2025
This patch is a follow-up to the commit 2120de9 ("core: increase error logging verbosity from C"). The test `test_increased_error_string_conversion_verbosity` extended in this commit assumes that the repository is cloned in the directory "tarantool", so the path to the test file ends like the following: | ...l/test/box-luatest/error_subsystem_improvements_test.lua In the case when the last letter in the directory name is different, the test fails. This patch chomps the last character in the directory name to avoid failures. Follows-up #11716 NO_DOC=test NO_CHANGELOG=test
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When compat option
box_error_serialize_verboseis set tonewthentostringof Lua box error object prints error with increased verbosity. This affects logging error from Lua. Unfortunately it does not affect logging box error from C.Let's move
tostringlogic to C code so we align logging from Lua and C.Note that:
box_error_serialize_verboseis not taken into account)ClientErrorfor example)While at it we did next changes in
tostring():Caused by:is added for cause framesNote that
say_logger_free()it patched so that logger can be reinitialized inunit/fiber.cctest.Closes #10116
Follow-up #9105