From 8d534f6803a45b1fca342366aa62e0cd8e67aed9 Mon Sep 17 00:00:00 2001
From: Jeff Mitchell <jeffrey.mitchell@gmail.com>
Date: Mon, 5 Oct 2020 23:54:56 -0400
Subject: [PATCH] Implementation and single-command PoC of proposed update to
 API error format

---
 api/apierror.go                     | 24 +--------------
 internal/cmd/base/format.go         | 45 +++++++++++++++++++++++++++++
 internal/cmd/commands/roles/role.go | 21 ++++++++++----
 3 files changed, 62 insertions(+), 28 deletions(-)

diff --git a/api/apierror.go b/api/apierror.go
index 146e638189..3af1255408 100644
--- a/api/apierror.go
+++ b/api/apierror.go
@@ -2,9 +2,7 @@ package api
 
 import (
 	"errors"
-	"fmt"
 	"net/http"
-	"strings"
 
 	"google.golang.org/grpc/codes"
 )
@@ -28,27 +26,7 @@ func AsServerError(in error) *Error {
 
 // Error satisfies the error interface.
 func (e *Error) Error() string {
-	res := fmt.Sprintf("Status: %d, Code: %q, Error: %q", e.Status, e.Code, e.Message)
-	var dets []string
-	if e.Details != nil {
-		if e.Details.ErrorId != "" {
-			dets = append(dets, fmt.Sprintf("error_id: %q", e.Details.ErrorId))
-		}
-		if e.Details.RequestId != "" {
-			dets = append(dets, fmt.Sprintf("request_id: %q", e.Details.RequestId))
-		}
-		if e.Details.TraceId != "" {
-			dets = append(dets, fmt.Sprintf("TraceId: %q", e.Details.TraceId))
-		}
-		for _, rf := range e.Details.RequestFields {
-			dets = append(dets, fmt.Sprintf("request_field: {name: %q, desc: %q}", rf.Name, rf.Description))
-		}
-	}
-	if len(dets) > 0 {
-		det := strings.Join(dets, ", ")
-		res = fmt.Sprintf("%s, Details: {%s}", res, det)
-	}
-	return res
+	return e.responseBody.String()
 }
 
 // Errors are considered the same iff they are both api.Errors and their statuses are the same.
diff --git a/internal/cmd/base/format.go b/internal/cmd/base/format.go
index 74aa4f740a..2f4286cd2e 100644
--- a/internal/cmd/base/format.go
+++ b/internal/cmd/base/format.go
@@ -9,6 +9,7 @@ import (
 	"unicode"
 	"unicode/utf8"
 
+	"github.com/hashicorp/boundary/api"
 	"github.com/hashicorp/boundary/api/scopes"
 	"github.com/mitchellh/cli"
 	"github.com/mitchellh/go-wordwrap"
@@ -133,6 +134,50 @@ func WrapMap(prefixSpaces, maxLengthOverride int, input map[string]interface{})
 	return strings.Join(ret, "\n")
 }
 
+func PrintApiError(in *api.Error) string {
+	nonAttributeMap := map[string]interface{}{
+		"Status":  in.Status,
+		"Code":    in.Code,
+		"Message": in.Message,
+	}
+	if in.Details != nil {
+		if in.Details.TraceId != "" {
+			nonAttributeMap["Trace ID"] = in.Details.TraceId
+		}
+		if in.Details.RequestId != "" {
+			nonAttributeMap["Request ID"] = in.Details.RequestId
+		}
+		if in.Details.ErrorId != "" {
+			nonAttributeMap["Error ID"] = in.Details.ErrorId
+		}
+	}
+
+	maxLength := MaxAttributesLength(nonAttributeMap, nil, nil)
+
+	ret := []string{
+		"",
+		"Error information:",
+		WrapMap(2, maxLength+2, nonAttributeMap),
+	}
+
+	if in.Details != nil {
+		if len(in.Details.RequestFields) > 0 {
+			ret = append(ret,
+				"",
+				"  Field-specific Errors:",
+			)
+			for _, field := range in.Details.RequestFields {
+				ret = append(ret,
+					fmt.Sprintf("    Name:              %s", strings.ReplaceAll(field.Name, "_", "-")),
+					fmt.Sprintf("      Error:           %s", field.Description),
+				)
+			}
+		}
+	}
+
+	return WrapForHelpText(ret)
+}
+
 // An output formatter for json output of an object
 type JsonFormatter struct{}
 
diff --git a/internal/cmd/commands/roles/role.go b/internal/cmd/commands/roles/role.go
index 377f094e2e..df3ce508fd 100644
--- a/internal/cmd/commands/roles/role.go
+++ b/internal/cmd/commands/roles/role.go
@@ -244,12 +244,23 @@ func (c *Command) Run(args []string) int {
 		plural = "roles"
 	}
 	if err != nil {
-		if api.AsServerError(err) != nil {
-			c.UI.Error(fmt.Sprintf("Error from controller when performing %s on %s: %s", c.Func, plural, err.Error()))
-			return 1
+		switch base.Format(c.UI) {
+		case "json":
+			if apiErr := api.AsServerError(err); apiErr != nil {
+				c.UI.Error(apiErr.Error())
+				return 1
+			}
+			c.UI.Error(fmt.Sprintf(`{"error": %q}`, err.Error()))
+			return 2
+
+		default:
+			if apiErr := api.AsServerError(err); apiErr != nil {
+				c.UI.Error(fmt.Sprintf("Error from controller when performing %s on %s\n%s", c.Func, plural, base.PrintApiError(apiErr)))
+				return 1
+			}
+			c.UI.Error(fmt.Sprintf("Error trying to %s %s: %s", c.Func, plural, err.Error()))
+			return 2
 		}
-		c.UI.Error(fmt.Sprintf("Error trying to %s %s: %s", c.Func, plural, err.Error()))
-		return 2
 	}
 
 	switch c.Func {