Skip to content

Commit

Permalink
feat: refactor schema error handling from Go (#1308)
Browse files Browse the repository at this point in the history
- no longer pass Go compiler errors through the LSP
- instead of attempting to parse out schema/build errors from an
aggregate error, we pass back schema errors and non-schema errors
("real" exceptions) explicitly and separately
- add errors build file to the ModuleConfig

fixes #1299
  • Loading branch information
worstell authored Apr 19, 2024
1 parent d8da724 commit 8544aa0
Show file tree
Hide file tree
Showing 9 changed files with 148 additions and 148 deletions.
14 changes: 6 additions & 8 deletions backend/schema/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,12 @@ func Wrapf(pos Position, endColumn int, err error, format string, args ...any) *
return &e
}

func SortErrorsByPosition(merr []error) {
func SortErrorsByPosition(merr []*Error) {
sort.Slice(merr, func(i, j int) bool {
var ipe, jpe Error
if errors.As(merr[i], &ipe) && errors.As(merr[j], &jpe) {
ipp := ipe.Pos
jpp := jpe.Pos
return ipp.Line < jpp.Line || (ipp.Line == jpp.Line && ipp.Column < jpp.Column) || (ipp.Line == jpp.Line && ipp.Column == jpp.Column && ipe.EndColumn < jpe.EndColumn)
}
return merr[i].Error() < merr[j].Error()
ipp := merr[i].Pos
jpp := merr[j].Pos
return ipp.Line < jpp.Line || (ipp.Line == jpp.Line && ipp.Column < jpp.Column) ||
(ipp.Line == jpp.Line && ipp.Column == jpp.Column && merr[i].EndColumn < merr[j].EndColumn) ||
(ipp.Line == jpp.Line && ipp.Column == jpp.Column && merr[i].EndColumn == merr[j].EndColumn && merr[i].Msg < merr[j].Msg)
})
}
35 changes: 22 additions & 13 deletions buildengine/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

schemapb "github.com/TBD54566975/ftl/backend/protos/xyz/block/ftl/v1/schema"
"github.com/TBD54566975/ftl/backend/schema"
"github.com/TBD54566975/ftl/common/moduleconfig"
"github.com/TBD54566975/ftl/internal/errors"
"github.com/TBD54566975/ftl/internal/log"
"github.com/TBD54566975/ftl/internal/slices"
Expand All @@ -33,6 +34,11 @@ func buildModule(ctx context.Context, sch *schema.Schema, module Module) error {
logger := log.FromContext(ctx).Scope(module.Module)
ctx = log.ContextWithLogger(ctx, logger)

// clear stale module errors before extracting schema
if err := os.RemoveAll(filepath.Join(module.AbsDeployDir(), module.Errors)); err != nil {
return fmt.Errorf("failed to clear errors: %w", err)
}

logger.Infof("Building module")
var err error
switch module.Language {
Expand All @@ -44,18 +50,21 @@ func buildModule(ctx context.Context, sch *schema.Schema, module Module) error {
return fmt.Errorf("unknown language %q", module.Language)
}

var errs []error
if err != nil {
errs := errors.UnwrapAllExcludingIntermediaries(err)
// read runtime-specific build errors from the build directory
errorList, err := loadProtoErrors(module.AbsDeployDir())
if err != nil {
return fmt.Errorf("failed to read build errors for module: %w", err)
}
for _, e := range errorList.Errors {
errs = append(errs, e)
}
errs = errors.DeduplicateErrors(errs)
schema.SortErrorsByPosition(errs)
errs = append(errs, err)
}
// read runtime-specific build errors from the build directory
errorList, err := loadProtoErrors(module.ModuleConfig)
if err != nil {
return fmt.Errorf("failed to read build errors for module: %w", err)
}
schema.SortErrorsByPosition(errorList.Errors)
for _, e := range errorList.Errors {
errs = append(errs, e)
}

if len(errs) > 0 {
return errors.Join(errs...)
}

Expand Down Expand Up @@ -88,8 +97,8 @@ func buildExternalLibrary(ctx context.Context, sch *schema.Schema, lib ExternalL
return nil
}

func loadProtoErrors(buildDir string) (*schema.ErrorList, error) {
f := filepath.Join(buildDir, "errors.pb")
func loadProtoErrors(module moduleconfig.ModuleConfig) (*schema.ErrorList, error) {
f := filepath.Join(module.AbsDeployDir(), module.Errors)
if _, err := os.Stat(f); errors.Is(err, os.ErrNotExist) {
return &schema.ErrorList{Errors: make([]*schema.Error, 0)}, nil
}
Expand Down
15 changes: 6 additions & 9 deletions buildengine/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/alecthomas/assert/v2"

"github.com/TBD54566975/ftl/backend/schema"
"github.com/TBD54566975/ftl/common/moduleconfig"
"github.com/TBD54566975/ftl/internal/log"
)

Expand Down Expand Up @@ -65,10 +66,12 @@ func assertGeneratedModule(generatedModulePath string, expectedContent string) a
func assertBuildProtoErrors(msgs ...string) assertion {
return func(t testing.TB, bctx buildContext) error {
t.Helper()
errorList, err := loadProtoErrors(filepath.Join(bctx.moduleDir, bctx.buildDir))
config, err := moduleconfig.LoadModuleConfig(bctx.moduleDir)
assert.NoError(t, err, "Error loading module config")
errorList, err := loadProtoErrors(config)
assert.NoError(t, err, "Error loading proto errors")

expected := make([]error, 0, len(msgs))
expected := make([]*schema.Error, 0, len(msgs))
for _, msg := range msgs {
expected = append(expected, &schema.Error{Msg: msg})
}
Expand All @@ -78,13 +81,7 @@ func assertBuildProtoErrors(msgs ...string) assertion {
e.EndColumn = 0
}

errs := make([]error, 0, len(errorList.Errors))
for _, e := range errorList.Errors {
errs = append(errs, e)
}
schema.SortErrorsByPosition(errs)

assert.Equal(t, errs, expected, assert.Exclude[schema.Position]())
assert.Equal(t, errorList.Errors, expected, assert.Exclude[schema.Position]())
return nil
}
}
7 changes: 7 additions & 0 deletions buildengine/discover_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ func TestDiscoverModules(t *testing.T) {
Deploy: []string{"main"},
DeployDir: "_ftl",
Schema: "schema.pb",
Errors: "errors.pb",
Watch: []string{"**/*.go", "go.mod", "go.sum"},
},
},
Expand All @@ -36,6 +37,7 @@ func TestDiscoverModules(t *testing.T) {
Deploy: []string{"main"},
DeployDir: "_ftl",
Schema: "schema.pb",
Errors: "errors.pb",
Watch: []string{"**/*.go", "go.mod", "go.sum"},
},
},
Expand All @@ -54,6 +56,7 @@ func TestDiscoverModules(t *testing.T) {
},
DeployDir: "target",
Schema: "schema.pb",
Errors: "errors.pb",
Watch: []string{
"pom.xml",
"src/**",
Expand All @@ -73,6 +76,7 @@ func TestDiscoverModules(t *testing.T) {
},
DeployDir: "_ftl",
Schema: "schema.pb",
Errors: "errors.pb",
Watch: []string{
"**/*.go",
"go.mod",
Expand All @@ -95,6 +99,7 @@ func TestDiscoverModules(t *testing.T) {
},
DeployDir: "target",
Schema: "schema.pb",
Errors: "errors.pb",
Watch: []string{
"pom.xml",
"src/**",
Expand All @@ -111,6 +116,7 @@ func TestDiscoverModules(t *testing.T) {
Deploy: []string{"main"},
DeployDir: "_ftl",
Schema: "schema.pb",
Errors: "errors.pb",
Watch: []string{"**/*.go", "go.mod", "go.sum"},
},
},
Expand All @@ -123,6 +129,7 @@ func TestDiscoverModules(t *testing.T) {
Deploy: []string{"main"},
DeployDir: "_ftl",
Schema: "schema.pb",
Errors: "errors.pb",
Watch: []string{"**/*.go", "go.mod", "go.sum"},
},
},
Expand Down
5 changes: 5 additions & 0 deletions common/moduleconfig/moduleconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ type ModuleConfig struct {
DeployDir string `toml:"deploy-dir"`
// Schema is the name of the schema file relative to the DeployDir.
Schema string `toml:"schema"`
// Errors is the name of the error file relative to the DeployDir.
Errors string `toml:"errors"`
// Watch is the list of files to watch for changes.
Watch []string `toml:"watch"`

Expand Down Expand Up @@ -66,6 +68,9 @@ func setConfigDefaults(moduleDir string, config *ModuleConfig) error {
if config.Schema == "" {
config.Schema = "schema.pb"
}
if config.Errors == "" {
config.Errors = "errors.pb"
}
switch config.Language {
case "kotlin":
if config.Build == "" {
Expand Down
40 changes: 18 additions & 22 deletions go-runtime/compile/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"github.com/TBD54566975/ftl/backend/schema"
"github.com/TBD54566975/ftl/common/moduleconfig"
"github.com/TBD54566975/ftl/internal"
"github.com/TBD54566975/ftl/internal/errors"
"github.com/TBD54566975/ftl/internal/exec"
"github.com/TBD54566975/ftl/internal/log"
"github.com/TBD54566975/ftl/internal/reflect"
Expand Down Expand Up @@ -108,29 +107,15 @@ func Build(ctx context.Context, moduleDir string, sch *schema.Schema) error {

buildDir := buildDir(moduleDir)
logger.Debugf("Extracting schema")
nativeNames, main, err := ExtractModuleSchema(moduleDir)
if originalErr := err; err != nil {
var schemaErrs []*schema.Error
for _, e := range errors.DeduplicateErrors(errors.UnwrapAll(err)) {
var ce *schema.Error
if errors.As(e, &ce) {
schemaErrs = append(schemaErrs, ce)
}
}
el := schema.ErrorList{
Errors: schemaErrs,
}
elBytes, err := proto.Marshal(el.ToProto())
if err != nil {
return fmt.Errorf("failed to marshal errors: %w", err)
}

err = os.WriteFile(filepath.Join(buildDir, "errors.pb"), elBytes, 0600)
if err != nil {
nativeNames, main, schemaErrs, err := ExtractModuleSchema(moduleDir)
if err != nil {
return fmt.Errorf("failed to extract module schema: %w", err)
}
if len(schemaErrs) > 0 {
if err := writeSchemaErrors(config, schemaErrs); err != nil {
return fmt.Errorf("failed to write errors: %w", err)
}

return fmt.Errorf("failed to extract module schema: %w", originalErr)
return nil
}
schemaBytes, err := proto.Marshal(main.ToProto())
if err != nil {
Expand Down Expand Up @@ -409,3 +394,14 @@ func shouldUpdateVersion(goModfile *modfile.File) bool {
}
return true
}

func writeSchemaErrors(config moduleconfig.ModuleConfig, errors []*schema.Error) error {
el := schema.ErrorList{
Errors: errors,
}
elBytes, err := proto.Marshal(el.ToProto())
if err != nil {
return fmt.Errorf("failed to marshal errors: %w", err)
}
return os.WriteFile(filepath.Join(config.AbsDeployDir(), config.Errors), elBytes, 0600)
}
Loading

0 comments on commit 8544aa0

Please sign in to comment.