From 35d8cef0c4524a2238d33ebcd9d68355d30c83c2 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Wed, 23 Nov 2022 15:55:55 +0300 Subject: [PATCH] api: support error type in MessagePack Tarantool supports error extension type since version 2.4.1 [1], encoding was introduced in Tarantool 2.10.0 [2]. This patch introduces the support of Tarantool error extension type in msgpack decoders and encoders. Tarantool error extension type objects are decoded to `*tarantool.BoxError` type. `*tarantool.BoxError` may be encoded to Tarantool error extension type objects. Error extension type internals are the same as errors extended information: the only difference is that extra information is encoded as a separate error dictionary field and error extension type objects are encoded as MessagePack extension type objects. The only way to receive an error extension type object from Tarantool is to receive an explicitly built `box.error` object: either from `return box.error.new(...)` or a tuple with it. All errors raised within Tarantool (including those raised with `box.error(...)`) are encoded based on the same rules as simple errors due to backward compatibility. It is possible to create error extension type objects with Go code, but it not likely to be really useful since most of their fields is computed on error initialization on the server side (even for custom error types). This patch also adds ErrorExtensionFeature flag to client protocol features list. Without this flag, all `box.error` object sent over iproto are encoded to string. We behave like Tarantool `net.box` here: if we support the feature, we provide the feature flag. 1. tarantool/tarantool#4398 2. tarantool/tarantool#6433 Closes #209 --- CHANGELOG.md | 1 + box_error.go | 122 +++++++++++++++ box_error_msgpack_test.go | 314 ++++++++++++++++++++++++++++++++++++++ config.lua | 71 +++++++++ example_test.go | 2 +- msgpack.go | 4 + msgpack_helper_test.go | 14 ++ msgpack_v5.go | 4 + msgpack_v5_helper_test.go | 17 +++ protocol.go | 7 +- tarantool_test.go | 16 +- test_helpers/utils.go | 14 ++ 12 files changed, 579 insertions(+), 7 deletions(-) create mode 100644 box_error_msgpack_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 39ba939ca..98c0a6d67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release. - Support iproto feature discovery (#120). - Support errors extended information (#209). +- Error type support in MessagePack (#209). ### Changed diff --git a/box_error.go b/box_error.go index 0bfec3a65..00c563e2e 100644 --- a/box_error.go +++ b/box_error.go @@ -4,6 +4,8 @@ import ( "fmt" ) +const errorExtID = 3 + // BoxError is a type representing Tarantool `box.error` object: a single // MP_ERROR_STACK object with a link to the previous stack error. // See https://www.tarantool.io/en/doc/latest/reference/reference_lua/box_error/error/ @@ -149,3 +151,123 @@ func decodeBoxError(d *decoder) (*BoxError, error) { return nil, nil } + +func encodeBoxError(enc *encoder, boxError *BoxError) (err error) { + if err = enc.EncodeMapLen(1); err != nil { + return err + } + if err = encodeUint(enc, KeyErrorStack); err != nil { + return err + } + + var stackDepth = boxError.Depth() + if err = enc.EncodeArrayLen(stackDepth); err != nil { + return err + } + + for ; stackDepth > 0; stackDepth-- { + fieldsLen := len(boxError.Fields) + + if fieldsLen > 0 { + if err = enc.EncodeMapLen(7); err != nil { + return err + } + } else { + if err = enc.EncodeMapLen(6); err != nil { + return err + } + } + + if err = encodeUint(enc, KeyErrorType); err != nil { + return err + } + if err = enc.EncodeString(boxError.Type); err != nil { + return err + } + + if err = encodeUint(enc, KeyErrorFile); err != nil { + return err + } + if err = enc.EncodeString(boxError.File); err != nil { + return err + } + + if err = encodeUint(enc, KeyErrorLine); err != nil { + return err + } + if err = enc.EncodeUint32(boxError.Line); err != nil { + return err + } + + if err = encodeUint(enc, KeyErrorMessage); err != nil { + return err + } + if err = enc.EncodeString(boxError.Msg); err != nil { + return err + } + + if err = encodeUint(enc, KeyErrorErrno); err != nil { + return err + } + if err = enc.EncodeUint32(boxError.Errno); err != nil { + return err + } + + if err = encodeUint(enc, KeyErrorErrcode); err != nil { + return err + } + if err = enc.EncodeUint32(boxError.Code); err != nil { + return err + } + + if fieldsLen > 0 { + if err = encodeUint(enc, KeyErrorFields); err != nil { + return err + } + + if err = enc.EncodeMapLen(fieldsLen); err != nil { + return err + } + + for k, v := range boxError.Fields { + if err = enc.Encode(k); err != nil { + return err + } + if err = enc.Encode(v); err != nil { + return err + } + } + } + + if stackDepth > 1 { + boxError = boxError.Prev + } + } + + return nil +} + +func (boxError *BoxError) MarshalMsgpack() (b []byte, err error) { + var buf smallWBuf + + enc := newEncoder(&buf) + if err = encodeBoxError(enc, boxError); err != nil { + return b, err + } + + return buf.b, nil +} + +func (boxError *BoxError) UnmarshalMsgpack(b []byte) error { + var buf smallBuf = smallBuf{b: b} + + dec := newDecoder(&buf) + val, err := decodeBoxError(dec) + if err != nil { + return err + } + + *boxError = *val + + return nil +} diff --git a/box_error_msgpack_test.go b/box_error_msgpack_test.go new file mode 100644 index 000000000..4ec859cd4 --- /dev/null +++ b/box_error_msgpack_test.go @@ -0,0 +1,314 @@ +package tarantool_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" + . "github.com/tarantool/go-tarantool" + "github.com/tarantool/go-tarantool/test_helpers" +) + +var space = "test_error_type" +var index = "primary" + +type TupleBoxError struct { + pk string // BoxError cannot be used as primary key + val BoxError +} + +func (t *TupleBoxError) EncodeMsgpack(e *encoder) error { + if err := e.EncodeArrayLen(2); err != nil { + return err + } + + if err := e.EncodeString(t.pk); err != nil { + return err + } + + return e.Encode(&t.val) +} + +func (t *TupleBoxError) DecodeMsgpack(d *decoder) error { + var err error + var l int + if l, err = d.DecodeArrayLen(); err != nil { + return err + } + if l != 2 { + return fmt.Errorf("Array length doesn't match: %d", l) + } + + if t.pk, err = d.DecodeString(); err != nil { + return err + } + + return d.Decode(&t.val) +} + +// Raw bytes encoding test is impossible for +// object with Fields since map iterating is random. +var samples = []struct { + tuple TupleBoxError + ttErrName string +}{ + { + TupleBoxError{ + "simple_error", + BoxError{ + Type: "ClientError", + File: "config.lua", + Line: 202, + Msg: "Unknown error", + Errno: 0, + Code: 0, + }, + }, + "simple_error", + }, + { + TupleBoxError{ + "access_denied_error", + BoxError{ + Type: "AccessDeniedError", + File: "/__w/sdk/sdk/tarantool-2.10/tarantool/src/box/func.c", + Line: 535, + Msg: "Execute access to function 'forbidden_function' is denied for user 'no_grants'", + Errno: 0, + Code: 42, + Fields: map[interface{}]interface{}{ + "object_type": "function", + "object_name": "forbidden_function", + "access_type": "Execute", + }, + }, + }, + "access_denied_error", + }, + { + TupleBoxError{ + "chained_error", + BoxError{ + Type: "ClientError", + File: "config.lua", + Line: 205, + Msg: "Timeout exceeded", + Errno: 0, + Code: 78, + Prev: &BoxError{ + Type: "ClientError", + File: "config.lua", + Line: 202, + Msg: "Unknown error", + Errno: 0, + Code: 0, + }, + }, + }, + "chained_error", + }, +} + +func TestErrorTypeMPEncodeDecode(t *testing.T) { + for _, testcase := range samples { + t.Run(testcase.ttErrName, func(t *testing.T) { + var buf []byte + var err error + + buf, err = marshal(&testcase.tuple) + require.Nil(t, err) + + var res TupleBoxError + err = unmarshal(buf, &res) + require.Nil(t, err) + + require.Equal(t, testcase.tuple, res) + }) + } +} + +func TestErrorTypeEval(t *testing.T) { + test_helpers.SkipIfErrorMessagePackTypeUnsupported(t) + + conn := test_helpers.ConnectWithValidation(t, server, opts) + defer conn.Close() + + for _, testcase := range samples { + t.Run(testcase.ttErrName, func(t *testing.T) { + resp, err := conn.Eval("return ...", []interface{}{&testcase.tuple.val}) + require.Nil(t, err) + require.NotNil(t, resp.Data) + require.Equal(t, len(resp.Data), 1) + actual, ok := toBoxError(resp.Data[0]) + require.Truef(t, ok, "Response data has valid type") + require.Equal(t, testcase.tuple.val, actual) + }) + } +} + +func TestErrorTypeEvalTyped(t *testing.T) { + test_helpers.SkipIfErrorMessagePackTypeUnsupported(t) + + conn := test_helpers.ConnectWithValidation(t, server, opts) + defer conn.Close() + + for _, testcase := range samples { + t.Run(testcase.ttErrName, func(t *testing.T) { + var res []BoxError + err := conn.EvalTyped("return ...", []interface{}{&testcase.tuple.val}, &res) + require.Nil(t, err) + require.NotNil(t, res) + require.Equal(t, len(res), 1) + require.Equal(t, testcase.tuple.val, res[0]) + }) + } +} + +func TestErrorTypeInsert(t *testing.T) { + test_helpers.SkipIfErrorMessagePackTypeUnsupported(t) + + conn := test_helpers.ConnectWithValidation(t, server, opts) + defer conn.Close() + + truncateEval := fmt.Sprintf("box.space[%q]:truncate()", space) + _, err := conn.Eval(truncateEval, []interface{}{}) + require.Nil(t, err) + + for _, testcase := range samples { + t.Run(testcase.ttErrName, func(t *testing.T) { + _, err = conn.Insert(space, &testcase.tuple) + require.Nil(t, err) + + checkEval := fmt.Sprintf(` + local err = rawget(_G, %q) + assert(err ~= nil) + + local tuple = box.space[%q]:get(%q) + assert(tuple ~= nil) + + local tuple_err = tuple[2] + assert(tuple_err ~= nil) + + return compare_box_errors(err, tuple_err) + `, testcase.tuple.pk, space, testcase.ttErrName) + + _, err := conn.Eval(checkEval, []interface{}{}) + require.Nilf(t, err, "Tuple has been successfully inserted") + }) + } +} + +func TestErrorTypeInsertTyped(t *testing.T) { + test_helpers.SkipIfErrorMessagePackTypeUnsupported(t) + + conn := test_helpers.ConnectWithValidation(t, server, opts) + defer conn.Close() + + truncateEval := fmt.Sprintf("box.space[%q]:truncate()", space) + _, err := conn.Eval(truncateEval, []interface{}{}) + require.Nil(t, err) + + for _, testcase := range samples { + t.Run(testcase.ttErrName, func(t *testing.T) { + var res []TupleBoxError + err = conn.InsertTyped(space, &testcase.tuple, &res) + require.Nil(t, err) + require.NotNil(t, res) + require.Equal(t, len(res), 1) + require.Equal(t, testcase.tuple, res[0]) + + checkEval := fmt.Sprintf(` + local err = rawget(_G, %q) + assert(err ~= nil) + + local tuple = box.space[%q]:get(%q) + assert(tuple ~= nil) + + local tuple_err = tuple[2] + assert(tuple_err ~= nil) + + return compare_box_errors(err, tuple_err) + `, testcase.tuple.pk, space, testcase.ttErrName) + + _, err := conn.Eval(checkEval, []interface{}{}) + require.Nilf(t, err, "Tuple has been successfully inserted") + }) + } +} + +func TestErrorTypeSelect(t *testing.T) { + test_helpers.SkipIfErrorMessagePackTypeUnsupported(t) + + conn := test_helpers.ConnectWithValidation(t, server, opts) + defer conn.Close() + + truncateEval := fmt.Sprintf("box.space[%q]:truncate()", space) + _, err := conn.Eval(truncateEval, []interface{}{}) + require.Nil(t, err) + + for _, testcase := range samples { + t.Run(testcase.ttErrName, func(t *testing.T) { + insertEval := fmt.Sprintf(` + local err = rawget(_G, %q) + assert(err ~= nil) + + local tuple = box.space[%q]:insert{%q, err} + assert(tuple ~= nil) + `, testcase.ttErrName, space, testcase.tuple.pk) + + _, err := conn.Eval(insertEval, []interface{}{}) + require.Nilf(t, err, "Tuple has been successfully inserted") + + var resp *Response + var offset uint32 = 0 + var limit uint32 = 1 + resp, err = conn.Select(space, index, offset, limit, IterEq, []interface{}{testcase.tuple.pk}) + require.Nil(t, err) + require.NotNil(t, resp.Data) + require.Equalf(t, len(resp.Data), 1, "Exactly one tuple had been found") + tpl, ok := resp.Data[0].([]interface{}) + require.Truef(t, ok, "Tuple has valid type") + require.Equal(t, testcase.tuple.pk, tpl[0]) + var actual BoxError + actual, ok = toBoxError(tpl[1]) + require.Truef(t, ok, "BoxError tuple field has valid type") + test_helpers.EqualBoxErrors(t, testcase.tuple.val, actual) + }) + } +} + +func TestErrorTypeSelectTyped(t *testing.T) { + test_helpers.SkipIfErrorMessagePackTypeUnsupported(t) + + conn := test_helpers.ConnectWithValidation(t, server, opts) + defer conn.Close() + + truncateEval := fmt.Sprintf("box.space[%q]:truncate()", space) + _, err := conn.Eval(truncateEval, []interface{}{}) + require.Nil(t, err) + + for _, testcase := range samples { + t.Run(testcase.ttErrName, func(t *testing.T) { + insertEval := fmt.Sprintf(` + local err = rawget(_G, %q) + assert(err ~= nil) + + local tuple = box.space[%q]:insert{%q, err} + assert(tuple ~= nil) + `, testcase.ttErrName, space, testcase.tuple.pk) + + _, err := conn.Eval(insertEval, []interface{}{}) + require.Nilf(t, err, "Tuple has been successfully inserted") + + var offset uint32 = 0 + var limit uint32 = 1 + var resp []TupleBoxError + err = conn.SelectTyped(space, index, offset, limit, IterEq, []interface{}{testcase.tuple.pk}, &resp) + require.Nil(t, err) + require.NotNil(t, resp) + require.Equalf(t, len(resp), 1, "Exactly one tuple had been found") + require.Equal(t, testcase.tuple.pk, resp[0].pk) + test_helpers.EqualBoxErrors(t, testcase.tuple.val, resp[0].val) + }) + } +} diff --git a/config.lua b/config.lua index c2d52d209..e2abc49f0 100644 --- a/config.lua +++ b/config.lua @@ -1,3 +1,5 @@ +local json = require('json') + -- Do not set listen for now so connector won't be -- able to send requests until everything is configured. box.cfg{ @@ -116,6 +118,21 @@ box.once("init", function() } end + local s = box.schema.space.create('test_error_type', { + id = 522, + temporary = true, + if_not_exists = true, + field_count = 2, + -- You can't specify box.error object as format type, + -- but can use them. + }) + s:create_index('primary', { + type = 'tree', + unique = true, + parts = {1, 'string'}, + if_not_exists = true + }) + --box.schema.user.grant('guest', 'read,write,execute', 'universe') box.schema.func.create('box.info') box.schema.func.create('simple_concat') @@ -126,6 +143,7 @@ box.once("init", function() box.schema.user.grant('test', 'read,write', 'space', 'test') box.schema.user.grant('test', 'read,write', 'space', 'schematest') box.schema.user.grant('test', 'read,write', 'space', 'test_perf') + box.schema.user.grant('test', 'read,write', 'space', 'test_error_type') -- grants for sql tests box.schema.user.grant('test', 'create,read,write,drop,alter', 'space') @@ -182,6 +200,8 @@ end if tarantool_version_at_least(2, 4, 1) then local e1 = box.error.new(box.error.UNKNOWN) + rawset(_G, 'simple_error', e1) + local e2 = box.error.new(box.error.TIMEOUT) e2:set_prev(e1) rawset(_G, 'chained_error', e2) @@ -192,6 +212,57 @@ if tarantool_version_at_least(2, 4, 1) then local _, access_denied_error = pcall(function() box.func.forbidden_function:call() end) box.session.su(user) rawset(_G, 'access_denied_error', access_denied_error) + + -- cdata structure is as follows: + -- + -- tarantool> err:unpack() + -- - code: val + -- base_type: val + -- type: val + -- message: val + -- field1: val + -- field2: val + -- trace: + -- - file: val + -- line: val + + local function compare_box_error_attributes(expected, actual, attr_provider) + for attr, _ in pairs(attr_provider:unpack()) do + if (attr ~= 'prev') and (attr ~= 'trace') then + local expected_val = json.encode(expected[attr]) + local actual_val = json.encode(actual[attr]) + if expected_val ~= actual_val then + error(('%s expected %s is not equal to actual %s'):format( + attr, expected_val, actual_val)) + end + end + end + end + + local function compare_box_errors(expected, actual) + if (expected == nil) and (actual ~= nil) then + error(('Expected error stack is empty, but actual error ' .. + 'has previous %s (%s) error'):format( + actual.type, actual.message)) + end + + if (expected ~= nil) and (actual == nil) then + error(('Actual error stack is empty, but expected error ' .. + 'has previous %s (%s) error'):format( + expected.type, expected.message)) + end + + compare_box_error_attributes(expected, actual, expected) + compare_box_error_attributes(expected, actual, actual) + + if (expected.prev ~= nil) or (actual.prev ~= nil) then + return compare_box_errors(expected.prev, actual.prev) + end + + return true + end + + rawset(_G, 'compare_box_errors', compare_box_errors) end box.space.test:truncate() diff --git a/example_test.go b/example_test.go index 070e44a69..7f0b64539 100644 --- a/example_test.go +++ b/example_test.go @@ -329,7 +329,7 @@ func ExampleProtocolVersion() { fmt.Println("Connector client protocol features:", clientProtocolInfo.Features) // Output: // Connector client protocol version: 4 - // Connector client protocol features: [StreamsFeature TransactionsFeature] + // Connector client protocol features: [StreamsFeature TransactionsFeature ErrorExtensionFeature] } func ExampleCommitRequest() { diff --git a/msgpack.go b/msgpack.go index 34ecc4b3b..9977e9399 100644 --- a/msgpack.go +++ b/msgpack.go @@ -48,3 +48,7 @@ func msgpackIsString(code byte) bool { return msgpcode.IsFixedString(code) || code == msgpcode.Str8 || code == msgpcode.Str16 || code == msgpcode.Str32 } + +func init() { + msgpack.RegisterExt(errorExtID, &BoxError{}) +} diff --git a/msgpack_helper_test.go b/msgpack_helper_test.go index fa47c2fda..896c105d3 100644 --- a/msgpack_helper_test.go +++ b/msgpack_helper_test.go @@ -4,6 +4,7 @@ package tarantool_test import ( + "github.com/tarantool/go-tarantool" "gopkg.in/vmihailenco/msgpack.v2" ) @@ -13,3 +14,16 @@ type decoder = msgpack.Decoder func encodeUint(e *encoder, v uint64) error { return e.EncodeUint(uint(v)) } + +func toBoxError(i interface{}) (v tarantool.BoxError, ok bool) { + v, ok = i.(tarantool.BoxError) + return +} + +func marshal(v interface{}) ([]byte, error) { + return msgpack.Marshal(v) +} + +func unmarshal(data []byte, v interface{}) error { + return msgpack.Unmarshal(data, v) +} diff --git a/msgpack_v5.go b/msgpack_v5.go index 806dd1632..e8cd9aa29 100644 --- a/msgpack_v5.go +++ b/msgpack_v5.go @@ -52,3 +52,7 @@ func msgpackIsString(code byte) bool { return msgpcode.IsFixedString(code) || code == msgpcode.Str8 || code == msgpcode.Str16 || code == msgpcode.Str32 } + +func init() { + msgpack.RegisterExt(errorExtID, (*BoxError)(nil)) +} diff --git a/msgpack_v5_helper_test.go b/msgpack_v5_helper_test.go index 347c1ba95..88154c26f 100644 --- a/msgpack_v5_helper_test.go +++ b/msgpack_v5_helper_test.go @@ -4,6 +4,7 @@ package tarantool_test import ( + "github.com/tarantool/go-tarantool" "github.com/vmihailenco/msgpack/v5" ) @@ -13,3 +14,19 @@ type decoder = msgpack.Decoder func encodeUint(e *encoder, v uint64) error { return e.EncodeUint(v) } + +func toBoxError(i interface{}) (v tarantool.BoxError, ok bool) { + var ptr *tarantool.BoxError + if ptr, ok = i.(*tarantool.BoxError); ok { + v = *ptr + } + return +} + +func marshal(v interface{}) ([]byte, error) { + return msgpack.Marshal(v) +} + +func unmarshal(data []byte, v interface{}) error { + return msgpack.Unmarshal(data, v) +} diff --git a/protocol.go b/protocol.go index 57c46e0e3..4b6ff7246 100644 --- a/protocol.go +++ b/protocol.go @@ -33,7 +33,7 @@ const ( // (supported by connector). TransactionsFeature ProtocolFeature = 1 // ErrorExtensionFeature represents support of MP_ERROR objects over MessagePack - // (unsupported by connector). + // (supported by connector). ErrorExtensionFeature ProtocolFeature = 2 // WatchersFeature represents support of watchers // (unsupported by connector). @@ -68,10 +68,13 @@ var clientProtocolInfo ProtocolInfo = ProtocolInfo{ // 1.10.0. Version: ProtocolVersion(4), // Streams and transactions were introduced in protocol version 1 - // (Tarantool 2.10.0), in connector since 1.7.0. + // (Tarantool 2.10.0), in connector since 1.7.0. Error extension + // type was introduced in protocol version 2 (Tarantool 2.10.0), + // in connector since 1.10.0. Features: []ProtocolFeature{ StreamsFeature, TransactionsFeature, + ErrorExtensionFeature, }, } diff --git a/tarantool_test.go b/tarantool_test.go index 4e7991375..3f17e875f 100644 --- a/tarantool_test.go +++ b/tarantool_test.go @@ -2868,8 +2868,12 @@ func TestConnectionProtocolInfoSupported(t *testing.T) { require.Equal(t, clientProtocolInfo, ProtocolInfo{ - Version: ProtocolVersion(4), - Features: []ProtocolFeature{StreamsFeature, TransactionsFeature}, + Version: ProtocolVersion(4), + Features: []ProtocolFeature{ + StreamsFeature, + TransactionsFeature, + ErrorExtensionFeature, + }, }) serverProtocolInfo := conn.ServerProtocolInfo() @@ -2997,8 +3001,12 @@ func TestConnectionProtocolInfoUnsupported(t *testing.T) { require.Equal(t, clientProtocolInfo, ProtocolInfo{ - Version: ProtocolVersion(4), - Features: []ProtocolFeature{StreamsFeature, TransactionsFeature}, + Version: ProtocolVersion(4), + Features: []ProtocolFeature{ + StreamsFeature, + TransactionsFeature, + ErrorExtensionFeature, + }, }) serverProtocolInfo := conn.ServerProtocolInfo() diff --git a/test_helpers/utils.go b/test_helpers/utils.go index 95997c932..c1ec4c166 100644 --- a/test_helpers/utils.go +++ b/test_helpers/utils.go @@ -196,3 +196,17 @@ func SkipIfErrorExtendedInfoUnsupported(t *testing.T) { t.Skip("Skipping test for Tarantool without extended error info support") } } + +func SkipIfErrorMessagePackTypeUnsupported(t *testing.T) { + t.Helper() + + // Tarantool error type over MessagePack supported only since 2.10.0 version + isLess, err := IsTarantoolVersionLess(2, 10, 0) + if err != nil { + t.Fatalf("Could not check the Tarantool version") + } + + if isLess { + t.Skip("Skipping test for Tarantool without support of error type over MessagePack") + } +}