From da09c28797f245dab181dac0c647b543512c73a2 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Wed, 23 Nov 2022 15:38:00 +0300 Subject: [PATCH] api: support errors extended information Since Tarantool 2.4.1, iproto error responses contain extended info with backtrace [1]. After this patch, Error would contain ExtendedInfo field (BoxError object), if it was provided. Error() handle now will print extended info, if possible. 1. https://www.tarantool.io/en/doc/latest/dev_guide/internals/box_protocol/#responses-for-errors Part of #209 --- CHANGELOG.md | 1 + box_error.go | 180 +++++++++++++++++++++++++++++++++++++++ box_error_test.go | 194 ++++++++++++++++++++++++++++++++++++++++++ config.lua | 37 ++++++++ const.go | 12 ++- errors.go | 9 +- response.go | 15 +++- tarantool_test.go | 85 ++++++++++++++++++ test_helpers/utils.go | 56 ++++++++++++ 9 files changed, 584 insertions(+), 5 deletions(-) create mode 100644 box_error.go create mode 100644 box_error_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 3019df4e0..109ff8e4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release. ### Added - Support iproto feature discovery (#120). +- Support errors extended information (#209). ### Changed diff --git a/box_error.go b/box_error.go new file mode 100644 index 000000000..06c6f574c --- /dev/null +++ b/box_error.go @@ -0,0 +1,180 @@ +package tarantool + +import ( + "bytes" + "fmt" +) + +// 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/ +// +// Since 1.10.0 +type BoxError struct { + // Type is error type that implies its source (for example, "ClientError"). + Type string + // File is a source code file where the error was caught. + File string + // Line is a number of line in the source code file where the error was caught. + Line uint64 + // Msg is the text of reason. + Msg string + // Errno is the ordinal number of the error. + Errno uint64 + // Code is the number of the error as defined in `errcode.h`. + Code uint64 + // Fields are additional fields depending on error type. For example, if + // type is "AccessDeniedError", then it will include "object_type", + // "object_name", "access_type". + Fields map[string]interface{} + // Prev is the previous error in stack. + Prev *BoxError +} + +// Error converts a BoxError to a string. +func (e *BoxError) Error() string { + s := fmt.Sprintf("%s (%s, code 0x%x), see %s line %d", + e.Msg, e.Type, e.Code, e.File, e.Line) + + if e.Prev != nil { + return fmt.Sprintf("%s: %s", s, e.Prev) + } + + return s +} + +// Depth computes the count of errors in stack, including the current one. +func (e *BoxError) Depth() int { + depth := int(0) + + cur := e + for cur != nil { + cur = cur.Prev + depth++ + } + + return depth +} + +func decodeBoxError(d *decoder) (*BoxError, error) { + var l, larr, l1, l2 int + var errorStack []BoxError + var err error + + if l, err = d.DecodeMapLen(); err != nil { + return nil, err + } + + for ; l > 0; l-- { + var cd int + if cd, err = d.DecodeInt(); err != nil { + return nil, err + } + switch cd { + case keyErrorStack: + if larr, err = d.DecodeArrayLen(); err != nil { + return nil, err + } + + errorStack = make([]BoxError, larr) + + for i := 0; i < larr; i++ { + if l1, err = d.DecodeMapLen(); err != nil { + return nil, err + } + + for ; l1 > 0; l1-- { + var cd1 int + if cd1, err = d.DecodeInt(); err != nil { + return nil, err + } + switch cd1 { + case keyErrorType: + if errorStack[i].Type, err = d.DecodeString(); err != nil { + return nil, err + } + case keyErrorFile: + if errorStack[i].File, err = d.DecodeString(); err != nil { + return nil, err + } + case keyErrorLine: + if errorStack[i].Line, err = d.DecodeUint64(); err != nil { + return nil, err + } + case keyErrorMessage: + if errorStack[i].Msg, err = d.DecodeString(); err != nil { + return nil, err + } + case keyErrorErrno: + if errorStack[i].Errno, err = d.DecodeUint64(); err != nil { + return nil, err + } + case keyErrorErrcode: + if errorStack[i].Code, err = d.DecodeUint64(); err != nil { + return nil, err + } + case keyErrorFields: + var mapk string + var mapv interface{} + + errorStack[i].Fields = make(map[string]interface{}) + + if l2, err = d.DecodeMapLen(); err != nil { + return nil, err + } + for ; l2 > 0; l2-- { + if mapk, err = d.DecodeString(); err != nil { + return nil, err + } + if mapv, err = d.DecodeInterface(); err != nil { + return nil, err + } + errorStack[i].Fields[mapk] = mapv + } + default: + if err = d.Skip(); err != nil { + return nil, err + } + } + } + + if i > 0 { + errorStack[i-1].Prev = &errorStack[i] + } + } + default: + if err = d.Skip(); err != nil { + return nil, err + } + } + } + + if len(errorStack) == 0 { + return nil, fmt.Errorf("msgpack: unexpected empty BoxError stack on decode") + } + + return &errorStack[0], nil +} + +// UnmarshalMsgpack deserializes a BoxError value from a MessagePack +// representation. +func (e *BoxError) UnmarshalMsgpack(b []byte) error { + var val *BoxError + var err error + + buf := bytes.NewBuffer(b) + dec := newDecoder(buf) + + if val, err = decodeBoxError(dec); err != nil { + return err + } + + if val != nil { + if e == nil { + e = &BoxError{} + } + *e = *val + } + + return nil +} diff --git a/box_error_test.go b/box_error_test.go new file mode 100644 index 000000000..59cd0b738 --- /dev/null +++ b/box_error_test.go @@ -0,0 +1,194 @@ +package tarantool_test + +import ( + "regexp" + "testing" + + "github.com/stretchr/testify/require" + . "github.com/tarantool/go-tarantool" +) + +var samples = map[string]BoxError{ + "SimpleError": { + Type: "ClientError", + File: "config.lua", + Line: uint64(202), + Msg: "Unknown error", + Errno: uint64(0), + Code: uint64(0), + }, + "AccessDeniedError": { + Type: "AccessDeniedError", + File: "/__w/sdk/sdk/tarantool-2.10/tarantool/src/box/func.c", + Line: uint64(535), + Msg: "Execute access to function 'forbidden_function' is denied for user 'no_grants'", + Errno: uint64(0), + Code: uint64(42), + Fields: map[string]interface{}{ + "object_type": "function", + "object_name": "forbidden_function", + "access_type": "Execute", + }, + }, + "ChainedError": { + Type: "ClientError", + File: "config.lua", + Line: uint64(205), + Msg: "Timeout exceeded", + Errno: uint64(0), + Code: uint64(78), + Prev: &BoxError{ + Type: "ClientError", + File: "config.lua", + Line: uint64(202), + Msg: "Unknown error", + Errno: uint64(0), + Code: uint64(0), + }, + }, +} + +var stringCases = map[string]struct { + e BoxError + s string +}{ + "SimpleError": { + samples["SimpleError"], + "Unknown error (ClientError, code 0x0), see config.lua line 202", + }, + "AccessDeniedError": { + samples["AccessDeniedError"], + "Execute access to function 'forbidden_function' is denied for user " + + "'no_grants' (AccessDeniedError, code 0x2a), see " + + "/__w/sdk/sdk/tarantool-2.10/tarantool/src/box/func.c line 535", + }, + "ChainedError": { + samples["ChainedError"], + "Timeout exceeded (ClientError, code 0x4e), see config.lua line 205: " + + "Unknown error (ClientError, code 0x0), see config.lua line 202", + }, +} + +func TestBoxErrorStringRepr(t *testing.T) { + for name, testcase := range stringCases { + t.Run(name, func(t *testing.T) { + require.Equal(t, testcase.s, testcase.e.Error()) + }) + } +} + +var mpDecodeSamples = map[string]struct { + b []byte + ok bool + err *regexp.Regexp +}{ + "OuterMapInvalidLen": { + []byte{0xc1}, + false, + regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding map length`), + }, + "OuterMapInvalidKey": { + []byte{0x81, 0xc1}, + false, + regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding int64`), + }, + "OuterMapExtraKey": { + []byte{0x82, 0x00, 0x91, 0x81, 0x02, 0x01, 0x11, 0x00}, + true, + regexp.MustCompile(``), + }, + "OuterMapExtraInvalidKey": { + []byte{0x81, 0x11, 0x81}, + false, + regexp.MustCompile(`EOF`), + }, + "ArrayInvalidLen": { + []byte{0x81, 0x00, 0xc1}, + false, + regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding array length`), + }, + "ArrayZeroLen": { + []byte{0x81, 0x00, 0x90}, + false, + regexp.MustCompile(`msgpack: unexpected empty BoxError stack on decode`), + }, + "InnerMapInvalidLen": { + []byte{0x81, 0x00, 0x91, 0xc1}, + false, + regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding map length`), + }, + "InnerMapInvalidKey": { + []byte{0x81, 0x00, 0x91, 0x81, 0xc1}, + false, + regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding int64`), + }, + "InnerMapInvalidErrorType": { + []byte{0x81, 0x00, 0x91, 0x81, 0x00, 0xc1}, + false, + regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding (?:string\/bytes|bytes) length`), + }, + "InnerMapInvalidErrorFile": { + []byte{0x81, 0x00, 0x91, 0x81, 0x01, 0xc1}, + false, + regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding (?:string\/bytes|bytes) length`), + }, + "InnerMapInvalidErrorLine": { + []byte{0x81, 0x00, 0x91, 0x81, 0x02, 0xc1}, + false, + regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding uint64`), + }, + "InnerMapInvalidErrorMessage": { + []byte{0x81, 0x00, 0x91, 0x81, 0x03, 0xc1}, + false, + regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding (?:string\/bytes|bytes) length`), + }, + "InnerMapInvalidErrorErrno": { + []byte{0x81, 0x00, 0x91, 0x81, 0x04, 0xc1}, + false, + regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding uint64`), + }, + "InnerMapInvalidErrorErrcode": { + []byte{0x81, 0x00, 0x91, 0x81, 0x05, 0xc1}, + false, + regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding uint64`), + }, + "InnerMapInvalidErrorFields": { + []byte{0x81, 0x00, 0x91, 0x81, 0x06, 0xc1}, + false, + regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding map length`), + }, + "InnerMapInvalidErrorFieldsKey": { + []byte{0x81, 0x00, 0x91, 0x81, 0x06, 0x81, 0xc1}, + false, + regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding (?:string\/bytes|bytes) length`), + }, + "InnerMapInvalidErrorFieldsValue": { + []byte{0x81, 0x00, 0x91, 0x81, 0x06, 0x81, 0xa3, 0x6b, 0x65, 0x79, 0xc1}, + false, + regexp.MustCompile(`msgpack: (?:unexpected|invalid|unknown) code.c1 decoding interface{}`), + }, + "InnerMapExtraKey": { + []byte{0x81, 0x00, 0x91, 0x81, 0x21, 0x00}, + true, + regexp.MustCompile(``), + }, + "InnerMapExtraInvalidKey": { + []byte{0x81, 0x00, 0x91, 0x81, 0x21, 0x81}, + false, + regexp.MustCompile(`EOF`), + }, +} + +func TestMessagePackDecode(t *testing.T) { + for name, testcase := range mpDecodeSamples { + t.Run(name, func(t *testing.T) { + var val *BoxError + err := val.UnmarshalMsgpack(testcase.b) + if testcase.ok { + require.Nilf(t, err, "No errors on decode") + } else { + require.Regexp(t, testcase.err, err.Error()) + } + }) + } +} diff --git a/config.lua b/config.lua index 5ab98cdfe..c2d52d209 100644 --- a/config.lua +++ b/config.lua @@ -130,6 +130,8 @@ box.once("init", function() -- grants for sql tests box.schema.user.grant('test', 'create,read,write,drop,alter', 'space') box.schema.user.grant('test', 'create', 'sequence') + + box.schema.user.create('no_grants') end) local function func_name() @@ -157,6 +159,41 @@ local function push_func(cnt) end rawset(_G, 'push_func', push_func) +local function tarantool_version_at_least(wanted_major, wanted_minor, wanted_patch) + -- https://github.com/tarantool/crud/blob/733528be02c1ffa3dacc12c034ee58c9903127fc/test/helper.lua#L316-L337 + local major_minor_patch = _TARANTOOL:split('-', 1)[1] + local major_minor_patch_parts = major_minor_patch:split('.', 2) + + local major = tonumber(major_minor_patch_parts[1]) + local minor = tonumber(major_minor_patch_parts[2]) + local patch = tonumber(major_minor_patch_parts[3]) + + if major < (wanted_major or 0) then return false end + if major > (wanted_major or 0) then return true end + + if minor < (wanted_minor or 0) then return false end + if minor > (wanted_minor or 0) then return true end + + if patch < (wanted_patch or 0) then return false end + if patch > (wanted_patch or 0) then return true end + + return true +end + +if tarantool_version_at_least(2, 4, 1) then + local e1 = box.error.new(box.error.UNKNOWN) + local e2 = box.error.new(box.error.TIMEOUT) + e2:set_prev(e1) + rawset(_G, 'chained_error', e2) + + local user = box.session.user() + box.schema.func.create('forbidden_function', {body = 'function() end'}) + box.session.su('no_grants') + local _, access_denied_error = pcall(function() box.func.forbidden_function:call() end) + box.session.su(user) + rawset(_G, 'access_denied_error', access_denied_error) +end + box.space.test:truncate() --box.schema.user.revoke('guest', 'read,write,execute', 'universe') diff --git a/const.go b/const.go index 8cd9bc490..b48263b6d 100644 --- a/const.go +++ b/const.go @@ -35,13 +35,14 @@ const ( KeyExpression = 0x27 KeyDefTuple = 0x28 KeyData = 0x30 - KeyError24 = 0x31 + KeyError24 = 0x31 /* Error in pre-2.4 format */ KeyMetaData = 0x32 KeyBindCount = 0x34 KeySQLText = 0x40 KeySQLBind = 0x41 KeySQLInfo = 0x42 KeyStmtID = 0x43 + KeyError = 0x52 /* Extended error in >= 2.4 format. */ KeyVersion = 0x54 KeyFeatures = 0x55 KeyTimeout = 0x56 @@ -56,6 +57,15 @@ const ( KeySQLInfoRowCount = 0x00 KeySQLInfoAutoincrementIds = 0x01 + keyErrorStack = 0x00 + keyErrorType = 0x00 + keyErrorFile = 0x01 + keyErrorLine = 0x02 + keyErrorMessage = 0x03 + keyErrorErrno = 0x04 + keyErrorErrcode = 0x05 + keyErrorFields = 0x06 + // https://github.com/fl00r/go-tarantool-1.6/issues/2 IterEq = uint32(0) // key == x ASC order diff --git a/errors.go b/errors.go index 5677d07fc..02e4635bb 100644 --- a/errors.go +++ b/errors.go @@ -4,12 +4,17 @@ import "fmt" // Error is wrapper around error returned by Tarantool. type Error struct { - Code uint32 - Msg string + Code uint32 + Msg string + ExtendedInfo *BoxError } // Error converts an Error to a string. func (tnterr Error) Error() string { + if tnterr.ExtendedInfo != nil { + return tnterr.ExtendedInfo.Error() + } + return fmt.Sprintf("%s (0x%x)", tnterr.Msg, tnterr.Code) } diff --git a/response.go b/response.go index df0b0eca7..6c3f69c99 100644 --- a/response.go +++ b/response.go @@ -151,6 +151,7 @@ func (resp *Response) decodeBody() (err error) { var stmtID, bindCount uint64 var serverProtocolInfo ProtocolInfo var feature ProtocolFeature + var errorExtendedInfo *BoxError = nil d := newDecoder(&resp.buf) @@ -172,6 +173,10 @@ func (resp *Response) decodeBody() (err error) { if resp.Data, ok = res.([]interface{}); !ok { return fmt.Errorf("result is not array: %v", res) } + case KeyError: + if errorExtendedInfo, err = decodeBoxError(d); err != nil { + return err + } case KeyError24: if resp.Error, err = d.DecodeString(); err != nil { return err @@ -236,7 +241,7 @@ func (resp *Response) decodeBody() (err error) { if resp.Code != OkCode && resp.Code != PushCode { resp.Code &^= ErrorCodeBit - err = Error{resp.Code, resp.Error} + err = Error{resp.Code, resp.Error, errorExtendedInfo} } } return @@ -247,6 +252,8 @@ func (resp *Response) decodeBodyTyped(res interface{}) (err error) { offset := resp.buf.Offset() defer resp.buf.Seek(offset) + var errorExtendedInfo *BoxError = nil + var l int d := newDecoder(&resp.buf) if l, err = d.DecodeMapLen(); err != nil { @@ -262,6 +269,10 @@ func (resp *Response) decodeBodyTyped(res interface{}) (err error) { if err = d.Decode(res); err != nil { return err } + case KeyError: + if errorExtendedInfo, err = decodeBoxError(d); err != nil { + return err + } case KeyError24: if resp.Error, err = d.DecodeString(); err != nil { return err @@ -282,7 +293,7 @@ func (resp *Response) decodeBodyTyped(res interface{}) (err error) { } if resp.Code != OkCode && resp.Code != PushCode { resp.Code &^= ErrorCodeBit - err = Error{resp.Code, resp.Error} + err = Error{resp.Code, resp.Error, errorExtendedInfo} } } return diff --git a/tarantool_test.go b/tarantool_test.go index 31d287272..1fe5e9704 100644 --- a/tarantool_test.go +++ b/tarantool_test.go @@ -3148,6 +3148,91 @@ func TestConnectionFeatureOptsImmutable(t *testing.T) { require.True(t, connected, "Reconnect success") } +func TestErrorExtendedInfoBasic(t *testing.T) { + test_helpers.SkipIfErrorExtendedInfoUnsupported(t) + + conn := test_helpers.ConnectWithValidation(t, server, opts) + defer conn.Close() + + _, err := conn.Eval("not a Lua code", []interface{}{}) + require.NotNilf(t, err, "expected error on invalid Lua code") + + ttErr, ok := err.(Error) + require.Equalf(t, ok, true, "error is built from a Tarantool error") + + expected := BoxError{ + Type: "LuajitError", + File: "eval", + Line: uint64(1), + Msg: "eval:1: unexpected symbol near 'not'", + Errno: uint64(0), + Code: uint64(32), + } + + test_helpers.CheckEqualBoxErrors(t, expected, *ttErr.ExtendedInfo) +} + +func TestErrorExtendedInfoStack(t *testing.T) { + test_helpers.SkipIfErrorExtendedInfoUnsupported(t) + + conn := test_helpers.ConnectWithValidation(t, server, opts) + defer conn.Close() + + _, err := conn.Eval("error(chained_error)", []interface{}{}) + require.NotNilf(t, err, "expected error on explicit error raise") + + ttErr, ok := err.(Error) + require.Equalf(t, ok, true, "error is built from a Tarantool error") + + expected := BoxError{ + Type: "ClientError", + File: "config.lua", + Line: uint64(214), + Msg: "Timeout exceeded", + Errno: uint64(0), + Code: uint64(78), + Prev: &BoxError{ + Type: "ClientError", + File: "config.lua", + Line: uint64(213), + Msg: "Unknown error", + Errno: uint64(0), + Code: uint64(0), + }, + } + + test_helpers.CheckEqualBoxErrors(t, expected, *ttErr.ExtendedInfo) +} + +func TestErrorExtendedInfoFields(t *testing.T) { + test_helpers.SkipIfErrorExtendedInfoUnsupported(t) + + conn := test_helpers.ConnectWithValidation(t, server, opts) + defer conn.Close() + + _, err := conn.Eval("error(access_denied_error)", []interface{}{}) + require.NotNilf(t, err, "expected error on forbidden action") + + ttErr, ok := err.(Error) + require.Equalf(t, ok, true, "error is built from a Tarantool error") + + expected := BoxError{ + Type: "AccessDeniedError", + File: "/__w/sdk/sdk/tarantool-2.10/tarantool/src/box/func.c", + Line: uint64(535), + Msg: "Execute access to function 'forbidden_function' is denied for user 'no_grants'", + Errno: uint64(0), + Code: uint64(42), + Fields: map[string]interface{}{ + "object_type": "function", + "object_name": "forbidden_function", + "access_type": "Execute", + }, + } + + test_helpers.CheckEqualBoxErrors(t, expected, *ttErr.ExtendedInfo) +} + // runTestMain is a body of TestMain function // (see https://pkg.go.dev/testing#hdr-Main). // Using defer + os.Exit is not works so TestMain body diff --git a/test_helpers/utils.go b/test_helpers/utils.go index dff0bb357..003d6cf6d 100644 --- a/test_helpers/utils.go +++ b/test_helpers/utils.go @@ -4,6 +4,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" "github.com/tarantool/go-tarantool" ) @@ -120,3 +121,58 @@ func SkipIfIdSupported(t *testing.T) { t.Skip("Skipping test for Tarantool with non-zero protocol version and features") } } + +// CheckEqualBoxErrors checks equivalence of tarantool.BoxError objects. +// +// Tarantool errors are not comparable by nature: +// +// tarantool> msgpack.decode(mp_error_repr) == msgpack.decode(mp_error_repr) +// --- +// - false +// ... +// +// Tarantool error file and line could differ even between +// different patches. +// +// So we check equivalence of all attributes except for Line and File. +// For Line and File, we check that they are filled with some non-default values +// (lines are counted starting with 1 and empty file path is not expected too). +func CheckEqualBoxErrors(t *testing.T, expected tarantool.BoxError, actual tarantool.BoxError) { + t.Helper() + + require.Equalf(t, expected.Depth(), actual.Depth(), "Error stack depth is the same") + + for { + require.Equal(t, expected.Type, actual.Type) + require.Greater(t, len(expected.File), 0) + require.Greater(t, expected.Line, uint64(0)) + require.Equal(t, expected.Msg, actual.Msg) + require.Equal(t, expected.Errno, actual.Errno) + require.Equal(t, expected.Code, actual.Code) + require.Equal(t, expected.Fields, actual.Fields) + + if expected.Prev != nil { + // Stack depth is the same + expected = *expected.Prev + actual = *actual.Prev + } else { + break + } + } +} + +// SkipIfErrorExtendedInfoUnsupported skips test run if Tarantool without +// IPROTO_ERROR (0x52) support is used. +func SkipIfErrorExtendedInfoUnsupported(t *testing.T) { + t.Helper() + + // Tarantool provides extended error info only since 2.4.1 version + isLess, err := IsTarantoolVersionLess(2, 4, 1) + if err != nil { + t.Fatalf("Could not check the Tarantool version") + } + + if isLess { + t.Skip("Skipping test for Tarantool without error extended info support") + } +}