-
Notifications
You must be signed in to change notification settings - Fork 13
fix: Fix data flag JSON error handling #214
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems fine now. |
||
| 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)) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably change |
||
| } | ||
|
|
||
| output, err := output.CmdOutput("create", viper.GetString(cliflags.OutputFlag), response) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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"), | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated these based on the test descriptions. |
||
| []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{}, | ||
| ) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The output has extra spacing if we don't do this. |
||
|
|
||
| return string(formattedOutput) | ||
| } | ||
|
|
||
| return ErrorPlaintextOutputFn(r) | ||
| } | ||
|
|
||
| func errJSON(s string) string { | ||
| return fmt.Sprintf( | ||
| `{ | ||
| "message": %q | ||
| }`, | ||
| s, | ||
| ) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used this new function here and for creating members. Another PR will add it elsewhere.