Skip to content
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

Allow marshalling a multi-error #15

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ConradIrwin
Copy link

Without this change any objects containing a multierror will fail to
Marshal, though most objects that implement the Error() interface
are safe (albeit not useful) to convert to JSON.

This pull request goes one step further and tries to make the JSON
output somewhat useful, and defines a way to reconstruct a
multierror that looks superficially similar to the original when
Unmarshalled again.

Without this change any objects containing a multierror will fail to
Marshal, though most objects that implement the Error() interface
are safe (albeit not useful) to convert to JSON.

This pull request goes one step further and tries to make the JSON
output somewhat useful, and defines a way to reconstruct a
multierror that looks superficially similar to the original when
Unmarshalled again.
@ConradIrwin ConradIrwin mentioned this pull request Sep 6, 2017
@hashicorp-cla
Copy link

hashicorp-cla commented Mar 26, 2019

CLA assistant check
All committers have signed the CLA.

@theherk
Copy link

theherk commented Oct 11, 2019

I don't think he is going to complete that cla, so I'm going to rework this so we can get it in.

theherk added a commit to theherk/go-multierror that referenced this pull request Oct 11, 2019
This is based off hashicorp#15, which was never merged due to cla.
@theherk
Copy link

theherk commented Oct 11, 2019

This can be closed if #25 is merged.

@ConradIrwin
Copy link
Author

fixed — sorry for missing that! Also happy for your version to be merged instead

@theherk
Copy link

theherk commented Oct 11, 2019

Ah, no sweat. I didn't think you'd jump on it so quickly after two years, else I wouldn't have done that. Yeah, I hope you get merged up.

@mentalisttraceur
Copy link

Suggestion: marshal to just the inner list, instead of wrapping it in an {"errors": } object.

(Disclaimer: I'm not affiliated with Hashicorp in any way, I'm just a rando on the internet with Ideas(TM) and the hubris to think they're worth saying.)

Consider the difference between:

{ "response": null, "errors": [ "foo", "bar" ] }
{ "response": null, "errors": { "errors": [ "foo", "bar" ] } }

Basically:

  1. don't we usually marshal data like a list of errors into a bigger surrounding JSON blob anyway?

  2. isn't that surrounding context going to have its own "errors" key or other indicator that the data is a list of errors?

theherk added a commit to theherk/go-multierror that referenced this pull request Dec 30, 2019
This is based off hashicorp#15, which was never merged due to cla.
theherk added a commit to theherk/go-multierror that referenced this pull request Apr 29, 2021
This is based off hashicorp#15, which was never merged due to cla.
theherk added a commit to theherk/go-multierror that referenced this pull request Apr 29, 2021
This is based off hashicorp#15, which was never merged due to cla.
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.

None yet

4 participants