fix: Fix data flag JSON error handling (#214)

Fix data flag JSON error handling

cmd/environments/get.go

@@ -64,16 +64,7 @@ func runGet(
 			viper.GetString(cliflags.ProjectFlag),
 		)
 		if err != nil {
-			output, err := output.CmdOutputSingular(
-				viper.GetString(cliflags.OutputFlag),
-				[]byte(err.Error()),
-				output.ErrorPlaintextOutputFn,
-			)
-			if err != nil {
-				return errors.NewError(err.Error())
-			}
-
-			return errors.NewError(output)
+			return errors.NewError(output.CmdOutputError(viper.GetString(cliflags.OutputFlag), err))
 		}
 
 		output, err := output.CmdOutput("get", viper.GetString(cliflags.OutputFlag), response)

cmd/flags/create.go

@@ -57,7 +57,7 @@ func runCreate(client flags.Client) func(*cobra.Command, []string) error {
 		var data inputData
 		err := json.Unmarshal([]byte(viper.GetString(cliflags.DataFlag)), &data)
 		if err != nil {
-			return err
+			return errors.NewError(output.CmdOutputError(viper.GetString(cliflags.OutputFlag), err))
 		}
 
 		response, err := client.Create(
@@ -69,16 +69,7 @@ func runCreate(client flags.Client) func(*cobra.Command, []string) error {
 			viper.GetString(cliflags.ProjectFlag),
 		)
 		if err != nil {
-			output, err := output.CmdOutputSingular(
-				viper.GetString(cliflags.OutputFlag),
-				[]byte(err.Error()),
-				output.ErrorPlaintextOutputFn,
-			)
-			if err != nil {
-				return errors.NewError(err.Error())
-			}
-
-			return errors.NewError(output)
+			return errors.NewError(output.CmdOutputError(viper.GetString(cliflags.OutputFlag), err))
 		}
 
 		output, err := output.CmdOutput("create", viper.GetString(cliflags.OutputFlag), response)

cmd/flags/get.go

@@ -67,16 +67,7 @@ func runGet(client flags.Client) func(*cobra.Command, []string) error {
 			viper.GetString(cliflags.EnvironmentFlag),
 		)
 		if err != nil {
-			output, err := output.CmdOutputSingular(
-				viper.GetString(cliflags.OutputFlag),
-				[]byte(err.Error()),
-				output.ErrorPlaintextOutputFn,
-			)
-			if err != nil {
-				return errors.NewError(err.Error())
-			}
-
-			return errors.NewError(output)
+			return errors.NewError(output.CmdOutputError(viper.GetString(cliflags.OutputFlag), err))
 		}
 
 		output, err := output.CmdOutput("get", viper.GetString(cliflags.OutputFlag), response)

cmd/flags/update.go

@@ -125,7 +125,7 @@ func runUpdate(client flags.Client) func(*cobra.Command, []string) error {
 		} else {
 			err := json.Unmarshal([]byte(viper.GetString(cliflags.DataFlag)), &patch)
 			if err != nil {
-				return err
+				return errors.NewError(output.CmdOutputError(viper.GetString(cliflags.OutputFlag), err))
 			}
 		}
 
@@ -138,16 +138,7 @@ func runUpdate(client flags.Client) func(*cobra.Command, []string) error {
 			patch,
 		)
 		if err != nil {
-			output, err := output.CmdOutputSingular(
-				viper.GetString(cliflags.OutputFlag),
-				[]byte(err.Error()),
-				output.ErrorPlaintextOutputFn,
-			)
-			if err != nil {
-				return errors.NewError(err.Error())
-			}
-
-			return errors.NewError(output)
+			return errors.NewError(output.CmdOutputError(viper.GetString(cliflags.OutputFlag), err))
 		}
 
 		output, err := output.CmdOutput("update", viper.GetString(cliflags.OutputFlag), response)

cmd/members/create.go

@@ -40,10 +40,9 @@ func NewCreateCmd(client members.Client) (*cobra.Command, error) {
 func runCreate(client members.Client) func(*cobra.Command, []string) error {
 	return func(cmd *cobra.Command, args []string) error {
 		var data []members.MemberInput
-		// TODO: why does viper.GetString(cliflags.DataFlag) not work?
-		err := json.Unmarshal([]byte(cmd.Flags().Lookup(cliflags.DataFlag).Value.String()), &data)
+		err := json.Unmarshal([]byte(viper.GetString(cliflags.DataFlag)), &data)
 		if err != nil {
-			return errors.NewError(err.Error())
+			return errors.NewError(output.CmdOutputError(viper.GetString(cliflags.OutputFlag), err))
 		}
 
 		response, err := client.Create(
@@ -53,16 +52,7 @@ func runCreate(client members.Client) func(*cobra.Command, []string) error {
 			data,
 		)
 		if err != nil {
-			output, err := output.CmdOutputSingular(
-				viper.GetString(cliflags.OutputFlag),
-				[]byte(err.Error()),
-				output.ErrorPlaintextOutputFn,
-			)
-			if err != nil {
-				return errors.NewError(err.Error())
-			}
-
-			return errors.NewError(output)
+			return errors.NewError(output.CmdOutputError(viper.GetString(cliflags.OutputFlag), err))
 		}
 
 		output, err := output.CmdOutput("create", viper.GetString(cliflags.OutputFlag), response)

cmd/members/invite.go

@@ -62,16 +62,7 @@ func runInvite(client members.Client) func(*cobra.Command, []string) error {
 			memberInputs,
 		)
 		if err != nil {
-			output, err := output.CmdOutputSingular(
-				viper.GetString(cliflags.OutputFlag),
-				[]byte(err.Error()),
-				output.ErrorPlaintextOutputFn,
-			)
-			if err != nil {
-				return errors.NewError(err.Error())
-			}
-
-			return errors.NewError(output)
+			return errors.NewError(output.CmdOutputError(viper.GetString(cliflags.OutputFlag), err))
 		}
 
 		output, err := output.CmdOutput("create", viper.GetString(cliflags.OutputFlag), response)

cmd/projects/create.go

@@ -48,7 +48,7 @@ func runCreate(client projects.Client) func(*cobra.Command, []string) error {
 		// TODO: why does viper.GetString(cliflags.DataFlag) not work?
 		err := json.Unmarshal([]byte(cmd.Flags().Lookup(cliflags.DataFlag).Value.String()), &data)
 		if err != nil {
-			return err
+			return errors.NewError(output.CmdOutputError(viper.GetString(cliflags.OutputFlag), err))
 		}
 
 		response, err := client.Create(
@@ -59,16 +59,7 @@ func runCreate(client projects.Client) func(*cobra.Command, []string) error {
 			data.Key,
 		)
 		if err != nil {
-			output, err := output.CmdOutputSingular(
-				viper.GetString(cliflags.OutputFlag),
-				[]byte(err.Error()),
-				output.ErrorPlaintextOutputFn,
-			)
-			if err != nil {
-				return errors.NewError(err.Error())
-			}
-
-			return errors.NewError(output)
+			return errors.NewError(output.CmdOutputError(viper.GetString(cliflags.OutputFlag), err))
 		}
 
 		output, err := output.CmdOutput("create", viper.GetString(cliflags.OutputFlag), response)

cmd/projects/list.go

@@ -34,16 +34,7 @@ func runList(client projects.Client) func(*cobra.Command, []string) error {
 			viper.GetString(cliflags.BaseURIFlag),
 		)
 		if err != nil {
-			output, err := output.CmdOutputSingular(
-				viper.GetString(cliflags.OutputFlag),
-				[]byte(err.Error()),
-				output.ErrorPlaintextOutputFn,
-			)
-			if err != nil {
-				return errors.NewError(err.Error())
-			}
-
-			return errors.NewError(output)
+			return errors.NewError(output.CmdOutputError(viper.GetString(cliflags.OutputFlag), err))
 		}
 
 		output, err := output.CmdOutput("list", viper.GetString(cliflags.OutputFlag), response)

internal/errors/errors_test.go

@@ -16,7 +16,7 @@ func TestAPIError(t *testing.T) {
 	t.Run("with a 400 error has a JSON response", func(t *testing.T) {
 		underlying := errs.NewAPIError(
 			[]byte(`{"code":"conflict","message":"an error"}`),
-			errors.New("409 Conflict"),
+			errors.New("400 an error"),
 			[]string{},
 		)
 
@@ -45,7 +45,7 @@ func TestAPIError(t *testing.T) {
 	t.Run("with a 403 error has a JSON response", func(t *testing.T) {
 		underlying := errs.NewAPIError(
 			[]byte(`{"code": "forbidden", "message": "an error"}`),
-			errors.New("409 Conflict"),
+			errors.New("403 Forbidden"),
 			[]string{},
 		)
 

internal/output/resource_output.go

@@ -4,10 +4,14 @@ import (
 	"encoding/json"
 	"fmt"
 	"strings"
+
+	"github.com/pkg/errors"
+
+	errs "ldcli/internal/errors"
 )
 
-// CmdOutput returns a response from a resource create action formatted based on the
-// output flag along with an optional message based on the action.
+// CmdOutput returns a response from a resource action formatted based on the output flag along with
+// an optional message based on the action.
 func CmdOutput(action string, outputKind string, input []byte) (string, error) {
 	if outputKind == "json" {
 		return string(input), nil
@@ -66,3 +70,38 @@ func plaintextOutput(out string, successMessage string) string {
 
 	return out
 }
+
+// CmdOutputError returns a response from a resource action error.
+func CmdOutputError(outputKind string, err error) string {
+	var output string
+	jsonErr := &json.UnmarshalTypeError{}
+	switch {
+	case errors.As(err, &jsonErr):
+		output = errJSON("invalid JSON")
+	case errors.As(err, &errs.Error{}):
+		output = err.Error()
+	default:
+		output = errJSON(err.Error())
+	}
+
+	var r resource
+	_ = json.Unmarshal([]byte(output), &r)
+
+	if outputKind == "json" {
+		// convert to a well-formatted output
+		formattedOutput, _ := json.Marshal(r)
+
+		return string(formattedOutput)
+	}
+
+	return ErrorPlaintextOutputFn(r)
+}
+
+func errJSON(s string) string {
+	return fmt.Sprintf(
+		`{
+			"message": %q
+		}`,
+		s,
+	)
+}

internal/output/resource_output_test.go

@@ -1,11 +1,14 @@
 package output_test
 
 import (
-	"ldcli/internal/output"
+	"encoding/json"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"ldcli/internal/errors"
+	"ldcli/internal/output"
 )
 
 func TestCmdOutput(t *testing.T) {
@@ -16,7 +19,7 @@ func TestCmdOutput(t *testing.T) {
 			"other": "other-value"
 		}`
 
-		t.Run("with json output", func(t *testing.T) {
+		t.Run("with JSON output", func(t *testing.T) {
 			t.Run("returns the JSON", func(t *testing.T) {
 				result, err := output.CmdOutput("create", "json", []byte(input))
 
@@ -48,7 +51,7 @@ func TestCmdOutput(t *testing.T) {
 			]
 		}`
 
-		t.Run("with json output", func(t *testing.T) {
+		t.Run("with JSON output", func(t *testing.T) {
 			t.Run("returns the JSON", func(t *testing.T) {
 				result, err := output.CmdOutput("create", "json", []byte(input))
 
@@ -80,7 +83,7 @@ func TestCmdOutput(t *testing.T) {
 			]
 		}`
 
-		t.Run("with json output", func(t *testing.T) {
+		t.Run("with JSON output", func(t *testing.T) {
 			t.Run("returns the JSON", func(t *testing.T) {
 				result, err := output.CmdOutput("create", "json", []byte(input))
 
@@ -107,7 +110,7 @@ func TestCmdOutput(t *testing.T) {
 			"name": "test-name"
 		}`
 
-		t.Run("with json output", func(t *testing.T) {
+		t.Run("with JSON output", func(t *testing.T) {
 			t.Run("does not return anything", func(t *testing.T) {
 				result, err := output.CmdOutput("delete", "json", []byte(""))
 
@@ -149,7 +152,7 @@ func TestCmdOutput(t *testing.T) {
 			"other": "other-value"
 		}`
 
-		t.Run("with json output", func(t *testing.T) {
+		t.Run("with JSON output", func(t *testing.T) {
 			t.Run("returns the JSON", func(t *testing.T) {
 				result, err := output.CmdOutput("update", "json", []byte(input))
 
@@ -170,3 +173,28 @@ func TestCmdOutput(t *testing.T) {
 		})
 	})
 }
+
+func TestCmdOutputError(t *testing.T) {
+	t.Run("with an API error", func(t *testing.T) {
+		t.Run("with JSON output", func(t *testing.T) {
+			expected := `{"code":"conflict", "message":"an error"}`
+			err := errors.NewError(`{"code":"conflict", "message":"an error"}`)
+
+			result := output.CmdOutputError("json", err)
+
+			assert.JSONEq(t, expected, result)
+		})
+
+		t.Run("with plaintext output", func(t *testing.T) {
+			expected := "invalid JSON"
+			type testType any
+			invalid := []byte(`{"invalid": true}`)
+			err := json.Unmarshal(invalid, &[]testType{})
+			require.Error(t, err)
+
+			result := output.CmdOutputError("plaintext", err)
+
+			assert.Equal(t, expected, result)
+		})
+	})
+}