cue icon indicating copy to clipboard operation
cue copied to clipboard

pkg/encoding: yaml.Marshal hides errors

Open mxey opened this issue 2 years ago • 2 comments

yaml.Marshal in a custom CUE command apparently hides validation errors. The same errors are visible when you instead use "cue export". That makes it harder to see what the real error is.

It seemed to me that there has to be a package boundary between the Marshal and the validation error for this to trigger.

What version of CUE are you using (cue version)?

$ cue version
cue version v0.6.0

go version go1.20.6
      -buildmode exe
       -compiler gc
       -trimpath true
     CGO_ENABLED 0
          GOARCH arm64
            GOOS darwin

Does this issue reproduce with the latest stable release?

v0.6.0 is the latest stable release.

What did you do?

run this testscript:

! exec cue export ./rules
stderr '.*conflicting values 0 and 1.*'

! exec cue cmd build-rules
stderr '.*conflicting values 0 and 1.*'

-- build_tool.cue --
package prometheus

import (
	"encoding/yaml"
	"tool/file"

	"gitlab.rz.babiel.com/provid/apps/prometheus/rules"
)

command: "build-rules": task: {
	for name, group in rules.groups {
		(name): file.Create & {
			filename: "rules/\(name).yaml"
			contents: yaml.Marshal({
				groups: [group]
			})
		}
	}
}
-- cue.mod/module.cue --
module: "gitlab.rz.babiel.com/provid/apps/prometheus"
-- rules/automainter.cue --
package rules

import (
	"list"
	"strings"
	"strconv"
)

groups: [NAME=string]: #RuleGroup & {name: NAME}

#RuleGroup: {
	name: string
	rules: [...#Rule]
	interval?: string
}

#Rule: #AlertRule | #RecordingRule

#AlertRule: {
	alert: string
	expr:  string
}

#RecordingRule: {
	record: string
	expr:   string
}

#AutomainterEntry: close({
	name:   string
	period: #TimePeriod & {
		periods: [...{
			windows: [...{
				#startTime: m: 0
			}]
		}]
	}

	_prom_ql: {

		_periods: [ for p in period.periods {
			_windows: [ for w in p.windows {
				_ql: "\(w.#startTime.h)"
			}]

			_ql: strings.Join([ for w in _windows {w._ql}], " or")
		}]
	}
})

blockedUnless: [G=string]: #AutomainterEntry & {
	name: G
}

groups: {
	automainter_blocked_unless: {

		rules: [ for n, g in blockedUnless {
			{
				alert: "OutsideMaintWindow"
				expr:  """
     (
     ) unless on () (
       \(strings.Join([ for p in g._prom_ql._periods {p._ql}], " or "))
     )
     """
				labels: severity: "none"
			}
		}]
	}
}

blockedUnless: "SOMETHING": {
	period: {
		periods: [{
			windows: [{
				start: "20:01"
			}]
		}]
	}
}

#timeString: string
#timeString: =~"^((?:24)?([01]?[0-9]|2[0-3]):[0-5][0-9]|24:00)$"

#time: {
	_timeString: #timeString
	_timeParts:  strings.SplitN(_timeString, ":", 2)

	h: int
	h: strconv.Atoi(_timeParts[0])

	m: int
	m: strconv.Atoi(_timeParts[1])
}

#timeWindow: {
	start:      #timeString
	#startTime: #time & {
		_timeString: start
		h:           !=24
	}
}

#period: {
	windows: [...#timeWindow] & list.MinItems(1)
}

#TimePeriod: {
	periods: [...#period] & list.MinItems(1)
}

What did you expect to see?

PASS

What did you see instead?

[…]
FAIL: /var/folders/k_/nbvcnqz17vdc75tz068w33jc0000gp/T/testscript3493091290/missing-errors.testscript/script.txtar:5: no match for `.*conflicting values 0 and 1.*` found in stderr
error running missing-errors.testscript in /var/folders/k_/nbvcnqz17vdc75tz068w33jc0000gp/T/testscript3493091290/missing-errors.testscript

mxey avatar Dec 01 '23 08:12 mxey

I also noticed this happens with yaml.Marshal, but not with json.Marshal. The difference is in these lines in yaml.Marshal:

	if err := v.Validate(cue.Concrete(true)); err != nil {
		return "", err
	}

If I add those to json.Marshal, I get the same error with JSON.

mxey avatar Dec 01 '23 08:12 mxey

Part of the problem is that approximateEqual for error comparison is imprecise. When marshalling, it seems all the errors are located at the same source position (where the marshal is called), and approximateEqual deduplicates errors by position, ignoring the error itself.

func approximateEqual(a, b Error) bool {
	aPos := a.Position()
	bPos := b.Position()
	if aPos == token.NoPos || bPos == token.NoPos {
		return a.Error() == b.Error()
	}
	return aPos.Filename() == bPos.Filename() &&
		aPos.Line() == bPos.Line() &&
		aPos.Column() == bPos.Column() &&
		equalPath(a.Path(), b.Path())
}

When I change it like this:

diff --git a/cue/errors/errors.go b/cue/errors/errors.go
index 8fb87039..bbb4e3a6 100644
--- a/cue/errors/errors.go
+++ b/cue/errors/errors.go
@@ -486,7 +486,7 @@ func approximateEqual(a, b Error) bool {
     return aPos.Filename() == bPos.Filename() &&
         aPos.Line() == bPos.Line() &&
         aPos.Column() == bPos.Column() &&
-        equalPath(a.Path(), b.Path())
+        equalPath(a.Path(), b.Path()) && a.Error() == b.Error()
 }
 
 // An List implements the error interface.

then I get all the errors.

mxey avatar Dec 01 '23 08:12 mxey

Is there any update on this ? As there is apparently a fix provided in the issue itself.

I tried on v0.15.0-alpha.2 and it does the same with json.Marshal

LelouBil avatar Oct 20 '25 13:10 LelouBil