Skip to content

Commit

Permalink
dockerfile: add more source information to errors
Browse files Browse the repository at this point in the history
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
  • Loading branch information
tonistiigi committed Apr 22, 2020
1 parent 10c1ee1 commit 2c859b0
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 58 deletions.
2 changes: 1 addition & 1 deletion frontend/dockerfile/builder/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ func Build(ctx context.Context, c client.Client) (*client.Result, error) {
if isNotLocalDockerfile {
localNameDockerfile = ""
}
err = wrapSource(err, dtDockerfile, filename, localNameDockerfile, el.Ranges)
err = wrapSource(err, dtDockerfile, filename, localNameDockerfile, el.Location)
}
}()
st, img, err := dockerfile2llb.Dockerfile2LLB(ctx, dtDockerfile, dockerfile2llb.ConvertOpt{
Expand Down
20 changes: 10 additions & 10 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,10 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State,
for i, st := range stages {
name, err := shlex.ProcessWordWithMap(st.BaseName, metaArgsToMap(optMetaArgs))
if err != nil {
return nil, nil, err
return nil, nil, parser.WithLocation(err, st.Location)
}
if name == "" {
return nil, nil, errors.Errorf("base name (%s) should not be blank", st.BaseName)
return nil, nil, parser.WithLocation(errors.Errorf("base name (%s) should not be blank", st.BaseName), st.Location)
}
st.BaseName = name

Expand All @@ -132,12 +132,12 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State,
if v := st.Platform; v != "" {
v, err := shlex.ProcessWordWithMap(v, metaArgsToMap(optMetaArgs))
if err != nil {
return nil, nil, errors.Wrapf(err, "failed to process arguments for platform %s", v)
return nil, nil, parser.WithLocation(errors.Wrapf(err, "failed to process arguments for platform %s", v), st.Location)
}

p, err := platforms.Parse(v)
if err != nil {
return nil, nil, errors.Wrapf(err, "failed to parse platform %s", v)
return nil, nil, parser.WithLocation(errors.Wrapf(err, "failed to parse platform %s", v), st.Location)
}
ds.platform = &p
}
Expand Down Expand Up @@ -204,7 +204,7 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State,
}

if has, state := hasCircularDependency(allDispatchStates.states); has {
return nil, nil, fmt.Errorf("circular dependency detected on stage: %s", state.stageName)
return nil, nil, errors.Errorf("circular dependency detected on stage: %s", state.stageName)
}

if len(allDispatchStates.states) == 1 {
Expand All @@ -225,7 +225,7 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State,
eg.Go(func() error {
ref, err := reference.ParseNormalizedNamed(d.stage.BaseName)
if err != nil {
return errors.Wrapf(err, "failed to parse stage name %q", d.stage.BaseName)
return parser.WithLocation(errors.Wrapf(err, "failed to parse stage name %q", d.stage.BaseName), d.stage.Location)
}
platform := d.platform
if platform == nil {
Expand Down Expand Up @@ -316,12 +316,12 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State,
}
if d.image.Config.WorkingDir != "" {
if err = dispatchWorkdir(d, &instructions.WorkdirCommand{Path: d.image.Config.WorkingDir}, false, nil); err != nil {
return nil, nil, err
return nil, nil, parser.WithLocation(err, d.stage.Location)
}
}
if d.image.Config.User != "" {
if err = dispatchUser(d, &instructions.UserCommand{User: d.image.Config.User}, false); err != nil {
return nil, nil, err
return nil, nil, parser.WithLocation(err, d.stage.Location)
}
}
d.state = d.state.Network(opt.ForceNetMode)
Expand All @@ -346,13 +346,13 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State,
}

if err = dispatchOnBuildTriggers(d, d.image.Config.OnBuild, opt); err != nil {
return nil, nil, err
return nil, nil, parser.WithLocation(err, d.stage.Location)
}
d.image.Config.OnBuild = nil

for _, cmd := range d.commands {
if err := dispatch(d, cmd, opt); err != nil {
return nil, nil, err
return nil, nil, parser.WithLocation(err, cmd.Location())
}
}

Expand Down
15 changes: 12 additions & 3 deletions frontend/dockerfile/instructions/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/strslice"
"github.com/moby/buildkit/frontend/dockerfile/parser"
"github.com/pkg/errors"
)

Expand Down Expand Up @@ -35,15 +36,17 @@ func (kvpo *KeyValuePairOptional) ValueString() string {
// Command is implemented by every command present in a dockerfile
type Command interface {
Name() string
Location() []parser.Range
}

// KeyValuePairs is a slice of KeyValuePair
type KeyValuePairs []KeyValuePair

// withNameAndCode is the base of every command in a Dockerfile (String() returns its source code)
type withNameAndCode struct {
code string
name string
code string
name string
location []parser.Range
}

func (c *withNameAndCode) String() string {
Expand All @@ -55,8 +58,13 @@ func (c *withNameAndCode) Name() string {
return c.name
}

// Location of the command in source
func (c *withNameAndCode) Location() []parser.Range {
return c.location
}

func newWithNameAndCode(req parseRequest) withNameAndCode {
return withNameAndCode{code: strings.TrimSpace(req.original), name: req.command}
return withNameAndCode{code: strings.TrimSpace(req.original), name: req.command, location: req.location}
}

// SingleWordExpander is a provider for variable expansion where 1 word => 1 output
Expand Down Expand Up @@ -400,6 +408,7 @@ type Stage struct {
BaseName string
SourceCode string
Platform string
Location []parser.Range
}

// AddCommand to the stage
Expand Down
13 changes: 8 additions & 5 deletions frontend/dockerfile/instructions/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type parseRequest struct {
attributes map[string]bool
flags *BFlags
original string
location []parser.Range
}

var parseRunPreHooks []func(*RunCommand, parseRequest) error
Expand Down Expand Up @@ -48,13 +49,14 @@ func newParseRequestFromNode(node *parser.Node) parseRequest {
attributes: node.Attributes,
original: node.Original,
flags: NewBFlagsWithArgs(node.Flags),
location: node.Location(),
}
}

// ParseInstruction converts an AST to a typed instruction (either a command or a build stage beginning when encountering a `FROM` statement)
func ParseInstruction(node *parser.Node) (v interface{}, err error) {
defer func() {
err = node.WrapError(err)
err = parser.WithLocation(err, node.Location())
}()
req := newParseRequestFromNode(node)
switch node.Value {
Expand Down Expand Up @@ -108,7 +110,7 @@ func ParseCommand(node *parser.Node) (Command, error) {
if c, ok := s.(Command); ok {
return c, nil
}
return nil, node.WrapError(errors.Errorf("%T is not a command type", s))
return nil, parser.WithLocation(errors.Errorf("%T is not a command type", s), node.Location())
}

// UnknownInstruction represents an error occurring when a command is unresolvable
Expand All @@ -130,7 +132,7 @@ func (e *parseError) Error() string {
return fmt.Sprintf("dockerfile parse error line %d: %v", e.node.StartLine, e.inner.Error())
}

func (e *parseError) Unwarp() error {
func (e *parseError) Unwrap() error {
return e.inner
}

Expand All @@ -155,11 +157,11 @@ func Parse(ast *parser.Node) (stages []Stage, metaArgs []ArgCommand, err error)
case Command:
stage, err := CurrentStage(stages)
if err != nil {
return nil, nil, n.WrapError(err)
return nil, nil, parser.WithLocation(err, n.Location())
}
stage.AddCommand(c)
default:
return nil, nil, n.WrapError(errors.Errorf("%T is not a command type", cmd))
return nil, nil, parser.WithLocation(errors.Errorf("%T is not a command type", cmd), n.Location())
}

}
Expand Down Expand Up @@ -282,6 +284,7 @@ func parseFrom(req parseRequest) (*Stage, error) {
SourceCode: code,
Commands: []Command{},
Platform: flPlatform.Value,
Location: req.location,
}, nil

}
Expand Down
39 changes: 33 additions & 6 deletions frontend/dockerfile/parser/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,37 +2,50 @@ package parser

import "github.com/pkg/errors"

// ErrorLocation gives a location in source code that caused the error
type ErrorLocation struct {
Ranges []Range
Location []Range
error
}

// Unwrap unwraps to the next error
func (e *ErrorLocation) Unwrap() error {
return e.error
}

// Range is a code section between two positions
type Range struct {
Start Position
End Position
}

// Position is a point in source code
type Position struct {
Line int
Character int
}

func withLocation(err error, startLine, endLine int) error {
func withLocation(err error, start, end int) error {
return WithLocation(err, toRanges(start, end))
}

// WithLocation extends an error with a source code location
func WithLocation(err error, location []Range) error {
if err == nil {
return nil
}
var el *ErrorLocation
if errors.As(err, &el) {
return err
}
return errors.WithStack(&ErrorLocation{
error: err,
Ranges: toRanges(startLine, endLine),
})
var err1 error = &ErrorLocation{
error: err,
Location: location,
}
if !hasLocalStackTrace(err) {
err1 = errors.WithStack(err1)
}
return err1
}

func toRanges(start, end int) (r []Range) {
Expand All @@ -44,3 +57,17 @@ func toRanges(start, end int) (r []Range) {
}
return
}

func hasLocalStackTrace(err error) bool {
wrapped, ok := err.(interface {
Unwrap() error
})
if ok && hasLocalStackTrace(wrapped.Unwrap()) {
return true
}

_, ok = err.(interface {
StackTrace() errors.StackTrace
})
return ok
}
10 changes: 5 additions & 5 deletions frontend/dockerfile/parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ type Node struct {
EndLine int // the line in the original dockerfile where the node ends
}

// Location return the location of node in source code
func (node *Node) Location() []Range {
return toRanges(node.StartLine, node.EndLine)
}

// Dump dumps the AST defined by `node` as a list of sexps.
// Returns a string suitable for printing.
func (node *Node) Dump() string {
Expand All @@ -63,11 +68,6 @@ func (node *Node) Dump() string {
return strings.TrimSpace(str)
}

// WrapError returns new error that implements ErrorLocation
func (node *Node) WrapError(err error) error {
return withLocation(err, node.StartLine, node.EndLine)
}

func (node *Node) lines(start, end int) {
node.StartLine = start
node.EndLine = end
Expand Down
50 changes: 22 additions & 28 deletions solver/errdefs/errdefs.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 2c859b0

Please sign in to comment.