From 5f063ae94a283f48acaefeee34264fe2e342d999 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Sun, 25 Oct 2020 11:19:09 -0400 Subject: [PATCH] make grpcErr work for either plugin type Extract a better function name and make the errors generic for different plugin types. --- plugin/grpc_error.go | 74 ++++++++++++++++++++++++++++++++++++++ plugin/grpc_provider.go | 69 ----------------------------------- plugin/grpc_provisioner.go | 9 +++-- 3 files changed, 78 insertions(+), 74 deletions(-) create mode 100644 plugin/grpc_error.go diff --git a/plugin/grpc_error.go b/plugin/grpc_error.go new file mode 100644 index 000000000..4875b576c --- /dev/null +++ b/plugin/grpc_error.go @@ -0,0 +1,74 @@ +package plugin + +import ( + "fmt" + "path" + "runtime" + + "github.com/hashicorp/terraform/tfdiags" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// grpcErr extracts some known error types and formats them into better +// representations for core. This must only be called from plugin methods. +// Since we don't use RPC status errors for the plugin protocol, these do not +// contain any useful details, and we can return some text that at least +// indicates the plugin call and possible error condition. +func grpcErr(err error) (diags tfdiags.Diagnostics) { + if err == nil { + return + } + + // extract the method name from the caller. + pc, _, _, ok := runtime.Caller(1) + if !ok { + logger.Error("unknown grpc call", "error", err) + return diags.Append(err) + } + + f := runtime.FuncForPC(pc) + + // Function names will contain the full import path. Take the last + // segment, which will let users know which method was being called. + _, requestName := path.Split(f.Name()) + + // Here we can at least correlate the error in the logs to a particular binary. + logger.Error(requestName, "error", err) + + // TODO: while this expands the error codes into somewhat better messages, + // this still does not easily link the error to an actual user-recognizable + // plugin. The grpc plugin does not know its configured name, and the + // errors are in a list of diagnostics, making it hard for the caller to + // annotate the returned errors. + switch status.Code(err) { + case codes.Unavailable: + // This case is when the plugin has stopped running for some reason, + // and is usually the result of a crash. + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Plugin did not respond", + fmt.Sprintf("The plugin encountered an error, and failed to respond to the %s call. "+ + "The plugin logs may contain more details.", requestName), + )) + case codes.Canceled: + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Request cancelled", + fmt.Sprintf("The %s request was cancelled.", requestName), + )) + case codes.Unimplemented: + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Unsupported plugin method", + fmt.Sprintf("The %s method is not supported by this plugin.", requestName), + )) + default: + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Plugin error", + fmt.Sprintf("The plugin returned an unexpected error from %s: %v", requestName, err), + )) + } + return +} diff --git a/plugin/grpc_provider.go b/plugin/grpc_provider.go index a4531286a..d2a287e61 100644 --- a/plugin/grpc_provider.go +++ b/plugin/grpc_provider.go @@ -3,9 +3,6 @@ package plugin import ( "context" "errors" - "fmt" - "runtime" - "strings" "sync" "github.com/zclconf/go-cty/cty" @@ -15,12 +12,9 @@ import ( proto "github.com/hashicorp/terraform/internal/tfplugin5" "github.com/hashicorp/terraform/plugin/convert" "github.com/hashicorp/terraform/providers" - "github.com/hashicorp/terraform/tfdiags" ctyjson "github.com/zclconf/go-cty/cty/json" "github.com/zclconf/go-cty/cty/msgpack" "google.golang.org/grpc" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" ) var logger = logging.HCLogger() @@ -625,66 +619,3 @@ func decodeDynamicValue(v *proto.DynamicValue, ty cty.Type) (cty.Value, error) { } return res, err } - -// grpcErr extracts some known error types and formats them into better -// representations for core. This must only be called from plugin methods. -// Since we don't use RPC status errors for the plugin protocol, these do not -// contain any useful details, and we can return some text that at least -// indicates the plugin call and possible error condition. -func grpcErr(err error) (diags tfdiags.Diagnostics) { - if err == nil { - return - } - - // extract the method name from the caller. - pc, _, _, ok := runtime.Caller(1) - if !ok { - return diags.Append(err) - } - - f := runtime.FuncForPC(pc) - requestName := f.Name() - dot := strings.LastIndex(requestName, ".") - if dot > 0 { - requestName = requestName[dot+1:] - } - - // Here we can at least correlate the error in the logs to a particular binary. - logger.Error("GRPCProvider."+requestName, "error", err) - - // TODO: while this expands the error codes into somewhat better messages, - // this still does not easily link the error to the an actual - // user-recognizable provider. The GRPCProvider does not know its - // configured name, and the errors are in a list of diagnostics, making it - // hard for the the caller to annotate the returned errors. - switch status.Code(err) { - case codes.Unavailable: - // This case is when the provider has stopped running for some reason, - // and is usually the result of a crash. - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Provider did not respond", - fmt.Sprintf("The provider encountered an error, and failed to respond to the %s call. "+ - "The provider logs may contain more details", requestName), - )) - case codes.Canceled: - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Request cancelled", - fmt.Sprintf("The %s request was cancelled.", requestName), - )) - case codes.Unimplemented: - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Unsupported plugin method", - fmt.Sprintf("The %s method is not supported by this provider", requestName), - )) - default: - diags = diags.Append(tfdiags.Sourceless( - tfdiags.Error, - "Provider error", - fmt.Sprintf("The provider returned an unexpected error from %s: %v", requestName, err), - )) - } - return -} diff --git a/plugin/grpc_provisioner.go b/plugin/grpc_provisioner.go index 136c88d68..c57daa9af 100644 --- a/plugin/grpc_provisioner.go +++ b/plugin/grpc_provisioner.go @@ -4,7 +4,6 @@ import ( "context" "errors" "io" - "log" "sync" plugin "github.com/hashicorp/go-plugin" @@ -61,7 +60,7 @@ func (p *GRPCProvisioner) GetSchema() (resp provisioners.GetSchemaResponse) { protoResp, err := p.client.GetSchema(p.ctx, new(proto.GetProvisionerSchema_Request)) if err != nil { - resp.Diagnostics = resp.Diagnostics.Append(err) + resp.Diagnostics = resp.Diagnostics.Append(grpcErr(err)) return resp } resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics)) @@ -96,7 +95,7 @@ func (p *GRPCProvisioner) ValidateProvisionerConfig(r provisioners.ValidateProvi } protoResp, err := p.client.ValidateProvisionerConfig(p.ctx, protoReq) if err != nil { - resp.Diagnostics = resp.Diagnostics.Append(err) + resp.Diagnostics = resp.Diagnostics.Append(grpcErr(err)) return resp } resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics)) @@ -130,7 +129,7 @@ func (p *GRPCProvisioner) ProvisionResource(r provisioners.ProvisionResourceRequ outputClient, err := p.client.ProvisionResource(p.ctx, protoReq) if err != nil { - resp.Diagnostics = resp.Diagnostics.Append(err) + resp.Diagnostics = resp.Diagnostics.Append(grpcErr(err)) return resp } @@ -169,7 +168,7 @@ func (p *GRPCProvisioner) Stop() error { func (p *GRPCProvisioner) Close() error { // check this since it's not automatically inserted during plugin creation if p.PluginClient == nil { - log.Println("[DEBUG] provider has no plugin.Client") + logger.Debug("provisioner has no plugin.Client") return nil }