From c4fcda170a1ad423872b283b31c335b51538bfe2 Mon Sep 17 00:00:00 2001 From: John Behm Date: Sun, 20 Aug 2023 21:45:05 +0200 Subject: [PATCH] decrease error handling complexity --- datamodel/high/v3/document_test.go | 6 ++-- datamodel/low/v2/swagger_test.go | 20 ++++++------ datamodel/low/v3/create_document_test.go | 22 ++++++------- document_test.go | 30 ++++++++--------- internal/errorutils/errorutils.go | 41 +++++++++++++----------- internal/errorutils/filter.go | 2 +- 6 files changed, 63 insertions(+), 58 deletions(-) diff --git a/datamodel/high/v3/document_test.go b/datamodel/high/v3/document_test.go index b8742bf7..360f62d5 100644 --- a/datamodel/high/v3/document_test.go +++ b/datamodel/high/v3/document_test.go @@ -386,7 +386,7 @@ func TestStripeAsDoc(t *testing.T) { info, _ := datamodel.ExtractSpecInfo(data) var err error lowDoc, err = lowv3.CreateDocumentFromConfig(info, datamodel.NewOpenDocumentConfiguration()) - assert.Len(t, errorutils.Unwrap(err), 3) + assert.Len(t, errorutils.ShallowUnwrap(err), 3) d := NewDocument(lowDoc) assert.NotNil(t, d) } @@ -397,7 +397,7 @@ func TestK8sAsDoc(t *testing.T) { var err error lowSwag, err := lowv2.CreateDocumentFromConfig(info, datamodel.NewOpenDocumentConfiguration()) d := v2.NewSwaggerDocument(lowSwag) - assert.Len(t, errorutils.Unwrap(err), 0) + assert.Len(t, errorutils.ShallowUnwrap(err), 0) assert.NotNil(t, d) } @@ -452,7 +452,7 @@ func TestCircularReferencesDoc(t *testing.T) { info, _ := datamodel.ExtractSpecInfo(data) var err error lowDoc, err = lowv3.CreateDocumentFromConfig(info, datamodel.NewOpenDocumentConfiguration()) - assert.Len(t, errorutils.Unwrap(err), 3) + assert.Len(t, errorutils.ShallowUnwrap(err), 3) d := NewDocument(lowDoc) assert.Len(t, d.Components.Schemas, 9) assert.Len(t, d.Index.GetCircularReferences(), 3) diff --git a/datamodel/low/v2/swagger_test.go b/datamodel/low/v2/swagger_test.go index 8ca3ea93..96eeecc1 100644 --- a/datamodel/low/v2/swagger_test.go +++ b/datamodel/low/v2/swagger_test.go @@ -193,7 +193,7 @@ func TestCreateDocument_ExternalDocsBad(t *testing.T) { wait = false } } - assert.Len(t, errorutils.Unwrap(err), 1) + assert.Len(t, errorutils.ShallowUnwrap(err), 1) } func TestCreateDocument_TagsBad(t *testing.T) { @@ -211,7 +211,7 @@ func TestCreateDocument_TagsBad(t *testing.T) { wait = false } } - assert.Len(t, errorutils.Unwrap(err), 1) + assert.Len(t, errorutils.ShallowUnwrap(err), 1) } func TestCreateDocument_PathsBad(t *testing.T) { @@ -233,7 +233,7 @@ func TestCreateDocument_PathsBad(t *testing.T) { wait = false } } - assert.Len(t, errorutils.Unwrap(err), 1) + assert.Len(t, errorutils.ShallowUnwrap(err), 1) } func TestCreateDocument_SecurityBad(t *testing.T) { @@ -251,7 +251,7 @@ func TestCreateDocument_SecurityBad(t *testing.T) { wait = false } } - assert.Len(t, errorutils.Unwrap(err), 1) + assert.Len(t, errorutils.ShallowUnwrap(err), 1) } func TestCreateDocument_SecurityDefinitionsBad(t *testing.T) { @@ -269,7 +269,7 @@ func TestCreateDocument_SecurityDefinitionsBad(t *testing.T) { wait = false } } - assert.Len(t, errorutils.Unwrap(err), 1) + assert.Len(t, errorutils.ShallowUnwrap(err), 1) } func TestCreateDocument_ResponsesBad(t *testing.T) { @@ -287,7 +287,7 @@ func TestCreateDocument_ResponsesBad(t *testing.T) { wait = false } } - assert.Len(t, errorutils.Unwrap(err), 1) + assert.Len(t, errorutils.ShallowUnwrap(err), 1) } func TestCreateDocument_ParametersBad(t *testing.T) { @@ -305,7 +305,7 @@ func TestCreateDocument_ParametersBad(t *testing.T) { wait = false } } - assert.Len(t, errorutils.Unwrap(err), 1) + assert.Len(t, errorutils.ShallowUnwrap(err), 1) } func TestCreateDocument_DefinitionsBad(t *testing.T) { @@ -323,7 +323,7 @@ func TestCreateDocument_DefinitionsBad(t *testing.T) { wait = false } } - assert.Len(t, errorutils.Unwrap(err), 1) + assert.Len(t, errorutils.ShallowUnwrap(err), 1) } func TestCreateDocument_InfoBad(t *testing.T) { @@ -341,7 +341,7 @@ func TestCreateDocument_InfoBad(t *testing.T) { wait = false } } - assert.Len(t, errorutils.Unwrap(err), 1) + assert.Len(t, errorutils.ShallowUnwrap(err), 1) } func TestCircularReferenceError(t *testing.T) { @@ -350,6 +350,6 @@ func TestCircularReferenceError(t *testing.T) { info, _ := datamodel.ExtractSpecInfo(data) circDoc, err := CreateDocument(info) assert.NotNil(t, circDoc) - assert.Len(t, errorutils.Unwrap(err), 3) + assert.Len(t, errorutils.ShallowUnwrap(err), 3) } diff --git a/datamodel/low/v3/create_document_test.go b/datamodel/low/v3/create_document_test.go index a5ab4ccf..1df4515e 100644 --- a/datamodel/low/v3/create_document_test.go +++ b/datamodel/low/v3/create_document_test.go @@ -76,7 +76,7 @@ func TestCircularReferenceError(t *testing.T) { AllowFileReferences: false, AllowRemoteReferences: false, }) - assert.Len(t, errorutils.Unwrap(err), 3) + assert.Len(t, errorutils.ShallowUnwrap(err), 3) // lower level packages return errors and potentially a document. assert.NotNil(t, circDoc) @@ -116,7 +116,7 @@ func TestCreateDocumentStripe(t *testing.T) { AllowRemoteReferences: false, BasePath: "/here", }) - assert.Len(t, errorutils.Unwrap(err), 3) + assert.Len(t, errorutils.ShallowUnwrap(err), 3) assert.Equal(t, "3.0.0", d.Version.Value) assert.Equal(t, "Stripe API", d.Info.Value.Title.Value) @@ -182,7 +182,7 @@ func TestCreateDocument_WebHooks_Error(t *testing.T) { AllowFileReferences: false, AllowRemoteReferences: false, }) - require.Len(t, errorutils.Unwrap(err), 1) + require.Len(t, errorutils.ShallowUnwrap(err), 1) } func TestCreateDocument_Servers(t *testing.T) { @@ -572,7 +572,7 @@ components: AllowRemoteReferences: false, }) require.NoError(t, err) - require.Len(t, errorutils.Unwrap(err), 0) + require.Len(t, errorutils.ShallowUnwrap(err), 0) ob := doc.Components.Value.FindSchema("bork").Value ob.Schema() @@ -591,7 +591,7 @@ webhooks: AllowFileReferences: false, AllowRemoteReferences: false, }) - require.Len(t, errorutils.Unwrap(err), 1) + require.Len(t, errorutils.ShallowUnwrap(err), 1) } func TestCreateDocument_Components_Error_Extract(t *testing.T) { @@ -607,7 +607,7 @@ components: AllowFileReferences: false, AllowRemoteReferences: false, }) - require.Len(t, errorutils.Unwrap(err), 1) + require.Len(t, errorutils.ShallowUnwrap(err), 1) } @@ -623,7 +623,7 @@ paths: AllowFileReferences: false, AllowRemoteReferences: false, }) - require.Len(t, errorutils.Unwrap(err), 1) + require.Len(t, errorutils.ShallowUnwrap(err), 1) } func TestCreateDocument_Tags_Errors(t *testing.T) { @@ -637,7 +637,7 @@ tags: AllowFileReferences: false, AllowRemoteReferences: false, }) - require.Len(t, errorutils.Unwrap(err), 1) + require.Len(t, errorutils.ShallowUnwrap(err), 1) } func TestCreateDocument_Security_Error(t *testing.T) { @@ -651,7 +651,7 @@ security: AllowFileReferences: false, AllowRemoteReferences: false, }) - require.Len(t, errorutils.Unwrap(err), 1) + require.Len(t, errorutils.ShallowUnwrap(err), 1) } func TestCreateDocument_ExternalDoc_Error(t *testing.T) { @@ -665,7 +665,7 @@ externalDocs: AllowFileReferences: false, AllowRemoteReferences: false, }) - require.Len(t, errorutils.Unwrap(err), 1) + require.Len(t, errorutils.ShallowUnwrap(err), 1) } func TestCreateDocument_YamlAnchor(t *testing.T) { @@ -746,7 +746,7 @@ func ExampleCreateDocument() { } func panicOnUnknown(err error) { - for _, err := range errorutils.Unwrap(err) { + for _, err := range errorutils.ShallowUnwrap(err) { var resolvErr *resolver.ResolvingError if errors.As(err, &resolvErr) && resolvErr.CircularReference != nil { continue diff --git a/document_test.go b/document_test.go index da7bbcab..8580f15a 100644 --- a/document_test.go +++ b/document_test.go @@ -24,7 +24,7 @@ func TestLoadDocument_Simple_V2(t *testing.T) { v2Doc, docErr := doc.BuildV2Model() require.Nil(t, docErr) - require.Len(t, errorutils.Unwrap(docErr), 0) + require.Len(t, errorutils.ShallowUnwrap(docErr), 0) require.NotNil(t, v2Doc) require.NotNil(t, doc.GetSpecInfo()) } @@ -36,7 +36,7 @@ func TestLoadDocument_Simple_V2_Error(t *testing.T) { assert.NoError(t, err) v2Doc, docErr := doc.BuildV3Model() - assert.Len(t, errorutils.Unwrap(docErr), 1) + assert.Len(t, errorutils.ShallowUnwrap(docErr), 1) assert.Nil(t, v2Doc) } @@ -50,7 +50,7 @@ definitions: require.NoError(t, err) v2Doc, docErr := doc.BuildV2Model() - require.Len(t, errorutils.Unwrap(docErr), 2) + require.Len(t, errorutils.ShallowUnwrap(docErr), 2) require.Nil(t, v2Doc) } @@ -61,7 +61,7 @@ func TestLoadDocument_Simple_V3_Error(t *testing.T) { assert.NoError(t, err) v2Doc, docErr := doc.BuildV2Model() - assert.Len(t, errorutils.Unwrap(docErr), 1) + assert.Len(t, errorutils.ShallowUnwrap(docErr), 1) assert.Nil(t, v2Doc) } @@ -69,14 +69,14 @@ func TestLoadDocument_Error_V2NoSpec(t *testing.T) { doc := new(document) // not how this should be instantiated. _, err := doc.BuildV2Model() - assert.Len(t, errorutils.Unwrap(err), 1) + assert.Len(t, errorutils.ShallowUnwrap(err), 1) } func TestLoadDocument_Error_V3NoSpec(t *testing.T) { doc := new(document) // not how this should be instantiated. _, err := doc.BuildV3Model() - assert.Len(t, errorutils.Unwrap(err), 1) + assert.Len(t, errorutils.ShallowUnwrap(err), 1) } func TestLoadDocument_Empty(t *testing.T) { @@ -93,7 +93,7 @@ func TestLoadDocument_Simple_V3(t *testing.T) { assert.Equal(t, "3.0.1", doc.GetVersion()) v3Doc, docErr := doc.BuildV3Model() - assert.Len(t, errorutils.Unwrap(docErr), 0) + assert.Len(t, errorutils.ShallowUnwrap(docErr), 0) assert.NotNil(t, v3Doc) } @@ -107,7 +107,7 @@ paths: assert.NoError(t, err) v3Doc, docErr := doc.BuildV3Model() - assert.Len(t, errorutils.Unwrap(docErr), 2) + assert.Len(t, errorutils.ShallowUnwrap(docErr), 2) assert.Nil(t, v3Doc) } @@ -302,7 +302,7 @@ func TestDocument_BuildModelPreBuild(t *testing.T) { require.NoError(t, err) _, _, _, err = doc.RenderAndReload() assert.Nil(t, err) - assert.Len(t, errorutils.Unwrap(err), 0) + assert.Len(t, errorutils.ShallowUnwrap(err), 0) } func TestDocument_AnyDoc(t *testing.T) { @@ -328,7 +328,7 @@ func TestDocument_BuildModelCircular(t *testing.T) { // top level library does not return broken objects // with an error, only one or the other assert.Nil(t, m) - assert.Len(t, errorutils.Unwrap(err), 3) + assert.Len(t, errorutils.ShallowUnwrap(err), 3) } func TestDocument_BuildModelBad(t *testing.T) { @@ -337,7 +337,7 @@ func TestDocument_BuildModelBad(t *testing.T) { require.NoError(t, err) m, err := doc.BuildV3Model() assert.Nil(t, m) - assert.Len(t, errorutils.Unwrap(err), 9) + assert.Len(t, errorutils.ShallowUnwrap(err), 9) } func TestDocument_Serialize_JSON_Modified(t *testing.T) { @@ -407,7 +407,7 @@ func TestDocument_BuildModel_CompareDocsV3_LeftError(t *testing.T) { WithAllowFileReferences(true), ) changes, err := CompareDocuments(originalDoc, updatedDoc) - require.Len(t, errorutils.Unwrap(err), 9) + require.Len(t, errorutils.ShallowUnwrap(err), 9) require.Nil(t, changes) } @@ -418,7 +418,7 @@ func TestDocument_BuildModel_CompareDocsV3_RightError(t *testing.T) { originalDoc, _ := NewDocument(burgerShopOriginal) updatedDoc, _ := NewDocument(burgerShopUpdated) changes, err := CompareDocuments(updatedDoc, originalDoc) - require.Len(t, errorutils.Unwrap(err), 9) + require.Len(t, errorutils.ShallowUnwrap(err), 9) require.Nil(t, changes) } @@ -429,7 +429,7 @@ func TestDocument_BuildModel_CompareDocsV2_Error(t *testing.T) { originalDoc, _ := NewDocument(burgerShopOriginal) updatedDoc, _ := NewDocument(burgerShopUpdated) changes, err := CompareDocuments(updatedDoc, originalDoc) - require.Len(t, errorutils.Unwrap(err), 2) + require.Len(t, errorutils.ShallowUnwrap(err), 2) require.Nil(t, changes) } @@ -446,7 +446,7 @@ func TestDocument_BuildModel_CompareDocsV2V3Mix_Error(t *testing.T) { require.NoError(t, err) changes, err := CompareDocuments(updatedDoc, originalDoc) - require.Len(t, errorutils.Unwrap(err), 1) + require.Len(t, errorutils.ShallowUnwrap(err), 1) require.Nil(t, changes) } diff --git a/internal/errorutils/errorutils.go b/internal/errorutils/errorutils.go index d99a51de..2719d4bc 100644 --- a/internal/errorutils/errorutils.go +++ b/internal/errorutils/errorutils.go @@ -24,6 +24,7 @@ func (e *MultiError) Unwrap() []error { return e.errs } +// Join should be used at the end of top level functions to join all errors. func Join(errs ...error) error { var result MultiError @@ -39,35 +40,39 @@ func Join(errs ...error) error { result.errs = make([]error, 0, size) for _, err := range errs { - // try to keep MultiError flat - if multi, ok := err.(*MultiError); ok { - result.errs = append(result.errs, multi.Unwrap()...) - } else { - result.errs = append(result.errs, err) + if err == nil { + continue } + // try to keep MultiError flat + result.errs = append(result.errs, deepUnwrapMultiError(err)...) } return &result } -// Unwrap recursively unwraps errors and flattens them into a slice of errors. -func Unwrap(err error) []error { +func ShallowUnwrap(err error) []error { + if err == nil { + return nil + } + unwrap, ok := err.(interface{ Unwrap() []error }) + if !ok { + return []error{err} + } + + return unwrap.Unwrap() +} + +func deepUnwrapMultiError(err error) []error { if err == nil { return nil } var result []error - if unwrap, ok := err.(interface{ Unwrap() []error }); ok { - // ignore wrapping error - no hierarchy - for _, e := range unwrap.Unwrap() { - result = append(result, Unwrap(e)...) + + if multi, ok := err.(*MultiError); ok { + for _, e := range multi.Unwrap() { + result = append(result, deepUnwrapMultiError(e)...) } - return result - } else if unwrap, ok := err.(interface{ Unwrap() error }); ok { - // add parent error to result + } else { result = append(result, err) - result = append(result, Unwrap(unwrap.Unwrap())...) - return result } - // no unwrapping needed, as it's not wrapped - result = append(result, err) return result } diff --git a/internal/errorutils/filter.go b/internal/errorutils/filter.go index 9207297a..f19f5ff8 100644 --- a/internal/errorutils/filter.go +++ b/internal/errorutils/filter.go @@ -4,7 +4,7 @@ func Filtered(err error, filters ...func(error) (keep bool)) error { if err == nil { return nil } - errs := Unwrap(err) + errs := ShallowUnwrap(err) filtered := Filter(errs, AndFilter(filters...)) if len(filtered) == 0 { return nil