From 295e46fc0f8d7a42bfd256eb634c77826c1bd67c Mon Sep 17 00:00:00 2001 From: Romain Marcadier Date: Mon, 16 Dec 2024 11:54:51 +0100 Subject: [PATCH] feat: improve context in error advice messages Provide more context information in the error messages propagated from advice application errors, so that they are easier to troubleshoot. These messages aim to provide a lineage that allows maintainers to know exactly what advice failed, on what file. --- internal/injector/aspect/advice/assign.go | 4 ++-- internal/injector/aspect/advice/block.go | 4 ++-- internal/injector/aspect/advice/call.go | 4 ++-- internal/injector/aspect/advice/inject.go | 4 +++- internal/injector/aspect/advice/wrap.go | 4 ++-- internal/injector/injector.go | 6 +++--- 6 files changed, 14 insertions(+), 12 deletions(-) diff --git a/internal/injector/aspect/advice/assign.go b/internal/injector/aspect/advice/assign.go index c975890c9..93947cd1f 100644 --- a/internal/injector/aspect/advice/assign.go +++ b/internal/injector/aspect/advice/assign.go @@ -26,12 +26,12 @@ func AssignValue(template *code.Template) *assignValue { func (a *assignValue) Apply(ctx context.AdviceContext) (bool, error) { spec, ok := ctx.Node().(*dst.ValueSpec) if !ok { - return false, fmt.Errorf("expected *dst.ValueSpec, got %T", ctx.Node()) + return false, fmt.Errorf("assign-value: expected *dst.ValueSpec, got %T", ctx.Node()) } expr, err := a.Template.CompileExpression(ctx) if err != nil { - return false, err + return false, fmt.Errorf("assign-value: %w", err) } spec.Values = make([]dst.Expr, len(spec.Names)) diff --git a/internal/injector/aspect/advice/block.go b/internal/injector/aspect/advice/block.go index 5b6e93cb0..02b34a55b 100644 --- a/internal/injector/aspect/advice/block.go +++ b/internal/injector/aspect/advice/block.go @@ -29,12 +29,12 @@ func PrependStmts(template *code.Template) *prependStatements { func (a *prependStatements) Apply(ctx context.AdviceContext) (bool, error) { block, ok := ctx.Node().(*dst.BlockStmt) if !ok { - return false, fmt.Errorf("expected *dst.BlockStmt, got %T", ctx.Node()) + return false, fmt.Errorf("prepend-statements: expected *dst.BlockStmt, got %T", ctx.Node()) } stmts, err := a.Template.CompileBlock(ctx) if err != nil { - return false, err + return false, fmt.Errorf("prepend-statements: %w", err) } list := make([]dst.Stmt, 1+len(block.List)) diff --git a/internal/injector/aspect/advice/call.go b/internal/injector/aspect/advice/call.go index f3bbf2f0b..a9f0158c9 100644 --- a/internal/injector/aspect/advice/call.go +++ b/internal/injector/aspect/advice/call.go @@ -32,7 +32,7 @@ func AppendArgs(typeName join.TypeName, templates ...*code.Template) *appendArgs func (a *appendArgs) Apply(ctx context.AdviceContext) (bool, error) { call, ok := ctx.Node().(*dst.CallExpr) if !ok { - return false, fmt.Errorf("expected a *dst.CallExpr, received %T", ctx.Node()) + return false, fmt.Errorf("append-arguments: expected a *dst.CallExpr, received %T", ctx.Node()) } newArgs := make([]dst.Expr, len(a.Templates)) @@ -40,7 +40,7 @@ func (a *appendArgs) Apply(ctx context.AdviceContext) (bool, error) { for i, t := range a.Templates { newArgs[i], err = t.CompileExpression(ctx) if err != nil { - return false, err + return false, fmt.Errorf("append-arguments[%d]: %w", i, err) } ctx.EnsureMinGoLang(t.Lang) } diff --git a/internal/injector/aspect/advice/inject.go b/internal/injector/aspect/advice/inject.go index 110e54db6..1be6baeb8 100644 --- a/internal/injector/aspect/advice/inject.go +++ b/internal/injector/aspect/advice/inject.go @@ -6,6 +6,8 @@ package advice import ( + "fmt" + "github.com/DataDog/orchestrion/internal/fingerprint" "github.com/DataDog/orchestrion/internal/injector/aspect/advice/code" "github.com/DataDog/orchestrion/internal/injector/aspect/context" @@ -26,7 +28,7 @@ func InjectDeclarations(template *code.Template, links []string) injectDeclarati func (a injectDeclarations) Apply(ctx context.AdviceContext) (bool, error) { decls, err := a.Template.CompileDeclarations(ctx) if err != nil { - return false, err + return false, fmt.Errorf("inject-declarations: %w", err) } if len(decls) == 0 { diff --git a/internal/injector/aspect/advice/wrap.go b/internal/injector/aspect/advice/wrap.go index 471c40289..f3196cb6e 100644 --- a/internal/injector/aspect/advice/wrap.go +++ b/internal/injector/aspect/advice/wrap.go @@ -33,12 +33,12 @@ func (a *wrapExpression) Apply(ctx context.AdviceContext) (bool, error) { ctx = ctx.Child(kve.Value, "Value", -1) defer ctx.Release() } else if _, ok = ctx.Node().(dst.Expr); !ok { - return false, fmt.Errorf("expected dst.Expr or *dst.KeyValueExpr, got %T", ctx.Node()) + return false, fmt.Errorf("wrap-expression: expected dst.Expr or *dst.KeyValueExpr, got %T", ctx.Node()) } repl, err := a.Template.CompileExpression(ctx) if err != nil { - return false, err + return false, fmt.Errorf("wrap-expression: %w", err) } if kve == nil { diff --git a/internal/injector/injector.go b/internal/injector/injector.go index 9b77a2f26..35892e6b1 100644 --- a/internal/injector/injector.go +++ b/internal/injector/injector.go @@ -159,7 +159,7 @@ func (i *Injector) injectFile(decorator *decorator.Decorator, file *dst.File, ty var err error result.Modified, result.References, result.GoLang, err = i.applyAspects(decorator, file, i.RootConfig, typeInfo) if err != nil { - return result, err + return result, fmt.Errorf("%q: %w", result.Filename, err) } if result.Modified { @@ -239,14 +239,14 @@ func (i *Injector) injectNode(ctx context.AdviceContext) (mod bool, err error) { if !inj.JoinPoint.Matches(ctx) { continue } - for _, act := range inj.Advice { + for idx, act := range inj.Advice { var changed bool changed, err = act.Apply(ctx) if changed { mod = true } if err != nil { - return + return mod, fmt.Errorf("%q[%d]: %w", inj.ID, idx, err) } } }