Skip to content

lua: increase error serialization verbosity#9716

Merged
sergepetrenko merged 2 commits intotarantool:masterfrom
CuriousGeorgiy:gh-9105-error-serialization-verbosity
Mar 11, 2024
Merged

lua: increase error serialization verbosity#9716
sergepetrenko merged 2 commits intotarantool:masterfrom
CuriousGeorgiy:gh-9105-error-serialization-verbosity

Conversation

@CuriousGeorgiy
Copy link
Member

@CuriousGeorgiy CuriousGeorgiy commented Feb 20, 2024

This patch increases error serialization verbosity.

Closes #9105

@CuriousGeorgiy CuriousGeorgiy requested review from Gumix and nshy February 20, 2024 10:21
@CuriousGeorgiy CuriousGeorgiy requested review from a team as code owners February 20, 2024 10:21
@CuriousGeorgiy CuriousGeorgiy force-pushed the gh-9105-error-serialization-verbosity branch from 75507d5 to 8d5718e Compare February 20, 2024 10:22
@CuriousGeorgiy CuriousGeorgiy self-assigned this Feb 20, 2024
@CuriousGeorgiy CuriousGeorgiy force-pushed the gh-9105-error-serialization-verbosity branch 10 times, most recently from 5910118 to 2ed6b5a Compare February 22, 2024 22:36
@CuriousGeorgiy CuriousGeorgiy force-pushed the gh-9105-error-serialization-verbosity branch from 2ed6b5a to 1dd7152 Compare February 22, 2024 23:49
@CuriousGeorgiy CuriousGeorgiy force-pushed the gh-9105-error-serialization-verbosity branch 4 times, most recently from 4c29eb8 to 98927c4 Compare February 26, 2024 11:17
@coveralls
Copy link

coveralls commented Feb 26, 2024

Coverage Status

coverage: 86.978% (+0.003%) from 86.975%
when pulling fda40fd on CuriousGeorgiy:gh-9105-error-serialization-verbosity
into deaa959
on tarantool:master
.

@CuriousGeorgiy CuriousGeorgiy force-pushed the gh-9105-error-serialization-verbosity branch from 4195a53 to 0ec7dbb Compare March 1, 2024 10:55
@CuriousGeorgiy CuriousGeorgiy requested a review from nshy March 1, 2024 11:06
@nshy nshy removed their assignment Mar 4, 2024
@CuriousGeorgiy CuriousGeorgiy force-pushed the gh-9105-error-serialization-verbosity branch from 0ec7dbb to 3717f5a Compare March 4, 2024 08:38
@CuriousGeorgiy CuriousGeorgiy assigned nshy and unassigned Totktonada Mar 4, 2024
@CuriousGeorgiy CuriousGeorgiy force-pushed the gh-9105-error-serialization-verbosity branch 3 times, most recently from e449b63 to 1c29178 Compare March 4, 2024 08:45
@CuriousGeorgiy CuriousGeorgiy force-pushed the gh-9105-error-serialization-verbosity branch 2 times, most recently from 817dbf0 to 897f6f3 Compare March 4, 2024 15:28
@nshy nshy assigned Totktonada and unassigned nshy Mar 5, 2024
Currently, we delegate the work of serializing extensions to serializers in
case of known extensions. In case of unknown extensions we try to convert
them from Lua using the serialization and string conversion metamethods.

However, there are cases where we would prefer to do the serialization in
Lua instead of delegating to serializers. Let's always try to serialize
extensions first using Lua via `luaL_convertfield`, and, if it fails, or
the conversion returns an extension again, fallback to the serializers.

Needed for tarantool#9105

NO_CHANGELOG=<refactoring>
NO_DOC=<refactoring>
NO_TEST=<refactoring>
@CuriousGeorgiy CuriousGeorgiy force-pushed the gh-9105-error-serialization-verbosity branch 2 times, most recently from 35dadd2 to 2afa268 Compare March 7, 2024 09:21
Copy link
Contributor

@Totktonada Totktonada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems, our serialization framework hardly fits our serialization needs. This pull request pushes it to the limit, but is has to.

I've no objections here.

@Totktonada Totktonada removed their assignment Mar 7, 2024
Currently, the error object's `__serialize` metamethod and the Lua
serializer only display the `message` field omitting all other potentially
useful fields.

Let's call the error object's `__serialize` metamethod from the Lua
serializer and call `error:unpack` from the `__serialize` metamethod to
display all other error object fields. This will transparently allow to
display the whole error stack (i.e., the cause chain).

Let's make the error object's  `__tostring` metamethod return the `message`
field followed by a whitespace and the rest of the fields encoded to JSON.
Let's print the error stack (i.e., the cause chain) on separate lines.

Since this change may potentially break existing users, let's add a new
`box_error_serialize_verbose` option to `compat` to retain old behavior,
and disable the new behaviour by default. Let's also retain the old
behaviour for the MsgPack serializers unconditionally.

Closes tarantool#9105

@TarantoolBot document
Title: Document increased error serialization verbosity
Product: Tarantool
Since: 3.1
Root documents: https://www.tarantool.io/en/doc/latest/reference/reference_lua/box_error/
and https://www.tarantool.io/en/doc/latest/reference/reference_lua/box_error/error
and https://tarantool.io/compat/box_error_serialize_verbose

[Link to the design document](https://www.notion.so/tarantool/Error-subsystem-improvements-90faa0a4714b4143abaf8bed2c10b2fc?pvs=4#072087684e094ea28cba88002236178a)

Please add a new https://tarantool.io/compat/box_error_serialize_verbose
page for the `box_error_serialize_verbose` compatibility option.
@CuriousGeorgiy CuriousGeorgiy force-pushed the gh-9105-error-serialization-verbosity branch from 2afa268 to fda40fd Compare March 11, 2024 09:27
@CuriousGeorgiy CuriousGeorgiy added the full-ci Enables all tests for a pull request label Mar 11, 2024
@sergepetrenko sergepetrenko merged commit cc7af2e into tarantool:master Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

full-ci Enables all tests for a pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Increase verbosity of error serialization

8 participants