SEBA-705 Wrap all gRPC methods;
translate permission errors;
print prefix only when emitting final error message
Change-Id: I3cbb0cf8f321c965d11354cc01491f2b3c0dc1b3
diff --git a/cmd/cordctl.go b/cmd/cordctl.go
index 55991d5..e01b027 100644
--- a/cmd/cordctl.go
+++ b/cmd/cordctl.go
@@ -17,6 +17,7 @@
package main
import (
+ "fmt"
flags "github.com/jessevdk/go-flags"
"github.com/opencord/cordctl/commands"
corderrors "github.com/opencord/cordctl/error"
@@ -26,7 +27,8 @@
func main() {
- parser := flags.NewNamedParser(path.Base(os.Args[0]), flags.Default|flags.PassAfterNonOption)
+ parser := flags.NewNamedParser(path.Base(os.Args[0]),
+ flags.HelpFlag|flags.PassDoubleDash|flags.PassAfterNonOption)
_, err := parser.AddGroup("Global Options", "", &commands.GlobalOptions)
if err != nil {
panic(err)
@@ -58,9 +60,8 @@
}
}
- // parser.ParseArgs already printed the error message
- // Any stack trace emitted by panic() here would be of main() and not useful
- // So just exit and be done with it.
+ fmt.Fprintf(os.Stderr, "%s: %s\n", os.Args[0], err.Error())
+
os.Exit(1)
}
}
diff --git a/commands/common.go b/commands/common.go
index 12b869f..c79c3ca 100644
--- a/commands/common.go
+++ b/commands/common.go
@@ -27,7 +27,6 @@
"golang.org/x/net/context"
"google.golang.org/grpc"
reflectpb "google.golang.org/grpc/reflection/grpc_reflection_v1alpha"
- "google.golang.org/grpc/status"
"log"
"os"
"strings"
@@ -57,11 +56,11 @@
h := &RpcEventHandler{}
err := grpcurl.InvokeRPC(ctx, descriptor, conn, "xos.utility.GetVersion", headers, h, h.GetParams)
if err != nil {
- return nil, err
+ return nil, corderrors.RpcErrorToCordError(err)
}
if h.Status != nil && h.Status.Err() != nil {
- return nil, h.Status.Err()
+ return nil, corderrors.RpcErrorToCordError(h.Status.Err())
}
d, err := dynamic.AsDynamicMessage(h.Response)
@@ -167,16 +166,3 @@
}
}
}
-
-func HumanReadableError(err error) string {
- st, ok := status.FromError(err)
- if ok {
- grpc_message := st.Message()
- if strings.HasPrefix(grpc_message, "Exception calling application: ") {
- return st.Message()[31:]
- } else {
- return st.Message()
- }
- }
- return err.Error()
-}
diff --git a/commands/models.go b/commands/models.go
index e539bd9..e413178 100644
--- a/commands/models.go
+++ b/commands/models.go
@@ -162,9 +162,9 @@
func UpdateModelStatusOutput(output *ModelStatusOutput, i int, id interface{}, status string, err error, final bool) {
if err != nil {
if output.Unbuffered {
- fmt.Printf("%v: %s\n", id, HumanReadableError(err))
+ fmt.Printf("%v: %s\n", id, err)
}
- output.Rows[i] = ModelStatusOutputRow{Id: id, Message: HumanReadableError(err)}
+ output.Rows[i] = ModelStatusOutputRow{Id: id, Message: err.Error()}
} else {
if output.Unbuffered && final {
fmt.Println(id)
diff --git a/commands/models_test.go b/commands/models_test.go
index 3a6031b..1f9abed 100644
--- a/commands/models_test.go
+++ b/commands/models_test.go
@@ -350,7 +350,7 @@
}
func TestModelDeleteNoExist(t *testing.T) {
- expected := `[{"id":77, "message":"Slice matching query does not exist."}]`
+ expected := `[{"id":77, "message":"Not Found [on model Slice <id=77>]"}]`
got := new(bytes.Buffer)
OutputStream = got
diff --git a/commands/orm.go b/commands/orm.go
index f57f21d..1d700d7 100644
--- a/commands/orm.go
+++ b/commands/orm.go
@@ -271,9 +271,9 @@
}
err := grpcurl.InvokeRPC(ctx, descriptor, conn, "xos.xos.Create"+modelName, headers, h, h.GetParams)
if err != nil {
- return err
+ return corderrors.RpcErrorWithModelNameToCordError(err, modelName)
} else if h.Status != nil && h.Status.Err() != nil {
- return h.Status.Err()
+ return corderrors.RpcErrorWithModelNameToCordError(h.Status.Err(), modelName)
}
resp, err := dynamic.AsDynamicMessage(h.Response)
@@ -302,9 +302,9 @@
}
err := grpcurl.InvokeRPC(ctx, descriptor, conn, "xos.xos.Update"+modelName, headers, h, h.GetParams)
if err != nil {
- return err
+ return corderrors.RpcErrorWithModelNameToCordError(err, modelName)
} else if h.Status != nil && h.Status.Err() != nil {
- return h.Status.Err()
+ return corderrors.RpcErrorWithModelNameToCordError(h.Status.Err(), modelName)
}
resp, err := dynamic.AsDynamicMessage(h.Response)
@@ -440,11 +440,11 @@
h := &RpcEventHandler{}
err := grpcurl.InvokeRPC(ctx, descriptor, conn, "xos.xos.List"+modelName, headers, h, h.GetParams)
if err != nil {
- return nil, err
+ return nil, corderrors.RpcErrorWithModelNameToCordError(err, modelName)
}
if h.Status != nil && h.Status.Err() != nil {
- return nil, h.Status.Err()
+ return nil, corderrors.RpcErrorWithModelNameToCordError(h.Status.Err(), modelName)
}
d, err := dynamic.AsDynamicMessage(h.Response)
@@ -489,11 +489,11 @@
}
err = grpcurl.InvokeRPC(ctx, descriptor, conn, "xos.xos.Filter"+modelName, headers, h, h.GetParams)
if err != nil {
- return nil, err
+ return nil, corderrors.RpcErrorWithQueriesToCordError(err, modelName, queries)
}
if h.Status != nil && h.Status.Err() != nil {
- return nil, h.Status.Err()
+ return nil, corderrors.RpcErrorWithQueriesToCordError(h.Status.Err(), modelName, queries)
}
d, err := dynamic.AsDynamicMessage(h.Response)
@@ -526,7 +526,9 @@
}
if len(models) == 0 {
- return nil, corderrors.WithStackTrace(&corderrors.ModelNotFoundError{ModelName: modelName, Queries: queries})
+ cordError := &corderrors.ModelNotFoundError{}
+ cordError.Obj = corderrors.ObjectReference{ModelName: modelName, Queries: queries}
+ return nil, corderrors.WithStackTrace(cordError)
}
return models[0], nil
@@ -621,11 +623,11 @@
}
err := grpcurl.InvokeRPC(ctx, descriptor, conn, "xos.xos.Delete"+modelName, headers, h, h.GetParams)
if err != nil {
- return err
+ return corderrors.RpcErrorWithIdToCordError(err, modelName, id)
}
if h.Status != nil && h.Status.Err() != nil {
- return h.Status.Err()
+ return corderrors.RpcErrorWithIdToCordError(h.Status.Err(), modelName, id)
}
_, err = dynamic.AsDynamicMessage(h.Response)
diff --git a/commands/services.go b/commands/services.go
index cc99dc5..19d4fd0 100644
--- a/commands/services.go
+++ b/commands/services.go
@@ -21,6 +21,7 @@
"github.com/fullstorydev/grpcurl"
flags "github.com/jessevdk/go-flags"
"github.com/jhump/protoreflect/dynamic"
+ corderrors "github.com/opencord/cordctl/error"
)
const (
@@ -62,11 +63,11 @@
h := &RpcEventHandler{}
err = grpcurl.InvokeRPC(ctx, descriptor, conn, "xos.dynamicload.GetLoadStatus", headers, h, h.GetParams)
if err != nil {
- return err
+ return corderrors.RpcErrorToCordError(err)
}
if h.Status != nil && h.Status.Err() != nil {
- return h.Status.Err()
+ return corderrors.RpcErrorToCordError(h.Status.Err())
}
d, err := dynamic.AsDynamicMessage(h.Response)
diff --git a/commands/status.go b/commands/status.go
index 0924db4..6527371 100644
--- a/commands/status.go
+++ b/commands/status.go
@@ -21,6 +21,7 @@
"github.com/fullstorydev/grpcurl"
flags "github.com/jessevdk/go-flags"
"github.com/jhump/protoreflect/dynamic"
+ corderrors "github.com/opencord/cordctl/error"
"strings"
)
@@ -62,11 +63,11 @@
h := &RpcEventHandler{}
err = grpcurl.InvokeRPC(ctx, descriptor, conn, "xos.utility.GetDatabaseInfo", headers, h, h.GetParams)
if err != nil {
- return err
+ return corderrors.RpcErrorToCordError(err)
}
if h.Status != nil && h.Status.Err() != nil {
- return h.Status.Err()
+ return corderrors.RpcErrorToCordError(h.Status.Err())
}
d, err := dynamic.AsDynamicMessage(h.Response)
diff --git a/error/error.go b/error/error.go
index c30096b..e35a14a 100644
--- a/error/error.go
+++ b/error/error.go
@@ -49,7 +49,7 @@
"bytes"
"fmt"
go_errors "github.com/go-errors/errors"
- "os"
+ "google.golang.org/grpc/status"
"runtime"
"strings"
)
@@ -58,9 +58,6 @@
MaxStackDepth = 50
)
-// Prefix applied to all error messages. Initialized in module init() function from os.Args[0]
-var prefix string
-
/* CordCtlError is the interface for errors created by cordctl.
* ShouldDumpStack()
* Returns false for well-understood problems such as invalid user input where a brief error message is sufficient
@@ -77,14 +74,68 @@
AddStackTrace(skip int)
}
-/* BaseError supports attaching stack traces to errors
+/* ObjectReference contains information about the object that the error applies to.
+ This may be empty (ModelName="") or it may contain a ModelName together with
+ option Id or Queries.
+*/
+
+type ObjectReference struct {
+ ModelName string
+ Id int32
+ Queries map[string]string
+}
+
+// Returns true if the reference is populated
+func (f *ObjectReference) IsValid() bool {
+ return (f.ModelName != "")
+}
+
+func (f *ObjectReference) String() string {
+ if !f.IsValid() {
+ // The reference is empty
+ return ""
+ }
+
+ if f.Queries != nil {
+ kv := make([]string, 0, len(f.Queries))
+ for k, v := range f.Queries {
+ kv = append(kv, fmt.Sprintf("%s%s", k, v))
+ }
+ return fmt.Sprintf("%s <%v>", f.ModelName, strings.Join(kv, ", "))
+ }
+
+ if f.Id > 0 {
+ return fmt.Sprintf("%s <id=%d>", f.ModelName, f.Id)
+ }
+
+ return fmt.Sprintf("%s", f.ModelName)
+}
+
+// Returns " on model ModelName [id]" if the reference is populated, or "" otherwise.
+func (f *ObjectReference) Clause() string {
+ if !f.IsValid() {
+ // The reference is empty
+ return ""
+ }
+
+ return fmt.Sprintf(" [on model %s]", f.String())
+}
+
+/* BaseError
+ *
+ * Supports attaching stack traces to errors
* Borrowed the technique from github.com/go-errors. Decided against using go-errors directly since it requires
* wrapping our error classes. Instead, incorporated the stack trace directly into our error class.
+ *
+ * Also supports encapsulating error messages, so that a CordError can encapsulate the error message from a
+ * function that was called.
*/
type BaseError struct {
- stack []uintptr
- frames []go_errors.StackFrame
+ Obj ObjectReference
+ Encapsulated error // in case this error encapsulates an error from a lower level
+ stack []uintptr // for stack trace
+ frames []go_errors.StackFrame // for stack trace
}
func (f *BaseError) AddStackTrace(skip int) {
@@ -163,9 +214,9 @@
func (f ChecksumMismatchError) Error() string {
if f.Name != "" {
- return fmt.Sprintf("%s %s: checksum mismatch (actual=%s, expected=%s)", prefix, f.Name, f.Expected, f.Actual)
+ return fmt.Sprintf("%s: checksum mismatch (actual=%s, expected=%s)", f.Name, f.Expected, f.Actual)
} else {
- return fmt.Sprintf("%s: checksum mismatch (actual=%s, expected=%s)", prefix, f.Expected, f.Actual)
+ return fmt.Sprintf("checksum mismatch (actual=%s, expected=%s)", f.Expected, f.Actual)
}
}
@@ -176,7 +227,7 @@
}
func (f UnknownModelTypeError) Error() string {
- return fmt.Sprintf("%s: Model %s does not exist. Use `cordctl modeltype list` to get a list of available models", prefix, f.Name)
+ return fmt.Sprintf("Model %s does not exist. Use `cordctl modeltype list` to get a list of available models", f.Name)
}
// User specified a model state that is not valid
@@ -186,7 +237,7 @@
}
func (f UnknownModelStateError) Error() string {
- return fmt.Sprintf("%s: Model state %s does not exist", prefix, f.Name)
+ return fmt.Sprintf("Model state %s does not exist", f.Name)
}
// Command requires a filter be specified
@@ -224,7 +275,7 @@
}
func (f FieldDoesNotExistError) Error() string {
- return fmt.Sprintf("%s: Model %s does not have field %s", prefix, f.ModelName, f.FieldName)
+ return fmt.Sprintf("Model %s does not have field %s", f.ModelName, f.FieldName)
}
// User specified a query string that is not properly formatted
@@ -234,7 +285,7 @@
}
func (f IllegalQueryError) Error() string {
- return fmt.Sprintf("%s: Illegal query string %s", prefix, f.Query)
+ return fmt.Sprintf("Illegal query string %s", f.Query)
}
// We failed to type convert something that we thought should have converted
@@ -245,7 +296,7 @@
}
func (f TypeConversionError) Error() string {
- return fmt.Sprintf("%s: Failed to type convert from %s to %s", prefix, f.Source, f.Destination)
+ return fmt.Sprintf("Failed to type convert from %s to %s", f.Source, f.Destination)
}
// Version did not match a constraint
@@ -257,23 +308,25 @@
}
func (f VersionConstraintError) Error() string {
- return fmt.Sprintf("%s: %s version %s did not match constraint '%s'", prefix, f.Name, f.Version, f.Constraint)
+ return fmt.Sprintf("%s version %s did not match constraint '%s'", f.Name, f.Version, f.Constraint)
}
// A model was not found
type ModelNotFoundError struct {
UserError
- ModelName string
- Id int32
- Queries map[string]string
}
func (f ModelNotFoundError) Error() string {
- if f.Queries != nil {
- return fmt.Sprintf("%s: %s query %v not Found", prefix, f.ModelName, f.Queries)
- } else {
- return fmt.Sprintf("%s: Model %s id %d not Found", prefix, f.ModelName, f.Id)
- }
+ return fmt.Sprintf("Not Found%s", f.Obj.Clause())
+}
+
+// Permission Denied
+type PermissionDeniedError struct {
+ UserError
+}
+
+func (f PermissionDeniedError) Error() string {
+ return fmt.Sprintf("Permission Denied%s. Please verify username and password are correct", f.Obj.Clause())
}
// InvalidInputError is a catch-all for user mistakes that aren't covered elsewhere
@@ -283,7 +336,7 @@
}
func (f InvalidInputError) Error() string {
- return fmt.Sprintf("%s: %s", prefix, f.Message)
+ return fmt.Sprintf("%s", f.Message)
}
func NewInvalidInputError(format string, params ...interface{}) *InvalidInputError {
@@ -300,7 +353,7 @@
}
func (f InternalError) Error() string {
- return fmt.Sprintf("%s: %s", prefix, f.Message)
+ return fmt.Sprintf("Internal Error%s: %s", f.Obj.Clause(), f.Message)
}
func NewInternalError(format string, params ...interface{}) *InternalError {
@@ -320,25 +373,63 @@
return err
}
-// Set the prefix rather than using os.Args[0]. This is useful for testing.
-func SetPrefix(s string) {
- prefix = s
-}
+/* RpcErrorWithObjToCordError
+ *
+ * Convert an RPC error into a Cord Error. The ObjectReference allows methods to attach
+ * object-related information to the error, and this varies by method. For example the Delete()
+ * method comes with an ModelName and an Id. The List() method has only a ModelName.
+ *
+ * Stubs (RpcErrorWithModelNameToCordError) are provided below to make common usage more convenient.
+ */
-// Convert an RPC error into a Cord Error
-func RpcErrorWithIdToCordError(err error, modelName string, id int32) error {
+func RpcErrorWithObjToCordError(err error, obj ObjectReference) error {
if err == nil {
return err
}
- if strings.Contains(err.Error(), "rpc error: code = NotFound") {
- err := &ModelNotFoundError{ModelName: modelName, Id: id}
- err.AddStackTrace(2)
- return err
+
+ st, ok := status.FromError(err)
+ if ok {
+ switch st.Code().String() {
+ case "PermissionDenied":
+ cordErr := &PermissionDeniedError{}
+ cordErr.Obj = obj
+ cordErr.Encapsulated = err
+ cordErr.AddStackTrace(2)
+ return cordErr
+ case "NotFound":
+ cordErr := &ModelNotFoundError{}
+ cordErr.Obj = obj
+ cordErr.Encapsulated = err
+ cordErr.AddStackTrace(2)
+ return cordErr
+ case "Unknown":
+ msg := st.Message()
+ if strings.HasPrefix(msg, "Exception calling application: ") {
+ msg = msg[31:]
+ }
+ cordErr := &InternalError{Message: msg}
+ cordErr.Obj = obj
+ cordErr.Encapsulated = err
+ cordErr.AddStackTrace(2)
+ return cordErr
+ }
}
+
return err
}
-// Module initialization. Automatically defaults prefix to program name
-func init() {
- prefix = os.Args[0]
+func RpcErrorToCordError(err error) error {
+ return RpcErrorWithObjToCordError(err, ObjectReference{})
+}
+
+func RpcErrorWithModelNameToCordError(err error, modelName string) error {
+ return RpcErrorWithObjToCordError(err, ObjectReference{ModelName: modelName})
+}
+
+func RpcErrorWithIdToCordError(err error, modelName string, id int32) error {
+ return RpcErrorWithObjToCordError(err, ObjectReference{ModelName: modelName, Id: id})
+}
+
+func RpcErrorWithQueriesToCordError(err error, modelName string, queries map[string]string) error {
+ return RpcErrorWithObjToCordError(err, ObjectReference{ModelName: modelName, Queries: queries})
}
diff --git a/error/error_test.go b/error/error_test.go
index 072b74e..3030e0d 100644
--- a/error/error_test.go
+++ b/error/error_test.go
@@ -19,13 +19,11 @@
import (
"fmt"
"github.com/stretchr/testify/assert"
+ "google.golang.org/grpc/codes"
+ "google.golang.org/grpc/status"
"testing"
)
-func init() {
- SetPrefix("cordctl")
-}
-
func TestGenericError(t *testing.T) {
var err error
@@ -45,10 +43,8 @@
err = WithStackTrace(&ChecksumMismatchError{Actual: "123", Expected: "456"})
- //assert.Equal(t, err.(*ChecksumMismatchError).Stack(), "foo")
-
// Check that the Error() function returns the right text
- assert.Equal(t, err.Error(), "cordctl: checksum mismatch (actual=456, expected=123)")
+ assert.Equal(t, err.Error(), "checksum mismatch (actual=456, expected=123)")
// Type conversion from `error` to ChecksumMismatchError should succeed
_, ok := err.(*ChecksumMismatchError)
@@ -77,5 +73,77 @@
err = WithStackTrace(&UnknownModelTypeError{Name: "foo"})
- _ = err
+ // Check that the Error() function returns the right text
+ assert.Equal(t, err.Error(), "Model foo does not exist. Use `cordctl modeltype list` to get a list of available models")
+}
+
+func TestRpcErrorToCordError(t *testing.T) {
+ // InternalError
+ err := status.Error(codes.Unknown, "A fake Unknown error")
+
+ cordErr := RpcErrorToCordError(err)
+
+ _, ok := cordErr.(*InternalError)
+ assert.True(t, ok)
+ assert.Equal(t, cordErr.Error(), "Internal Error: A fake Unknown error")
+
+ // NotFound
+ err = status.Error(codes.NotFound, "A fake not found error")
+
+ cordErr = RpcErrorToCordError(err)
+
+ _, ok = cordErr.(*ModelNotFoundError)
+ assert.True(t, ok)
+ assert.Equal(t, cordErr.Error(), "Not Found")
+
+ // PermissionDeniedError
+ err = status.Error(codes.PermissionDenied, "A fake Permission error")
+
+ cordErr = RpcErrorToCordError(err)
+
+ _, ok = cordErr.(*PermissionDeniedError)
+ assert.True(t, ok)
+ assert.Equal(t, cordErr.Error(), "Permission Denied. Please verify username and password are correct")
+}
+
+func TestRpcErrorWithModelNameToCordError(t *testing.T) {
+ // InternalError
+ err := status.Error(codes.Unknown, "A fake Unknown error")
+
+ cordErr := RpcErrorWithModelNameToCordError(err, "Foo")
+
+ _, ok := cordErr.(*InternalError)
+ assert.True(t, ok)
+ assert.Equal(t, cordErr.Error(), "Internal Error [on model Foo]: A fake Unknown error")
+}
+
+func TestRpcErrorWithIdToCordError(t *testing.T) {
+ // InternalError
+ err := status.Error(codes.Unknown, "A fake Unknown error")
+
+ cordErr := RpcErrorWithIdToCordError(err, "Foo", 7)
+
+ _, ok := cordErr.(*InternalError)
+ assert.True(t, ok)
+ assert.Equal(t, cordErr.Error(), "Internal Error [on model Foo <id=7>]: A fake Unknown error")
+}
+
+func TestRpcErrorWithQueriesToCordError(t *testing.T) {
+ // InternalError
+ err := status.Error(codes.Unknown, "A fake Unknown error")
+
+ cordErr := RpcErrorWithQueriesToCordError(err, "Foo", map[string]string{"id": "=3"})
+
+ _, ok := cordErr.(*InternalError)
+ assert.True(t, ok)
+ assert.Equal(t, cordErr.Error(), "Internal Error [on model Foo <id=3>]: A fake Unknown error")
+}
+
+func TestStackTrace(t *testing.T) {
+ var err error
+
+ err = WithStackTrace(&UnknownModelTypeError{Name: "foo"})
+
+ // goexit occurs near the end of the stack trace
+ assert.Contains(t, err.(CordCtlError).Stack(), "goexit")
}