From 60365984a254034e12c28a033187f83db31837b7 Mon Sep 17 00:00:00 2001 From: Alec Thomas Date: Mon, 12 Feb 2024 15:00:37 +1100 Subject: [PATCH] fix: improve Go schema extraction errors Previously: ``` failed to build module: failed to extract module schema: ftl/foo: /Users/alec/dev/example/backend/modules/foo/foo.go:14:2: could not import ftl/bar (invalid package name: "") ``` Now: ``` failed to build module: failed to extract module schema: /Users/alec/dev/example/backend/modules/foo/foo.go:2:1: /Users/alec/dev/example/backend/modules/foo/foo.go:88:1: /Users/alec/dev/example/backend/modules/foo/foo.go:88:138: /Users/alec/dev/example/backend/modules/foo/foo.go:106:2: /Users/alec/dev/example/backend/modules/foo/foo.go:106:23: call first argument must be a function but is an unresolved reference to bar.BarRequest ``` --- .github/workflows/workflow-roadmap.yml | 2 ++ examples/go/httpingress/httpingress.go | 38 +++++++++++++------------- go-runtime/compile/build.go | 3 +- go-runtime/compile/schema.go | 13 +++++++-- 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/.github/workflows/workflow-roadmap.yml b/.github/workflows/workflow-roadmap.yml index 7579521046..5a7c9bb708 100644 --- a/.github/workflows/workflow-roadmap.yml +++ b/.github/workflows/workflow-roadmap.yml @@ -4,6 +4,8 @@ on: types: - assigned - labeled + - closed + - deleted workflow_dispatch: # We need to separately trigger when one of the other workflows completes # because GHA won't trigger another workflow based only on changes from diff --git a/examples/go/httpingress/httpingress.go b/examples/go/httpingress/httpingress.go index f661764ad1..d5109169e9 100644 --- a/examples/go/httpingress/httpingress.go +++ b/examples/go/httpingress/httpingress.go @@ -6,6 +6,8 @@ import ( "fmt" "ftl/builtin" + + "github.com/TBD54566975/ftl/go-runtime/sdk" ) type GetRequest struct { @@ -26,13 +28,13 @@ type GetResponse struct { // //ftl:verb //ftl:ingress http GET /http/users/{userID}/posts -func Get(ctx context.Context, req builtin.HttpRequest[GetRequest]) (builtin.HttpResponse[GetResponse], error) { - return builtin.HttpResponse[GetResponse]{ +func Get(ctx context.Context, req builtin.HttpRequest[GetRequest]) (builtin.HttpResponse[GetResponse, sdk.Unit], error) { + return builtin.HttpResponse[GetResponse, sdk.Unit]{ Headers: map[string][]string{"Get": {"Header from FTL"}}, - Body: GetResponse{ + Body: sdk.Some(GetResponse{ Message: fmt.Sprintf("Got userId %s and postId %s", req.Body.UserID, req.Body.PostID), Nested: Nested{GoodStuff: "Nested Good Stuff"}, - }, + }), }, nil } @@ -49,11 +51,11 @@ type PostResponse struct { // //ftl:verb //ftl:ingress http POST /http/users -func Post(ctx context.Context, req builtin.HttpRequest[PostRequest]) (builtin.HttpResponse[PostResponse], error) { - return builtin.HttpResponse[PostResponse]{ +func Post(ctx context.Context, req builtin.HttpRequest[PostRequest]) (builtin.HttpResponse[PostResponse, sdk.Unit], error) { + return builtin.HttpResponse[PostResponse, sdk.Unit]{ Status: 201, Headers: map[string][]string{"Post": {"Header from FTL"}}, - Body: PostResponse{Success: true}, + Body: sdk.Some(PostResponse{Success: true}), }, nil } @@ -68,10 +70,9 @@ type PutResponse struct{} // //ftl:verb //ftl:ingress http PUT /http/users/{userID} -func Put(ctx context.Context, req builtin.HttpRequest[PutRequest]) (builtin.HttpResponse[PutResponse], error) { - return builtin.HttpResponse[PutResponse]{ +func Put(ctx context.Context, req builtin.HttpRequest[PutRequest]) (builtin.HttpResponse[PutResponse, sdk.Unit], error) { + return builtin.HttpResponse[PutResponse, sdk.Unit]{ Headers: map[string][]string{"Put": {"Header from FTL"}}, - Body: PutResponse{}, }, nil } @@ -85,10 +86,9 @@ type DeleteResponse struct{} // //ftl:verb //ftl:ingress http DELETE /http/users/{userID} -func Delete(ctx context.Context, req builtin.HttpRequest[DeleteRequest]) (builtin.HttpResponse[DeleteResponse], error) { - return builtin.HttpResponse[DeleteResponse]{ +func Delete(ctx context.Context, req builtin.HttpRequest[DeleteRequest]) (builtin.HttpResponse[DeleteResponse, sdk.Unit], error) { + return builtin.HttpResponse[DeleteResponse, sdk.Unit]{ Headers: map[string][]string{"Put": {"Header from FTL"}}, - Body: DeleteResponse{}, }, nil } @@ -96,10 +96,10 @@ type HtmlRequest struct{} //ftl:verb //ftl:ingress http GET /http/html -func Html(ctx context.Context, req builtin.HttpRequest[HtmlRequest]) (builtin.HttpResponse[string], error) { - return builtin.HttpResponse[string]{ +func Html(ctx context.Context, req builtin.HttpRequest[HtmlRequest]) (builtin.HttpResponse[string, sdk.Unit], error) { + return builtin.HttpResponse[string, sdk.Unit]{ Headers: map[string][]string{"Content-Type": {"text/html; charset=utf-8"}}, - Body: "

HTML Page From FTL 🚀!

", + Body: sdk.Some("

HTML Page From FTL 🚀!

"), }, nil } @@ -107,9 +107,9 @@ func Html(ctx context.Context, req builtin.HttpRequest[HtmlRequest]) (builtin.Ht // //ftl:verb //ftl:ingress http POST /http/bytes -func Bytes(ctx context.Context, req builtin.HttpRequest[[]byte]) (builtin.HttpResponse[[]byte], error) { - return builtin.HttpResponse[[]byte]{ +func Bytes(ctx context.Context, req builtin.HttpRequest[[]byte]) (builtin.HttpResponse[[]byte, sdk.Unit], error) { + return builtin.HttpResponse[[]byte, sdk.Unit]{ Headers: map[string][]string{"Content-Type": {"application/octet-stream"}}, - Body: req.Body, + Body: sdk.Some(req.Body), }, nil } diff --git a/go-runtime/compile/build.go b/go-runtime/compile/build.go index cd0aa42a16..aca7cde428 100644 --- a/go-runtime/compile/build.go +++ b/go-runtime/compile/build.go @@ -10,10 +10,11 @@ import ( "reflect" "strings" - "github.com/TBD54566975/scaffolder" "golang.org/x/mod/modfile" "google.golang.org/protobuf/proto" + "github.com/TBD54566975/scaffolder" + "github.com/TBD54566975/ftl" "github.com/TBD54566975/ftl/backend/common/exec" "github.com/TBD54566975/ftl/backend/common/log" diff --git a/go-runtime/compile/schema.go b/go-runtime/compile/schema.go index abf25e1e50..c5e04fb7c9 100644 --- a/go-runtime/compile/schema.go +++ b/go-runtime/compile/schema.go @@ -50,9 +50,12 @@ func ExtractModuleSchema(dir string) (NativeNames, *schema.Module, error) { } nativeNames := NativeNames{} module := &schema.Module{} + merr := []error{} for _, pkg := range pkgs { if len(pkg.Errors) > 0 { - return nil, nil, fmt.Errorf("%s: %w", pkg.PkgPath, pkg.Errors[0]) + for _, perr := range pkg.Errors { + merr = append(merr, fmt.Errorf("%s: %w", pkg.PkgPath, perr)) + } } pctx := &parseContext{pkg: pkg, pkgs: pkgs, module: module, nativeNames: NativeNames{}} for _, file := range pkg.Syntax { @@ -102,6 +105,9 @@ func ExtractModuleSchema(dir string) (NativeNames, *schema.Module, error) { nativeNames[decl] = nativeName } } + if len(merr) > 0 { + return nil, nil, errors.Join(merr...) + } if module.Name == "" { return nil, nil, fmt.Errorf("//ftl:module directive is required") } @@ -121,7 +127,10 @@ func visitCallExpr(pctx *parseContext, verb *schema.Verb, node *ast.CallExpr) er } _, verbFn := deref[*types.Func](pctx.pkg, node.Args[1]) if verbFn == nil { - return fmt.Errorf("call first argument must be a function but is %s", node.Args[1]) + 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 fmt.Errorf("call first argument must be a function but is %T", node.Args[1]) } moduleName := verbFn.Pkg().Name() if moduleName == pctx.pkg.Name {