pkg/tool: improve error reporting
Bail early in case of unavailable fields.
Adds helper methods to internal/task for this purpose.
Fixes #243
Change-Id: I664e8b91e119929be9babce293af5a31ad550dd4
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/4483
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cmd/cue/cmd/custom.go b/cmd/cue/cmd/custom.go
index af08fd4..ae62f5f 100644
--- a/cmd/cue/cmd/custom.go
+++ b/cmd/cue/cmd/custom.go
@@ -165,6 +165,14 @@
}
func getTasks(q []*task, v cue.Value, stack []string) ([]*task, error) {
+ // Allow non-task values, but do not allow errors.
+ if err := v.Err(); err != nil {
+ return nil, err
+ }
+ if v.Kind()&cue.StructKind == 0 {
+ return q, nil
+ }
+
if v.Lookup("$id").Exists() || v.Lookup("kind").Exists() {
t, err := newTask(len(q), stack, v)
if err != nil {
@@ -289,7 +297,11 @@
// NOTE: ignore the linter warning for the following line:
// itask.Context is an internal type and we want to break if any
// fields are added.
- update, err := t.Run(&itask.Context{ctx, stdout, stderr}, obj)
+ c := &itask.Context{ctx, stdout, stderr, obj, nil}
+ update, err := t.Run(c)
+ if c.Err != nil {
+ err = c.Err
+ }
if err == nil && update != nil {
cr.root, err = cr.root.Fill(update, cr.taskPath(t)...)
@@ -448,6 +460,6 @@
type testServerCmd string
-func (s testServerCmd) Run(ctx *itask.Context, v cue.Value) (x interface{}, err error) {
+func (s testServerCmd) Run(ctx *itask.Context) (x interface{}, err error) {
return map[string]interface{}{"url": string(s)}, nil
}
diff --git a/cmd/cue/cmd/testdata/script/cmd_err.txt b/cmd/cue/cmd/testdata/script/cmd_err.txt
new file mode 100644
index 0000000..61b9b42
--- /dev/null
+++ b/cmd/cue/cmd/testdata/script/cmd_err.txt
@@ -0,0 +1,19 @@
+! cue cmd ref
+! stdout .
+cmp stderr cmd_badfields.out
+
+-- cmd_badfields.out --
+command.ref.task.display.filename: non-concrete value string:
+ tool/file:9:16
+command.ref.task.display.contents: invalid bytes argument for field "contents": non-concrete value string|bytes:
+ ./task_tool.cue:6:17
+-- task_tool.cue --
+package home
+
+import "tool/file"
+
+command: ref: {
+ task: display: file.Create & {
+ filename: filename
+ }
+}
diff --git a/internal/task/task.go b/internal/task/task.go
index b494a1a..a6e563c 100644
--- a/internal/task/task.go
+++ b/internal/task/task.go
@@ -21,6 +21,7 @@
"sync"
"cuelang.org/go/cue"
+ "cuelang.org/go/cue/errors"
)
// A Context provides context for running a task.
@@ -28,6 +29,61 @@
Context context.Context
Stdout io.Writer
Stderr io.Writer
+ Obj cue.Value
+ Err errors.Error
+}
+
+func (c *Context) Lookup(field string) cue.Value {
+ f := c.Obj.Lookup(field)
+ if !f.Exists() {
+ c.Err = errors.Append(c.Err, errors.Newf(c.Obj.Pos(),
+ "could not find field %q", field))
+ return cue.Value{}
+ }
+ if err := f.Err(); err != nil {
+ c.Err = errors.Append(c.Err, errors.Promote(err, "lookup"))
+ }
+ return f
+}
+
+func (c *Context) Int64(field string) int64 {
+ f := c.Obj.Lookup(field)
+ value, err := f.Int64()
+ if err != nil {
+ // TODO: use v for position for now, as f has not yet a
+ // position associated with it.
+ c.Err = errors.Append(c.Err, errors.Wrapf(err, c.Obj.Pos(),
+ "invalid integer argument for field %q", field))
+ return 0
+ }
+ return value
+}
+
+func (c *Context) String(field string) string {
+ f := c.Obj.Lookup(field)
+ value, err := f.String()
+ if err != nil {
+ // TODO: use v for position for now, as f has not yet a
+ // position associated with it.
+ c.Err = errors.Append(c.Err, errors.Wrapf(err, c.Obj.Pos(),
+ "invalid string argument for field %q", field))
+ c.Err = errors.Promote(err, "ddd")
+ return ""
+ }
+ return value
+}
+
+func (c *Context) Bytes(field string) []byte {
+ f := c.Obj.Lookup(field)
+ value, err := f.Bytes()
+ if err != nil {
+ // TODO: use v for position for now, as f has not yet a
+ // position associated with it.
+ c.Err = errors.Append(c.Err, errors.Wrapf(err, c.Obj.Pos(),
+ "invalid bytes argument for field %q", field))
+ return nil
+ }
+ return value
}
// A RunnerFunc creates a Runner.
@@ -42,7 +98,7 @@
// Runner runs given the current value and returns a new value which is to
// be unified with the original result.
- Run(ctx *Context, v cue.Value) (results interface{}, err error)
+ Run(ctx *Context) (results interface{}, err error)
}
// Register registers a task for cue commands.
diff --git a/pkg/tool/cli/cli.go b/pkg/tool/cli/cli.go
index 4b41c09..dd29681 100644
--- a/pkg/tool/cli/cli.go
+++ b/pkg/tool/cli/cli.go
@@ -36,10 +36,10 @@
return &printCmd{}, nil
}
-func (c *printCmd) Run(ctx *task.Context, v cue.Value) (res interface{}, err error) {
- str, err := v.Lookup("text").String()
- if err != nil {
- return nil, err
+func (c *printCmd) Run(ctx *task.Context) (res interface{}, err error) {
+ str := ctx.String("text")
+ if ctx.Err != nil {
+ return nil, ctx.Err
}
fmt.Fprintln(ctx.Stdout, str)
return nil, nil
diff --git a/pkg/tool/exec/exec.go b/pkg/tool/exec/exec.go
index 6146e53..c64dc1c 100644
--- a/pkg/tool/exec/exec.go
+++ b/pkg/tool/exec/exec.go
@@ -40,17 +40,18 @@
return &execCmd{}, nil
}
-func (c *execCmd) Run(ctx *task.Context, v cue.Value) (res interface{}, err error) {
+func (c *execCmd) Run(ctx *task.Context) (res interface{}, err error) {
// TODO: set environment variables, if defined.
var bin string
var args []string
doc := ""
- switch v := v.Lookup("cmd"); v.Kind() {
+ v := ctx.Lookup("cmd")
+ if ctx.Err != nil {
+ return nil, ctx.Err
+ }
+ switch v.Kind() {
case cue.StringKind:
- str, _ := v.String()
- if str == "" {
- return cue.Value{}, errors.New("empty command")
- }
+ str := ctx.String("cmd")
doc = str
list := strings.Fields(str)
bin = list[0]
@@ -76,16 +77,20 @@
}
}
+ if bin == "" {
+ return cue.Value{}, errors.New("empty command")
+ }
+
cmd := exec.CommandContext(ctx.Context, bin, args...)
stream := func(name string) (stream cue.Value, ok bool) {
- c := v.Lookup(name)
+ c := ctx.Obj.Lookup(name)
// Although the schema defines a default versions, older implementations
// may not use it yet.
if !c.Exists() {
return
}
- if err := c.Null(); err == nil {
+ if err := c.Null(); ctx.Err != nil || err == nil {
return
}
return c, true
diff --git a/pkg/tool/file/file.go b/pkg/tool/file/file.go
index 694f832..b0d1e23 100644
--- a/pkg/tool/file/file.go
+++ b/pkg/tool/file/file.go
@@ -42,32 +42,35 @@
type cmdCreate struct{}
type cmdGlob struct{}
-func lookupStr(v cue.Value, str string) string {
- str, _ = v.Lookup(str).String()
- return str
-}
+func (c *cmdRead) Run(ctx *task.Context) (res interface{}, err error) {
+ filename := ctx.String("filename")
+ if ctx.Err != nil {
+ return nil, ctx.Err
+ }
-func (c *cmdRead) Run(ctx *task.Context, v cue.Value) (res interface{}, err error) {
- b, err := ioutil.ReadFile(lookupStr(v, "filename"))
+ b, err := ioutil.ReadFile(filename)
if err != nil {
return nil, err
}
update := map[string]interface{}{"contents": b}
- switch v.Lookup("contents").IncompleteKind() {
+ switch v := ctx.Lookup("contents"); v.IncompleteKind() {
case cue.BytesKind:
+ // already set above
case cue.StringKind:
update["contents"] = string(b)
}
return update, nil
}
-func (c *cmdAppend) Run(ctx *task.Context, v cue.Value) (res interface{}, err error) {
- filename := lookupStr(v, "filename")
- filename = filepath.FromSlash(filename)
- mode, err := v.Lookup("permissions").Int64()
- if err != nil {
- return nil, err
+func (c *cmdAppend) Run(ctx *task.Context) (res interface{}, err error) {
+ var (
+ filename = filepath.FromSlash(ctx.String("filename"))
+ mode = ctx.Int64("permissions")
+ b = ctx.Bytes("contents")
+ )
+ if ctx.Err != nil {
+ return nil, ctx.Err
}
f, err := os.OpenFile(filename, os.O_APPEND|os.O_WRONLY, os.FileMode(mode))
@@ -76,27 +79,30 @@
}
defer f.Close()
- b, _ := v.Lookup("contents").Bytes()
if _, err := f.Write(b); err != nil {
return nil, err
}
return nil, nil
}
-func (c *cmdCreate) Run(ctx *task.Context, v cue.Value) (res interface{}, err error) {
- filename := lookupStr(v, "filename")
- filename = filepath.FromSlash(filename)
- mode, err := v.Lookup("permissions").Int64()
- if err != nil {
- return nil, err
+func (c *cmdCreate) Run(ctx *task.Context) (res interface{}, err error) {
+ var (
+ filename = filepath.FromSlash(ctx.String("filename"))
+ mode = ctx.Int64("permissions")
+ b = ctx.Bytes("contents")
+ )
+ if ctx.Err != nil {
+ return nil, ctx.Err
}
- b, _ := v.Lookup("contents").Bytes()
return nil, ioutil.WriteFile(filename, b, os.FileMode(mode))
}
-func (c *cmdGlob) Run(ctx *task.Context, v cue.Value) (res interface{}, err error) {
- glob := filepath.FromSlash(lookupStr(v, "glob"))
+func (c *cmdGlob) Run(ctx *task.Context) (res interface{}, err error) {
+ glob := ctx.String("glob")
+ if ctx.Err != nil {
+ return nil, ctx.Err
+ }
m, err := filepath.Glob(glob)
for i, s := range m {
m[i] = filepath.ToSlash(s)
diff --git a/pkg/tool/file/file_test.go b/pkg/tool/file/file_test.go
index b284445..6dfb17f 100644
--- a/pkg/tool/file/file_test.go
+++ b/pkg/tool/file/file_test.go
@@ -25,6 +25,7 @@
"cuelang.org/go/cue"
"cuelang.org/go/cue/parser"
"cuelang.org/go/internal"
+ "cuelang.org/go/internal/task"
)
func parse(t *testing.T, kind, expr string) cue.Value {
@@ -43,7 +44,7 @@
}
func TestRead(t *testing.T) {
v := parse(t, "tool/file.Read", `{filename: "testdata/input.foo"}`)
- got, err := (*cmdRead).Run(nil, nil, v)
+ got, err := (*cmdRead).Run(nil, &task.Context{Obj: v})
if err != nil {
t.Fatal(err)
}
@@ -56,7 +57,7 @@
filename: "testdata/input.foo"
contents: string
}`)
- got, err = (*cmdRead).Run(nil, nil, v)
+ got, err = (*cmdRead).Run(nil, &task.Context{Obj: v})
if err != nil {
t.Fatal(err)
}
@@ -80,7 +81,7 @@
filename: "%s"
contents: "This is a test."
}`, name))
- _, err = (*cmdAppend).Run(nil, nil, v)
+ _, err = (*cmdAppend).Run(nil, &task.Context{Obj: v})
if err != nil {
t.Fatal(err)
}
@@ -109,7 +110,7 @@
filename: "%s"
contents: "This is a test."
}`, name))
- _, err = (*cmdCreate).Run(nil, nil, v)
+ _, err = (*cmdCreate).Run(nil, &task.Context{Obj: v})
if err != nil {
t.Fatal(err)
}
@@ -128,7 +129,7 @@
v := parse(t, "tool/file.Glob", fmt.Sprintf(`{
glob: "testdata/input.*"
}`))
- got, err := (*cmdGlob).Run(nil, nil, v)
+ got, err := (*cmdGlob).Run(nil, &task.Context{Obj: v})
if err != nil {
t.Fatal(err)
}
diff --git a/pkg/tool/http/http.go b/pkg/tool/http/http.go
index f43ff7c..eaa77bc 100644
--- a/pkg/tool/http/http.go
+++ b/pkg/tool/http/http.go
@@ -38,20 +38,14 @@
return &httpCmd{}, nil
}
-func lookupString(obj cue.Value, key string) string {
- str, err := obj.Lookup(key).String()
- if err != nil {
- return ""
- }
- return str
-}
-
-func (c *httpCmd) Run(ctx *task.Context, v cue.Value) (res interface{}, err error) {
+func (c *httpCmd) Run(ctx *task.Context) (res interface{}, err error) {
var header, trailer http.Header
- method := lookupString(v, "method")
- u := lookupString(v, "url")
+ var (
+ method = ctx.String("method")
+ u = ctx.String("url")
+ )
var r io.Reader
- if obj := v.Lookup("request"); v.Exists() {
+ if obj := ctx.Obj.Lookup("request"); obj.Exists() {
if v := obj.Lookup("body"); v.Exists() {
r, err = v.Reader()
if err != nil {
@@ -65,6 +59,10 @@
return nil, err
}
}
+ if ctx.Err != nil {
+ return nil, ctx.Err
+ }
+
req, err := http.NewRequest(method, u, r)
if err != nil {
return nil, err
diff --git a/pkg/tool/os/env.go b/pkg/tool/os/env.go
index 487d397..32880ff 100644
--- a/pkg/tool/os/env.go
+++ b/pkg/tool/os/env.go
@@ -51,7 +51,7 @@
return &clearenvCmd{}, nil
}
-func (c *clearenvCmd) Run(ctx *task.Context, v cue.Value) (res interface{}, err error) {
+func (c *clearenvCmd) Run(ctx *task.Context) (res interface{}, err error) {
os.Clearenv()
return map[string]interface{}{}, nil
}
@@ -62,8 +62,8 @@
return &setenvCmd{}, nil
}
-func (c *setenvCmd) Run(ctx *task.Context, v cue.Value) (res interface{}, err error) {
- iter, err := v.Fields()
+func (c *setenvCmd) Run(ctx *task.Context) (res interface{}, err error) {
+ iter, err := ctx.Obj.Fields()
if err != nil {
return nil, err
}
@@ -117,8 +117,8 @@
return &getenvCmd{}, nil
}
-func (c *getenvCmd) Run(ctx *task.Context, v cue.Value) (res interface{}, err error) {
- iter, err := v.Fields()
+func (c *getenvCmd) Run(ctx *task.Context) (res interface{}, err error) {
+ iter, err := ctx.Obj.Fields()
if err != nil {
return nil, err
}
@@ -157,8 +157,8 @@
return &environCmd{}, nil
}
-func (c *environCmd) Run(ctx *task.Context, v cue.Value) (res interface{}, err error) {
- iter, err := v.Fields()
+func (c *environCmd) Run(ctx *task.Context) (res interface{}, err error) {
+ iter, err := ctx.Obj.Fields()
if err != nil {
return nil, err
}
@@ -171,7 +171,7 @@
name := a[0]
str := a[1]
- if v := v.Lookup(name); v.Exists() {
+ if v := ctx.Obj.Lookup(name); v.Exists() {
update[name], err = fromString(name, str, v)
if err != nil {
return nil, err
diff --git a/pkg/tool/os/env_test.go b/pkg/tool/os/env_test.go
index 5b426b7..8b5adef 100644
--- a/pkg/tool/os/env_test.go
+++ b/pkg/tool/os/env_test.go
@@ -38,7 +38,7 @@
CUEOSTESTNUM: 34K
CUEOSTESTUNSET: null
}`)
- _, err := (*setenvCmd).Run(nil, nil, v)
+ _, err := (*setenvCmd).Run(nil, &task.Context{Obj: v})
if err != nil {
t.Fatal(err)
}
@@ -61,7 +61,7 @@
v = parse(t, "tool/os.Setenv", `{
CUEOSTESTMOOD: string
}`)
- _, err = (*setenvCmd).Run(nil, nil, v)
+ _, err = (*setenvCmd).Run(nil, &task.Context{Obj: v})
if err == nil {
t.Fatal("expected incomplete error")
}
@@ -124,7 +124,7 @@
{"tool/os.Environ", &environCmd{}},
} {
v := parse(t, tc.pkg, config)
- got, err := tc.runner.Run(nil, v)
+ got, err := tc.runner.Run(&task.Context{Obj: v})
if err != nil {
t.Fatal(err)
}
@@ -157,7 +157,7 @@
}} {
t.Run(etc.err, func(t *testing.T) {
v = parse(t, tc.pkg, etc.config)
- if _, err = tc.runner.Run(nil, v); err == nil {
+ if _, err = tc.runner.Run(&task.Context{Obj: v}); err == nil {
t.Error(etc.err)
}
})
