Skip to content

Commit

Permalink
provider functions return an error
Browse files Browse the repository at this point in the history
The call site for language functions doesn't currently have a way to
handle complex diagnostics, so rather than appear to support them in the
protocol we remove the concepts of diagnostics for now. We do however
retain the argument index fields, which we can wrap in a
function.ArgError and get a little more precise hcl diagnostic from
expression.
  • Loading branch information
jbardin committed Jan 31, 2024
1 parent f44e743 commit c34c53e
Show file tree
Hide file tree
Showing 12 changed files with 2,328 additions and 2,099 deletions.
8 changes: 7 additions & 1 deletion docs/plugin-protocol/tfplugin5.5.proto
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ message Diagnostic {
string summary = 2;
string detail = 3;
AttributePath attribute = 4;
}

message FunctionDiagnostic {
string summary = 2;
string detail = 3;
// function_argument is the positional function argument for aligning
// configuration source.
optional int64 function_argument = 5;
Expand Down Expand Up @@ -569,6 +573,8 @@ message CallFunction {
}
message Response {
DynamicValue result = 1;
repeated Diagnostic diagnostics = 2;
// since we don't support true diagnostics yet, only accept a single
// value and label it as error.
FunctionDiagnostic error = 2;
}
}
8 changes: 7 additions & 1 deletion docs/plugin-protocol/tfplugin6.5.proto
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ message Diagnostic {
string summary = 2;
string detail = 3;
AttributePath attribute = 4;
}

message FunctionDiagnostic {
string summary = 2;
string detail = 3;
// function_argument is the positional function argument for aligning
// configuration source.
optional int64 function_argument = 5;
Expand Down Expand Up @@ -549,6 +553,8 @@ message CallFunction {
}
message Response {
DynamicValue result = 1;
repeated Diagnostic diagnostics = 2;
// since we don't support true diagnostics yet, only accept a single
// value and label it as error.
FunctionDiagnostic error = 2;
}
}
38 changes: 31 additions & 7 deletions internal/grpcwrap/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"

"github.com/zclconf/go-cty/cty"
"github.com/zclconf/go-cty/cty/function"
ctyjson "github.com/zclconf/go-cty/cty/json"
"github.com/zclconf/go-cty/cty/msgpack"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -444,25 +445,34 @@ func (p *provider) CallFunction(_ context.Context, req *tfplugin5.CallFunction_R
if len(req.Arguments) != 0 {
args = make([]cty.Value, len(req.Arguments))
for i, rawArg := range req.Arguments {
idx := int64(i)

var argTy cty.Type
if i < len(funcSchema.Parameters) {
argTy = funcSchema.Parameters[i].Type
} else {
if funcSchema.VariadicParameter == nil {
resp.Diagnostics = convert.AppendProtoDiag(
resp.Diagnostics, fmt.Errorf("too many arguments for non-variadic function"),
)

resp.Error = &tfplugin5.FunctionDiagnostic{
Summary: fmt.Sprintf("Call to function %q failed", req.Name),
Detail: "too many arguments for non-variadic function",
FunctionArgument: &idx,
}
return resp, nil
}
argTy = funcSchema.VariadicParameter.Type
}

argVal, err := decodeDynamicValue(rawArg, argTy)
if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
resp.Error = &tfplugin5.FunctionDiagnostic{
Summary: fmt.Sprintf("Call to function %q failed", req.Name),
Detail: err.Error(),
FunctionArgument: &idx,
}
return resp, nil
}

args[i] = argVal
}
}
Expand All @@ -471,14 +481,28 @@ func (p *provider) CallFunction(_ context.Context, req *tfplugin5.CallFunction_R
FunctionName: req.Name,
Arguments: args,
})
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, callResp.Diagnostics)
if callResp.Diagnostics.HasErrors() {

if callResp.Err != nil {
resp.Error = &tfplugin5.FunctionDiagnostic{
Summary: fmt.Sprintf("Call to function %q failed", req.Name),
Detail: callResp.Err.Error(),
}

if argErr, ok := callResp.Err.(function.ArgError); ok {
idx := int64(argErr.Index)
resp.Error.FunctionArgument = &idx
}

return resp, nil
}

resp.Result, err = encodeDynamicValue(callResp.Result, funcSchema.ReturnType)
if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
resp.Error = &tfplugin5.FunctionDiagnostic{
Summary: fmt.Sprintf("Call to function %q failed", req.Name),
Detail: err.Error(),
}

return resp, nil
}

Expand Down
36 changes: 29 additions & 7 deletions internal/grpcwrap/provider6.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"

"github.com/zclconf/go-cty/cty"
"github.com/zclconf/go-cty/cty/function"
ctyjson "github.com/zclconf/go-cty/cty/json"
"github.com/zclconf/go-cty/cty/msgpack"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -445,25 +446,33 @@ func (p *provider6) CallFunction(_ context.Context, req *tfplugin6.CallFunction_
if len(req.Arguments) != 0 {
args = make([]cty.Value, len(req.Arguments))
for i, rawArg := range req.Arguments {
idx := int64(i)

var argTy cty.Type
if i < len(funcSchema.Parameters) {
argTy = funcSchema.Parameters[i].Type
} else {
if funcSchema.VariadicParameter == nil {
resp.Diagnostics = convert.AppendProtoDiag(
resp.Diagnostics, fmt.Errorf("too many arguments for non-variadic function"),
)
resp.Error = &tfplugin6.FunctionDiagnostic{
Summary: fmt.Sprintf("Call to function %q failed", req.Name),
Detail: "too many arguments for non-variadic function",
FunctionArgument: &idx,
}
return resp, nil
}
argTy = funcSchema.VariadicParameter.Type
}

argVal, err := decodeDynamicValue6(rawArg, argTy)
if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
resp.Error = &tfplugin6.FunctionDiagnostic{
Summary: fmt.Sprintf("Call to function %q failed", req.Name),
Detail: err.Error(),
FunctionArgument: &idx,
}
return resp, nil
}

args[i] = argVal
}
}
Expand All @@ -472,14 +481,27 @@ func (p *provider6) CallFunction(_ context.Context, req *tfplugin6.CallFunction_
FunctionName: req.Name,
Arguments: args,
})
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, callResp.Diagnostics)
if callResp.Diagnostics.HasErrors() {
if callResp.Err != nil {
resp.Error = &tfplugin6.FunctionDiagnostic{
Summary: fmt.Sprintf("Call to function %q failed", req.Name),
Detail: callResp.Err.Error(),
}

if argErr, ok := callResp.Err.(function.ArgError); ok {
idx := int64(argErr.Index)
resp.Error.FunctionArgument = &idx
}

return resp, nil
}

resp.Result, err = encodeDynamicValue6(callResp.Result, funcSchema.ReturnType)
if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
resp.Error = &tfplugin6.FunctionDiagnostic{
Summary: fmt.Sprintf("Call to function %q failed", req.Name),
Detail: err.Error(),
}

return resp, nil
}

Expand Down
30 changes: 21 additions & 9 deletions internal/plugin/grpc_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/zclconf/go-cty/cty"

plugin "github.com/hashicorp/go-plugin"
"github.com/zclconf/go-cty/cty/function"
ctyjson "github.com/zclconf/go-cty/cty/json"
"github.com/zclconf/go-cty/cty/msgpack"
"google.golang.org/grpc"
Expand Down Expand Up @@ -741,7 +742,7 @@ func (p *GRPCProvider) CallFunction(r providers.CallFunctionRequest) (resp provi

schema := p.GetProviderSchema()
if schema.Diagnostics.HasErrors() {
resp.Diagnostics = schema.Diagnostics
resp.Err = schema.Diagnostics.Err()
return resp
}

Expand All @@ -755,15 +756,15 @@ func (p *GRPCProvider) CallFunction(r providers.CallFunctionRequest) (resp provi
// Should only get here if the caller has a bug, because we should
// have detected earlier any attempt to call a function that the
// provider didn't declare.
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("provider has no function named %q", r.FunctionName))
resp.Err = fmt.Errorf("provider has no function named %q", r.FunctionName)
return resp
}
if len(r.Arguments) < len(funcDecl.Parameters) {
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("not enough arguments for function %q", r.FunctionName))
resp.Err = fmt.Errorf("not enough arguments for function %q", r.FunctionName)
return resp
}
if funcDecl.VariadicParameter == nil && len(r.Arguments) > len(funcDecl.Parameters) {
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("too many arguments for function %q", r.FunctionName))
resp.Err = fmt.Errorf("too many arguments for function %q", r.FunctionName)
return resp
}
args := make([]*proto.DynamicValue, len(r.Arguments))
Expand All @@ -777,7 +778,7 @@ func (p *GRPCProvider) CallFunction(r providers.CallFunctionRequest) (resp provi

argValRaw, err := msgpack.Marshal(argVal, paramDecl.Type)
if err != nil {
resp.Diagnostics = resp.Diagnostics.Append(err)
resp.Err = err
return resp
}
args[i] = &proto.DynamicValue{
Expand All @@ -790,17 +791,28 @@ func (p *GRPCProvider) CallFunction(r providers.CallFunctionRequest) (resp provi
Arguments: args,
})
if err != nil {
resp.Diagnostics = resp.Diagnostics.Append(grpcErr(err))
// functions can only support simple errors, but use our grpcError
// diagnostic function to format common problems is a more
// user-friendly manner.
resp.Err = grpcErr(err).Err()
return resp
}
resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics))
if resp.Diagnostics.HasErrors() {

if protoResp.Error != nil {
resp.Err = fmt.Errorf("%s:\n%s", protoResp.Error.Summary, protoResp.Error.Detail)

// If this is a problem with a specific argument, we can wrap the error
// in a function.ArgError
if protoResp.Error.FunctionArgument != nil {
resp.Err = function.NewArgError(int(*protoResp.Error.FunctionArgument), resp.Err)
}

return resp
}

resultVal, err := decodeDynamicValue(protoResp.Result, funcDecl.ReturnType)
if err != nil {
resp.Diagnostics = resp.Diagnostics.Append(err)
resp.Err = err
return resp
}

Expand Down
30 changes: 21 additions & 9 deletions internal/plugin6/grpc_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/zclconf/go-cty/cty"

plugin "github.com/hashicorp/go-plugin"
"github.com/zclconf/go-cty/cty/function"
ctyjson "github.com/zclconf/go-cty/cty/json"
"github.com/zclconf/go-cty/cty/msgpack"
"google.golang.org/grpc"
Expand Down Expand Up @@ -730,7 +731,7 @@ func (p *GRPCProvider) CallFunction(r providers.CallFunctionRequest) (resp provi

schema := p.GetProviderSchema()
if schema.Diagnostics.HasErrors() {
resp.Diagnostics = schema.Diagnostics
resp.Err = schema.Diagnostics.Err()
return resp
}

Expand All @@ -744,15 +745,15 @@ func (p *GRPCProvider) CallFunction(r providers.CallFunctionRequest) (resp provi
// Should only get here if the caller has a bug, because we should
// have detected earlier any attempt to call a function that the
// provider didn't declare.
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("provider has no function named %q", r.FunctionName))
resp.Err = fmt.Errorf("provider has no function named %q", r.FunctionName)
return resp
}
if len(r.Arguments) < len(funcDecl.Parameters) {
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("not enough arguments for function %q", r.FunctionName))
resp.Err = fmt.Errorf("not enough arguments for function %q", r.FunctionName)
return resp
}
if funcDecl.VariadicParameter == nil && len(r.Arguments) > len(funcDecl.Parameters) {
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("too many arguments for function %q", r.FunctionName))
resp.Err = fmt.Errorf("too many arguments for function %q", r.FunctionName)
return resp
}
args := make([]*proto6.DynamicValue, len(r.Arguments))
Expand All @@ -766,7 +767,7 @@ func (p *GRPCProvider) CallFunction(r providers.CallFunctionRequest) (resp provi

argValRaw, err := msgpack.Marshal(argVal, paramDecl.Type)
if err != nil {
resp.Diagnostics = resp.Diagnostics.Append(err)
resp.Err = err
return resp
}
args[i] = &proto6.DynamicValue{
Expand All @@ -779,17 +780,28 @@ func (p *GRPCProvider) CallFunction(r providers.CallFunctionRequest) (resp provi
Arguments: args,
})
if err != nil {
resp.Diagnostics = resp.Diagnostics.Append(grpcErr(err))
// functions can only support simple errors, but use our grpcError
// diagnostic function to format common problems is a more
// user-friendly manner.
resp.Err = grpcErr(err).Err()
return resp
}
resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics))
if resp.Diagnostics.HasErrors() {

if protoResp.Error != nil {
resp.Err = fmt.Errorf("%s:\n%s", protoResp.Error.Summary, protoResp.Error.Detail)

// If this is a problem with a specific argument, we can wrap the error
// in a function.ArgError
if protoResp.Error.FunctionArgument != nil {
resp.Err = function.NewArgError(int(*protoResp.Error.FunctionArgument), resp.Err)
}

return resp
}

resultVal, err := decodeDynamicValue(protoResp.Result, funcDecl.ReturnType)
if err != nil {
resp.Diagnostics = resp.Diagnostics.Append(err)
resp.Err = err
return resp
}

Expand Down
2 changes: 1 addition & 1 deletion internal/provider-simple-v6/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func (s simple) ReadDataSource(req providers.ReadDataSourceRequest) (resp provid

func (s simple) CallFunction(req providers.CallFunctionRequest) (resp providers.CallFunctionResponse) {
if req.FunctionName != "noop" {
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("CallFunction for undefined function %q", req.FunctionName))
resp.Err = fmt.Errorf("CallFunction for undefined function %q", req.FunctionName)
return resp
}

Expand Down
7 changes: 2 additions & 5 deletions internal/providers/functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,8 @@ func (d FunctionDecl) BuildFunction(providerAddr addrs.Provider, name string, re
FunctionName: name,
Arguments: args,
})
// NOTE: We don't actually have any way to surface warnings
// from the function here, because functions just return normal
// Go errors rather than diagnostics.
if resp.Diagnostics.HasErrors() {
return cty.UnknownVal(retType), resp.Diagnostics.Err()
if resp.Err != nil {
return cty.UnknownVal(retType), resp.Err
}

if resp.Result == cty.NilVal {
Expand Down
6 changes: 4 additions & 2 deletions internal/providers/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,8 @@ type CallFunctionResponse struct {
// so can be left as cty.NilVal to represent the absense of a value.
Result cty.Value

// Diagnostics contains any warnings or errors from the function call.
Diagnostics tfdiags.Diagnostics
// Err is the error value from the function call. This may be an instance
// of function.ArgError from the go-cty package to specify a problem with a
// specific argument.
Err error
}
Loading

0 comments on commit c34c53e

Please sign in to comment.