libopenapi icon indicating copy to clipboard operation
libopenapi copied to clipboard

Convert `[]error` to use multi errors available in 1.20

Open thrawn01 opened this issue 2 years ago • 5 comments

Thanks for the wonderful library! After converting from a different library, I have a single suggestion!

The current code base returns multiple errors which is inconvenient to use if multiple errors are not useful to the caller. We could instead create an error that implements the Unwrap() []error method which can be used to retrieve multiple errors.

// Existing signature
RenderAndReload() ([]byte, Document, *DocumentModel[v3high.Document], []error)
// Would become
RenderAndReload() ([]byte, Document, *DocumentModel[v3high.Document], error)

Example implementation

type MultiError struct {
	errs []error
}

func (e *MultiError) Error() string {
	var b []byte
	for i, err := range e.errs {
		if i > 0 {
			b = append(b, '\n')
		}
		b = append(b, err.Error()...)
	}
	return string(b)
}

func (e *MultiError) Unwrap() []error {
	return e.errs
}

The conversion would be easy, but would break the compatibility. As such it should probably wait for a v1 or v2 release.

See https://pkg.go.dev/errors#Join

thrawn01 avatar May 17 '23 19:05 thrawn01

Ah yes, I thought the same thing when I saw the release notes for 1.20.

Yes this is a good idea, adding it to the backlog.

daveshanley avatar May 18 '23 09:05 daveshanley

@daveshanley is this still the approach you want to be followed? If so, I can work on this.

n0rig avatar Feb 18 '24 22:02 n0rig

I'd suggest just using errors.Join and return a single error.

jxsl13 avatar Feb 18 '24 23:02 jxsl13

I would avoid working on this until the library is ready for 1.0, it's a major breaking change to the top level of the library in terms of signature. It's a small change with big consequences.

daveshanley avatar Feb 20 '24 16:02 daveshanley

@daveshanley I would suggest doing it ASAP as that will result in less people having to alter their code.

stevenh avatar May 24 '24 13:05 stevenh