Skip to content

fix: Fix data flag JSON error handling#214

Merged
dbolson merged 2 commits into
mainfrom
sc-241911/better-err-handling
Apr 29, 2024
Merged

fix: Fix data flag JSON error handling#214
dbolson merged 2 commits into
mainfrom
sc-241911/better-err-handling

Conversation

@dbolson
Copy link
Copy Markdown
Contributor

@dbolson dbolson commented Apr 29, 2024

Running the members create command with invalid JSON would show a bad error:

json: cannot unmarshal object into Go value of type []members.MemberInput

Now it shows a nicer one:

$ ldcli members create --data '{"email": "friend+11@launchdarkly.com", "role": "writer"}'
invalid JSON
$ ldcli members create --data '{"email": "friend+11@launchdarkly.com", "role": "writer"}' --output json
{"message":"invalid JSON"}

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Related issues

Provide links to any issues in this repository or elsewhere relating to this pull request.

Describe the solution you've provided

Provide a clear and concise description of what you expect to happen.

Describe alternatives you've considered

Provide a clear and concise description of any alternative solutions or features you've considered.

Additional context

Add any other context about the pull request here.

Comment thread cmd/flags/get.go
}

return errors.NewError(output)
return errors.NewError(output.CmdOutputError(viper.GetString(cliflags.OutputFlag), err))
Copy link
Copy Markdown
Contributor Author

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.

Comment thread cmd/members/create.go
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)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine now.

Comment thread cmd/members/create.go
}

return errors.NewError(output)
return errors.NewError(output.CmdOutputError(viper.GetString(cliflags.OutputFlag), err))
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably change CmdOutput to only return a string, not a possible error.

underlying := errs.NewAPIError(
[]byte(`{"code":"conflict","message":"an error"}`),
errors.New("409 Conflict"),
errors.New("400 an error"),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated these based on the test descriptions.


if outputKind == "json" {
// convert to a well-formatted output
formattedOutput, _ := json.Marshal(r)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The output has extra spacing if we don't do this.

@dbolson dbolson merged commit a469c0c into main Apr 29, 2024
@dbolson dbolson deleted the sc-241911/better-err-handling branch April 29, 2024 23:15
@sunnyguduru sunnyguduru mentioned this pull request Apr 30, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants