Skip to content

Commit

Permalink
fix(go-runtime): remove duplicate positional information in errors (#…
Browse files Browse the repository at this point in the history
…1064)

Fixes #1062
  • Loading branch information
alecthomas authored Mar 12, 2024
1 parent 0e4ea0a commit 4221f57
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 13 deletions.
40 changes: 40 additions & 0 deletions go-runtime/compile/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package compile

import (
"errors"
"fmt"
"go/ast"

"github.com/TBD54566975/ftl/backend/schema"
)

type Error struct {
Msg string
Pos schema.Position
Err error // Wrapped error, if any
}

func (e Error) Error() string { return fmt.Sprintf("%s: %s", e.Pos, e.Msg) }
func (e Error) Unwrap() error { return e.Err }

func errorf(node ast.Node, format string, args ...any) Error {
return Error{Msg: fmt.Sprintf(format, args...), Pos: goPosToSchemaPos(node.Pos())}
}

func wrapf(node ast.Node, err error, format string, args ...any) Error {
if format == "" {
format = "%s"
} else {
format += ": %s"
}
// Propagate existing error position if available
var pos schema.Position
if perr := (Error{}); errors.As(err, &perr) {
pos = perr.Pos
args = append(args, perr.Msg)
} else {
pos = goPosToSchemaPos(node.Pos())
args = append(args, err)
}
return Error{Msg: fmt.Sprintf(format, args...), Pos: pos, Err: err}
}
30 changes: 17 additions & 13 deletions go-runtime/compile/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,19 @@ func ExtractModuleSchema(dir string) (NativeNames, *schema.Module, error) {
for _, pkg := range pkgs {
if len(pkg.Errors) > 0 {
for _, perr := range pkg.Errors {
merr = append(merr, fmt.Errorf("%s: %w", pkg.PkgPath, perr))
if len(pkg.Syntax) > 0 {
merr = append(merr, wrapf(pkg.Syntax[0], perr, "%s", pkg.PkgPath))
} else {
merr = append(merr, fmt.Errorf("%s: %w", pkg.PkgPath, perr))
}
}
}
pctx := &parseContext{pkg: pkg, pkgs: pkgs, module: module, nativeNames: NativeNames{}, enums: enums{}}
for _, file := range pkg.Syntax {
err := goast.Visit(file, func(node ast.Node, next func() error) (err error) {
defer func() {
if err != nil {
err = fmt.Errorf("%s: %w", fset.Position(node.Pos()).String(), err)
err = wrapf(node, err, "")
}
}()
switch node := node.(type) {
Expand Down Expand Up @@ -149,14 +153,14 @@ func visitCallExpr(pctx *parseContext, node *ast.CallExpr) error {

func parseCall(pctx *parseContext, node *ast.CallExpr) error {
if len(node.Args) != 3 {
return fmt.Errorf("%s: call must have exactly three arguments", goPosToSchemaPos(node.Pos()))
return errorf(node, "call must have exactly three arguments")
}
_, verbFn := deref[*types.Func](pctx.pkg, node.Args[1])
if verbFn == nil {
if sel, ok := node.Args[1].(*ast.SelectorExpr); ok {
return fmt.Errorf("call first argument must be a function but is an unresolved reference to %s.%s", sel.X, sel.Sel)
return errorf(node.Args[1], "call first argument must be a function but is an unresolved reference to %s.%s", sel.X, sel.Sel)
}
return fmt.Errorf("call first argument must be a function but is %T", node.Args[1])
return errorf(node.Args[1], "call first argument must be a function but is %T", node.Args[1])
}
if pctx.activeVerb == nil {
return nil
Expand All @@ -181,12 +185,12 @@ func parseConfigDecl(pctx *parseContext, node *ast.CallExpr, fn *types.Func) err
var err error
name, err = strconv.Unquote(literal.Value)
if err != nil {
return fmt.Errorf("%s: %w", goPosToSchemaPos(node.Pos()), err)
return wrapf(node, err, "")
}
}
}
if name == "" {
return fmt.Errorf("%s: config and secret declarations must have a single string literal argument", goPosToSchemaPos(node.Pos()))
return errorf(node, "config and secret declarations must have a single string literal argument")
}
index := node.Fun.(*ast.IndexExpr) //nolint:forcetypeassert

Expand Down Expand Up @@ -229,12 +233,12 @@ func visitFile(pctx *parseContext, node *ast.File) error {
switch dir := dir.(type) {
case *directiveModule:
if dir.Name != pctx.pkg.Name {
return fmt.Errorf("%s: FTL module name %q does not match Go package name %q", dir, dir.Name, pctx.pkg.Name)
return errorf(node, "%s: FTL module name %q does not match Go package name %q", dir, dir.Name, pctx.pkg.Name)
}
pctx.module.Name = dir.Name

default:
return fmt.Errorf("%s: invalid directive", dir)
return errorf(node, "%s: invalid directive", dir)
}
}
return nil
Expand Down Expand Up @@ -442,7 +446,7 @@ func visitFuncDecl(pctx *parseContext, node *ast.FuncDecl) (verb *schema.Verb, e
results := sig.Results()
reqt, respt, err := checkSignature(sig)
if err != nil {
return nil, err
return nil, wrapf(node, err, "")
}
var req schema.Type
if reqt != nil {
Expand Down Expand Up @@ -688,7 +692,7 @@ func visitType(pctx *parseContext, node ast.Node, tnode types.Type) (schema.Type
return &schema.Float{Pos: goPosToSchemaPos(node.Pos())}, nil

default:
return nil, fmt.Errorf("unsupported basic type %s", underlying)
return nil, errorf(node, "unsupported basic type %s", underlying)
}

case *types.Struct:
Expand Down Expand Up @@ -726,10 +730,10 @@ func visitType(pctx *parseContext, node ast.Node, tnode types.Type) (schema.Type
if underlying.String() == "any" {
return &schema.Any{Pos: goPosToSchemaPos(node.Pos())}, nil
}
return nil, fmt.Errorf("%s: unsupported type %T", goPosToSchemaPos(node.Pos()), node)
return nil, errorf(node, "unsupported type %T", node)

default:
return nil, fmt.Errorf("%s: unsupported type %T", goPosToSchemaPos(node.Pos()), node)
return nil, errorf(node, "unsupported type %T", node)
}
}

Expand Down
8 changes: 8 additions & 0 deletions go-runtime/compile/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package compile
import (
"go/ast"
"go/types"
"os"
"path/filepath"
"strings"
"testing"

Expand Down Expand Up @@ -175,3 +177,9 @@ func TestParseBasicTypes(t *testing.T) {
func normaliseString(s string) string {
return strings.TrimSpace(strings.Join(slices.Map(strings.Split(s, "\n"), strings.TrimSpace), "\n"))
}

func TestErrorReporting(t *testing.T) {
pwd, _ := os.Getwd()
_, _, err := ExtractModuleSchema("testdata/failing")
assert.EqualError(t, err, filepath.Join(pwd, `testdata/failing/failing.go`)+`:15:2: call must have exactly three arguments`)
}
17 changes: 17 additions & 0 deletions go-runtime/compile/testdata/failing/failing.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//ftl:module failing
package failing

import (
"context"

"github.com/TBD54566975/ftl/go-runtime/ftl"
)

type Request struct{}
type Response struct{}

//ftl:verb
func FailingVerb(ctx context.Context, req Request) (Response, error) {
ftl.Call(ctx, "failing", "failingVerb", req)
return Response{}, nil
}

0 comments on commit 4221f57

Please sign in to comment.