Skip to content

Commit 2ccabd8

Browse files
committed
refactor: improve error handling and logging in validation functions
1 parent 1661442 commit 2ccabd8

File tree

3 files changed

+4
-5
lines changed

3 files changed

+4
-5
lines changed

internal/orchestrator/app/validator.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,9 @@ import (
1010

1111
// ValidateBricks checks that all bricks referenced in the given AppDescriptor exist in the provided BricksIndex,
1212
// It collects and returns all validation errors as a single joined error, allowing the caller to see all issues at once rather than stopping at the first error.
13-
// If the index or the app is nil, validation is skipped and nil is returned.
1413
func ValidateBricks(a AppDescriptor, index *bricksindex.BricksIndex) error {
1514
if index == nil {
16-
return nil
15+
return fmt.Errorf("bricks index cannot be nil")
1716
}
1817

1918
var allErrors error
@@ -28,6 +27,7 @@ func ValidateBricks(a AppDescriptor, index *bricksindex.BricksIndex) error {
2827
for appBrickVariableName := range appBrick.Variables {
2928
_, exist := indexBrick.GetVariable(appBrickVariableName)
3029
if !exist {
30+
// TODO: we should return warnings
3131
slog.Warn("variable is referenced but not declared in the brick configuration", "variable", appBrickVariableName, "brick", indexBrick.ID)
3232
}
3333
}

internal/orchestrator/app/validator_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414

1515
func TestValidateAppDescriptorBricks(t *testing.T) {
1616
bricksIndex, err := bricksindex.GenerateBricksIndexFromFile(paths.New("testdata/validator"))
17-
require.Nil(t, err)
17+
require.NoError(t, err)
1818
require.NotNil(t, bricksIndex)
1919

2020
testCases := []struct {

internal/orchestrator/bricks/bricks.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,8 +290,7 @@ func (s *Service) BrickCreate(
290290
for _, brickVar := range brick.Variables {
291291
if brickVar.DefaultValue == "" {
292292
if _, exist := req.Variables[brickVar.Name]; !exist {
293-
// See issue https://github.com/arduino/arduino-app-cli/issues/68
294-
slog.Warn("[Skip] a required variable is not set by user", "variable", brickVar.Name)
293+
slog.Warn("[Skip] a required variable is not set by user", "variable", brickVar.Name, "brick", brickVar.Name)
295294
}
296295
}
297296
}

0 commit comments

Comments
 (0)