From 54a51bd3e32641a824ea1f7b96d58bdcc0822196 Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Mon, 31 Mar 2025 12:00:40 +0200 Subject: [PATCH] Data source plugin: Improve error message when plugin has connection issues (#102625) * Improve data source error message when stackID * Update comment * Revert "Update comment" This reverts commit 48922bc55259803f1717e91afc9f749a60d61184. * Revert "Improve data source error message when stackID" This reverts commit 4bf0a2f7b712e77b9de4655716695a7ee75c183b. * Make public messagic configurable based on context * Update, simplify * Update * Update getting stack * Update pkg/plugins/errors.go Co-authored-by: Andres Martinez Gotor * Refactor test to test for when context has stack value * Remove duplicated test * Fix error checking logic --------- Co-authored-by: Andres Martinez Gotor --- .../backendplugin/grpcplugin/client_v2.go | 2 +- pkg/plugins/errors.go | 22 +++++-- pkg/plugins/errors_test.go | 58 +++++++++++++++++++ pkg/plugins/manager/client/client.go | 7 ++- pkg/plugins/manager/client/client_test.go | 4 +- 5 files changed, 83 insertions(+), 10 deletions(-) create mode 100644 pkg/plugins/errors_test.go diff --git a/pkg/plugins/backendplugin/grpcplugin/client_v2.go b/pkg/plugins/backendplugin/grpcplugin/client_v2.go index 78d7b0d6dcc..da503189ec7 100644 --- a/pkg/plugins/backendplugin/grpcplugin/client_v2.go +++ b/pkg/plugins/backendplugin/grpcplugin/client_v2.go @@ -173,7 +173,7 @@ func (c *ClientV2) QueryData(ctx context.Context, req *backend.QueryDataRequest) } if status.Code(err) == codes.Unavailable { - return nil, plugins.ErrPluginGrpcConnectionUnavailableBase.Errorf("%v", err) + return nil, plugins.ErrPluginGrpcConnectionUnavailableBaseFn(ctx).Errorf("%v", err) } if status.Code(err) == codes.ResourceExhausted { diff --git a/pkg/plugins/errors.go b/pkg/plugins/errors.go index 77e35735ee0..8a0494898bb 100644 --- a/pkg/plugins/errors.go +++ b/pkg/plugins/errors.go @@ -1,6 +1,12 @@ package plugins -import "github.com/grafana/grafana/pkg/apimachinery/errutil" +import ( + "context" + + "github.com/grafana/authlib/types" + "github.com/grafana/grafana/pkg/apimachinery/errutil" + "github.com/grafana/grafana/pkg/apimachinery/identity" +) var ( errPluginNotRegisteredBase = errutil.NotFound("plugin.notRegistered", @@ -42,9 +48,13 @@ var ( errutil.WithPublicMessage("The response is too large. Please try to reduce the time range or narrow down your query to return fewer data points."), errutil.WithDownstream()) - // ErrPluginGrpcConnectionUnavailableBase error returned when a plugin connection issue occurs. - // Exposed as a base error to wrap it with plugin connection issue errors. - ErrPluginGrpcConnectionUnavailableBase = errutil.Internal("plugin.connectionUnavailable", - errutil.WithPublicMessage("Data source became unavailable during request. Please try again."), - errutil.WithDownstream()) + ErrPluginGrpcConnectionUnavailableBaseFn = func(ctx context.Context) errutil.Base { + pubMsg := "Data source became unavailable during request. Please try again." + if requester, err := identity.GetRequester(ctx); err == nil && requester != nil { + if namespace, err := types.ParseNamespace(requester.GetNamespace()); err == nil && namespace.StackID != 0 { + pubMsg += " If the problem persists, please contact customer support." + } + } + return errutil.Internal("plugin.connectionUnavailable", errutil.WithPublicMessage(pubMsg)) + } ) diff --git a/pkg/plugins/errors_test.go b/pkg/plugins/errors_test.go new file mode 100644 index 00000000000..6543c5fa6bf --- /dev/null +++ b/pkg/plugins/errors_test.go @@ -0,0 +1,58 @@ +package plugins + +import ( + "context" + "errors" + "testing" + + "github.com/grafana/grafana/pkg/apimachinery/errutil" + "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/stretchr/testify/assert" +) + +func TestErrPluginGrpcConnectionUnavailableBase(t *testing.T) { + tests := []struct { + name string + ctx context.Context + err error + expectedPublic string + }{ + { + name: "without stack ID in context", + ctx: identity.WithRequester(context.Background(), &identity.StaticRequester{ + Namespace: "org-123", + }), + err: errors.New("connection failed"), + expectedPublic: "Data source became unavailable during request. Please try again.", + }, + { + name: "with stack ID in context", + ctx: identity.WithRequester(context.Background(), &identity.StaticRequester{ + Namespace: "stacks-123", + }), + err: errors.New("connection failed"), + expectedPublic: "Data source became unavailable during request. Please try again. If the problem persists, please contact customer support.", + }, + { + name: "without static requester in context", + ctx: context.Background(), + err: errors.New("connection failed"), + expectedPublic: "Data source became unavailable during request. Please try again.", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ErrPluginGrpcConnectionUnavailableBaseFn(tt.ctx).Errorf("%v", tt.err) + assert.Error(t, err) + + // Check if it's a Grafana error + var grafanaErr errutil.Error + assert.ErrorAs(t, err, &grafanaErr) + + // Check the public message + publicErr := grafanaErr.Public() + assert.Equal(t, tt.expectedPublic, publicErr.Message) + }) + } +} diff --git a/pkg/plugins/manager/client/client.go b/pkg/plugins/manager/client/client.go index 4f4d19aa6b0..5cc8f44c5b8 100644 --- a/pkg/plugins/manager/client/client.go +++ b/pkg/plugins/manager/client/client.go @@ -35,7 +35,6 @@ var passthroughErrors = []error{ plugins.ErrPluginUnavailable, plugins.ErrMethodNotImplemented, plugins.ErrPluginGrpcResourceExhaustedBase, - plugins.ErrPluginGrpcConnectionUnavailableBase, } type Service struct { @@ -66,6 +65,12 @@ func (s *Service) QueryData(ctx context.Context, req *backend.QueryDataRequest) } } + // If the error is a plugin grpc connection unavailable error, return it directly + // This error is created dynamically based on the context, so we need to check for it here + if errors.Is(err, plugins.ErrPluginGrpcConnectionUnavailableBaseFn(ctx)) { + return nil, err + } + if errors.Is(err, context.Canceled) { return nil, plugins.ErrPluginRequestCanceledErrorBase.Errorf("client: query data request canceled: %w", err) } diff --git a/pkg/plugins/manager/client/client_test.go b/pkg/plugins/manager/client/client_test.go index 4964a3b1631..0369365c269 100644 --- a/pkg/plugins/manager/client/client_test.go +++ b/pkg/plugins/manager/client/client_test.go @@ -50,8 +50,8 @@ func TestQueryData(t *testing.T) { shouldPassThrough: false, }, { - err: plugins.ErrPluginGrpcConnectionUnavailableBase.Errorf("unavailable"), - expectedError: plugins.ErrPluginGrpcConnectionUnavailableBase.Errorf("unavailable"), + err: plugins.ErrPluginGrpcConnectionUnavailableBaseFn(context.Background()).Errorf("unavailable"), + expectedError: plugins.ErrPluginGrpcConnectionUnavailableBaseFn(context.Background()).Errorf("unavailable"), shouldPassThrough: true, }, {