Skip to content

Commit d8ebefe

Browse files
authored
♻️ [config] Improve validation error formatting (#585)
<!-- Copyright (C) 2020-2022 Arm Limited or its affiliates and Contributors. All rights reserved. SPDX-License-Identifier: Apache-2.0 --> ### Description Improve validation errors returned by configuration validation ### Test Coverage <!-- Please put an `x` in the correct box e.g. `[x]` to indicate the testing coverage of this change. --> - [x] This change is covered by existing or additional automated tests. - [ ] Manual testing has been performed (and evidence provided) as automated testing was not feasible. - [ ] Additional tests are not required for this change (e.g. documentation update).
1 parent 11d8613 commit d8ebefe

File tree

8 files changed

+377
-31
lines changed

8 files changed

+377
-31
lines changed

.secrets.baseline

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,21 +130,21 @@
130130
"filename": "utils/config/service_configuration_test.go",
131131
"hashed_secret": "7ca1cc114e7e5f955880bb96a5bf391b4dc20ab6",
132132
"is_verified": false,
133-
"line_number": 481
133+
"line_number": 497
134134
},
135135
{
136136
"type": "Secret Keyword",
137137
"filename": "utils/config/service_configuration_test.go",
138138
"hashed_secret": "11519c144be4850d95b34220a40030cbd5a36b57",
139139
"is_verified": false,
140-
"line_number": 576
140+
"line_number": 592
141141
},
142142
{
143143
"type": "Secret Keyword",
144144
"filename": "utils/config/service_configuration_test.go",
145145
"hashed_secret": "15fae91d8fa7f2c531c1cf3ddc745e1f4473c02d",
146146
"is_verified": false,
147-
"line_number": 583
147+
"line_number": 599
148148
}
149149
],
150150
"utils/filesystem/filehash_test.go": [
@@ -265,5 +265,5 @@
265265
}
266266
]
267267
},
268-
"generated_at": "2025-03-17T21:06:33Z"
268+
"generated_at": "2025-03-28T10:35:43Z"
269269
}

changes/20250328102314.bugfix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
:recycle: [config] Improve error formatting to ease understanding configuration validation errors

utils/commonerrors/serialisation.go

Lines changed: 76 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,23 @@ type iMarshallingError interface {
1919
error
2020
ConvertToError() error
2121
SetWrappedError(err error)
22+
GetReason() string
23+
GetErrorType() error
2224
}
2325

2426
type marshallingError struct {
2527
Reason string
2628
ErrorType error
2729
}
2830

31+
func (e *marshallingError) GetReason() string {
32+
return e.Reason
33+
}
34+
35+
func (e *marshallingError) GetErrorType() error {
36+
return e.ErrorType
37+
}
38+
2939
func (e *marshallingError) MarshalText() (text []byte, err error) {
3040
str := e.String()
3141
return []byte(str), nil
@@ -73,6 +83,22 @@ type multiplemarshallingError struct {
7383
subErrs []iMarshallingError
7484
}
7585

86+
func (m *multiplemarshallingError) GetReason() string {
87+
reasons := make([]string, 0, len(m.subErrs))
88+
for i := range m.subErrs {
89+
reasons = append(reasons, m.subErrs[i].GetReason())
90+
}
91+
return strings.Join(reasons, string(MultipleErrorSeparator))
92+
}
93+
94+
func (m *multiplemarshallingError) GetErrorType() error {
95+
errs := make([]error, 0, len(m.subErrs))
96+
for i := range m.subErrs {
97+
errs = append(errs, m.subErrs[i].GetErrorType())
98+
}
99+
return errors.Join(errs...)
100+
}
101+
76102
func (m *multiplemarshallingError) MarshalText() (text []byte, err error) {
77103
for i := range m.subErrs {
78104
subtext, suberr := m.subErrs[i].MarshalText()
@@ -112,7 +138,7 @@ func (m *multiplemarshallingError) UnmarshalText(text []byte) error {
112138
}
113139

114140
func (m *multiplemarshallingError) ConvertToError() error {
115-
var errs []error
141+
errs := make([]error, 0, len(m.subErrs))
116142
for i := range m.subErrs {
117143
errs = append(errs, m.subErrs[i].ConvertToError())
118144
}
@@ -227,20 +253,62 @@ func SerialiseError(err error) ([]byte, error) {
227253

228254
// DeserialiseError unmarshals text into an error. It tries to determine the error type.
229255
func DeserialiseError(text []byte) (deserialisedError, err error) {
230-
if len(text) == 0 {
256+
mErr, err := deserialiseError(text)
257+
if err != nil || mErr == nil {
231258
return
232259
}
233-
var mErr iMarshallingError
260+
deserialisedError = mErr.ConvertToError()
261+
return
262+
}
234263

264+
func deserialiseError(text []byte) (deserialisedError iMarshallingError, err error) {
265+
if len(text) == 0 {
266+
return
267+
}
235268
if strings.Contains(string(text), string(MultipleErrorSeparator)) {
236-
mErr = &multiplemarshallingError{}
269+
deserialisedError = &multiplemarshallingError{}
237270
} else {
238-
mErr = &marshallingError{}
271+
deserialisedError = &marshallingError{}
239272
}
240-
err = mErr.UnmarshalText(text)
241-
if err != nil {
273+
err = deserialisedError.UnmarshalText(text)
274+
return
275+
}
276+
277+
func GetErrorReason(srcErr error) (reason string, err error) {
278+
if srcErr == nil {
279+
err = UndefinedVariable("source error")
242280
return
243281
}
244-
deserialisedError = mErr.ConvertToError()
282+
mErr, err := deserialiseError([]byte(srcErr.Error()))
283+
if err != nil || mErr == nil {
284+
return
285+
}
286+
reason = mErr.GetReason()
287+
return
288+
}
289+
290+
func GetCommonErrorReason(srcErr error) (reason string, err error) {
291+
if srcErr == nil {
292+
err = UndefinedVariable("source error")
293+
return
294+
}
295+
if !IsCommonError(srcErr) {
296+
reason = srcErr.Error()
297+
return
298+
}
299+
reason, err = GetErrorReason(srcErr)
300+
return
301+
}
302+
303+
func GetUnderlyingErrorType(srcErr error) (commonerrorType, err error) {
304+
if srcErr == nil {
305+
err = UndefinedVariable("source error")
306+
return
307+
}
308+
mErr, err := deserialiseError([]byte(srcErr.Error()))
309+
if err != nil || mErr == nil {
310+
return
311+
}
312+
commonerrorType = mErr.GetErrorType()
245313
return
246314
}

utils/commonerrors/serialisation_test.go

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func TestDeserialise(t *testing.T) {
4242
expectedError: errors.New(errStr),
4343
},
4444
{
45-
text: fmt.Errorf("%w: %v", errors.New(errStr), sentence).Error(),
45+
text: New(errors.New(errStr), sentence).Error(),
4646
expectedReason: sentence,
4747
expectedError: errors.New(errStr),
4848
},
@@ -144,7 +144,7 @@ func (m multiErr) Error() string { return errors.Join(m...).Error() }
144144
func (m multiErr) Unwrap() []error { return []error(m) }
145145

146146
func TestMultipleError(t *testing.T) {
147-
expectedErr := multiErr([]error{fmt.Errorf("%w: %v", ErrInvalid, strings.ReplaceAll(faker.Sentence(), string(MultipleErrorSeparator), "sep")), errors.New(""), fmt.Errorf("%w: %v", ErrUnexpected, strings.ReplaceAll(faker.Sentence(), string(MultipleErrorSeparator), ";"))})
147+
expectedErr := multiErr([]error{New(ErrInvalid, strings.ReplaceAll(faker.Sentence(), string(MultipleErrorSeparator), "sep")), errors.New(""), New(ErrUnexpected, strings.ReplaceAll(faker.Sentence(), string(MultipleErrorSeparator), ";"))})
148148
text, err := SerialiseError(expectedErr)
149149
require.NoError(t, err)
150150
assert.NotEmpty(t, text)
@@ -191,6 +191,31 @@ func TestGenericSerialisation(t *testing.T) {
191191
assert.NoError(t, deserialisedErr)
192192
})
193193

194+
t.Run("error Reason", func(t *testing.T) {
195+
reason := faker.Sentence()
196+
dErr := New(errors.New(faker.Word()), reason)
197+
dReason, err := GetCommonErrorReason(dErr)
198+
require.NoError(t, err)
199+
assert.Equal(t, dErr.Error(), dReason)
200+
dReason, err = GetErrorReason(dErr)
201+
require.NoError(t, err)
202+
assert.Equal(t, reason, dReason)
203+
reason = faker.Sentence()
204+
dErr = New(ErrNotFound, reason)
205+
dReason, err = GetCommonErrorReason(dErr)
206+
require.NoError(t, err)
207+
assert.Equal(t, reason, dReason)
208+
dReason, err = GetErrorReason(dErr)
209+
require.NoError(t, err)
210+
assert.Equal(t, reason, dReason)
211+
dReason, err = GetCommonErrorReason(nil)
212+
assert.True(t, Any(err, ErrUndefined))
213+
assert.Empty(t, dReason)
214+
dReason, err = GetErrorReason(nil)
215+
assert.True(t, Any(err, ErrUndefined))
216+
assert.Empty(t, dReason)
217+
})
218+
194219
tests := []struct {
195220
commonError error
196221
}{
@@ -227,12 +252,21 @@ func TestGenericSerialisation(t *testing.T) {
227252
test := tests[i]
228253
t.Run(test.commonError.Error(), func(t *testing.T) {
229254
reason := strings.ReplaceAll(faker.Sentence(), "\n", ";")
230-
text, err := SerialiseError(fmt.Errorf("%w : %v", test.commonError, reason))
255+
text, err := SerialiseError(New(test.commonError, reason))
231256
require.NoError(t, err)
232257
dErr, err := DeserialiseError(text)
233258
require.NoError(t, err)
234259
assert.True(t, Any(dErr, test.commonError))
235260
assert.True(t, strings.Contains(dErr.Error(), reason))
261+
underlyingErr, err := GetUnderlyingErrorType(dErr)
262+
require.NoError(t, err)
263+
assert.True(t, Any(underlyingErr, test.commonError))
264+
dReason, err := GetErrorReason(dErr)
265+
require.NoError(t, err)
266+
assert.Equal(t, reason, dReason)
267+
dReason, err = GetCommonErrorReason(dErr)
268+
require.NoError(t, err)
269+
assert.Equal(t, reason, dReason)
236270
})
237271
}
238272
}

0 commit comments

Comments
 (0)