From cb6e315d36d318cd701b1e675da581958dbc5406 Mon Sep 17 00:00:00 2001 From: Brandon Bennett Date: Fri, 7 Apr 2023 09:08:26 -0600 Subject: [PATCH] implement RPC errors for (#34) Adds support for returning RPC errors as Go errors. This does it by always returning a list of RPCErrors (even when there is one) and supporting the `Unwrap() []error` implicit interface for `errors` package (Go 1.20 and later) --- TODO.md | 2 +- go.mod | 8 +++++++- go.sum | 16 ++++++++++++++++ inttest/ssh_test.go | 12 ++++++++++++ msg.go | 29 ++++++++++++++++++++++++++--- session.go | 11 ++++++----- 6 files changed, 68 insertions(+), 10 deletions(-) diff --git a/TODO.md b/TODO.md index c28f5ed..474b293 100644 --- a/TODO.md +++ b/TODO.md @@ -4,7 +4,7 @@ - [X] Upgrade transport based on versions (1.0->1.1 upgrade to chunked) - [X] Cleanup request/response API (Session.Do, session.Call?) -- [ ] Convert rpc errors to go errors +- [X] Convert rpc errors to go errors - [X] shutdown / close - [ ] all RFC6241 operations (methods + op structs?) - [ ] unit tests (>80% coverage?) diff --git a/go.mod b/go.mod index fe6e034..db53bea 100644 --- a/go.mod +++ b/go.mod @@ -4,4 +4,10 @@ go 1.17 require golang.org/x/crypto v0.7.0 -require golang.org/x/sys v0.6.0 // indirect +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/stretchr/testify v1.8.2 // indirect + golang.org/x/sys v0.6.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) diff --git a/go.sum b/go.sum index 52d8629..8dafeda 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,15 @@ +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8= +github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= @@ -36,3 +48,7 @@ golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtn golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/inttest/ssh_test.go b/inttest/ssh_test.go index 03dd91f..1bfe0df 100644 --- a/inttest/ssh_test.go +++ b/inttest/ssh_test.go @@ -11,6 +11,7 @@ import ( "github.com/nemith/netconf" ncssh "github.com/nemith/netconf/transport/ssh" + "github.com/stretchr/testify/assert" "golang.org/x/crypto/ssh" ) @@ -105,3 +106,14 @@ func TestSSHGetConfig(t *testing.T) { t.Fatalf("failed to close session: %v", err) } } + +func TestBadGetConfig(t *testing.T) { + session := setupSSH(t) + + ctx := context.Background() + cfg, err := session.GetConfig(ctx, "non-exist") + assert.Nil(t, cfg) + var rpcErrors netconf.RPCErrors + assert.ErrorAs(t, err, &rpcErrors) + assert.Len(t, rpcErrors, 1) +} diff --git a/msg.go b/msg.go index c94807b..df2e731 100644 --- a/msg.go +++ b/msg.go @@ -2,6 +2,8 @@ package netconf import ( "encoding/xml" + "fmt" + "strings" "time" ) @@ -28,8 +30,8 @@ type RPCReplyMsg struct { // RPC call and there were no errors. This IS NOT set to true if data is // also returned. To check if a call is ok then look at the Errors field - Errors []RPCError `xml:"rpc-error,omitempty"` - Data []byte `xml:",innerxml"` + Errors RPCErrors `xml:"rpc-error,omitempty"` + Data []byte `xml:",innerxml"` } type NotificationMsg struct { @@ -67,5 +69,26 @@ type RPCError struct { } func (e RPCError) Error() string { - return e.Message + return fmt.Sprintf("rpc error: %s", e.Message) +} + +type RPCErrors []RPCError + +func (errs RPCErrors) Error() string { + var sb strings.Builder + for i, err := range errs { + if i > 0 { + sb.WriteRune('\n') + } + sb.WriteString(err.Error()) + } + return sb.String() +} + +func (errs RPCErrors) Unwrap() []error { + boxed := make([]error, len(errs)) + for i, err := range errs { + boxed[i] = err + } + return boxed } diff --git a/session.go b/session.go index dc4b753..9cd6a03 100644 --- a/session.go +++ b/session.go @@ -302,14 +302,15 @@ func (s *Session) Call(ctx context.Context, op interface{}, resp interface{}) er } if resp != nil { - if err := xml.Unmarshal(reply.Data, resp); err != nil { - return fmt.Errorf("failed to decode response: %w", err) - } + err = xml.Unmarshal(reply.Data, resp) } - // XXX: Need to handle RPC errors here. + // if we have RPC errors return them + if reply.Errors != nil { + err = reply.Errors + } - return nil + return err } // Close will gracefully close the sessions first by sending a `close-session`