From bc0110f72bb3e50668dc7c0c871d7aa4a5cc9cb3 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 10 Oct 2024 14:55:16 +0200 Subject: [PATCH 1/9] Error source HTTP client middleware --- backend/data_adapter.go | 9 +- backend/error_source.go | 118 ++-------- backend/error_source_test.go | 142 +----------- backend/httpclient/error_source_middleware.go | 24 ++ backend/httpclient/http_client.go | 1 + backend/httpclient/http_client_test.go | 3 +- backend/httpclient/provider_test.go | 9 +- backend/request_status.go | 30 +-- backend/status/doc.go | 2 + backend/status/status_source.go | 213 ++++++++++++++++++ backend/status/status_source_test.go | 193 ++++++++++++++++ .../errorsource/error_source_middleware.go | 11 +- 12 files changed, 490 insertions(+), 265 deletions(-) create mode 100644 backend/httpclient/error_source_middleware.go create mode 100644 backend/status/doc.go create mode 100644 backend/status/status_source.go create mode 100644 backend/status/status_source_test.go diff --git a/backend/data_adapter.go b/backend/data_adapter.go index 827efd85d..31d69f429 100644 --- a/backend/data_adapter.go +++ b/backend/data_adapter.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" + "github.com/grafana/grafana-plugin-sdk-go/backend/status" "github.com/grafana/grafana-plugin-sdk-go/genproto/pluginv2" ) @@ -29,9 +30,9 @@ func (a *dataSDKAdapter) QueryData(ctx context.Context, req *pluginv2.QueryDataR var innerErr error resp, innerErr = a.queryDataHandler.QueryData(ctx, parsedReq) - status := RequestStatusFromQueryDataResponse(resp, innerErr) + requestStatus := RequestStatusFromQueryDataResponse(resp, innerErr) if innerErr != nil { - return status, innerErr + return requestStatus, innerErr } else if resp == nil { return RequestStatusError, errors.New("both response and error are nil, but one must be provided") } @@ -41,7 +42,7 @@ func (a *dataSDKAdapter) QueryData(ctx context.Context, req *pluginv2.QueryDataR // and if there's no plugin error var hasPluginError, hasDownstreamError bool for refID, r := range resp.Responses { - if r.Error == nil || isCancelledError(r.Error) { + if r.Error == nil || status.IsCancelledError(r.Error) { continue } @@ -81,7 +82,7 @@ func (a *dataSDKAdapter) QueryData(ctx context.Context, req *pluginv2.QueryDataR } } - return status, nil + return requestStatus, nil }) if err != nil { return nil, err diff --git a/backend/error_source.go b/backend/error_source.go index 8c157cf30..4ff4d2462 100644 --- a/backend/error_source.go +++ b/backend/error_source.go @@ -2,125 +2,62 @@ package backend import ( "context" - "errors" "fmt" - "net/http" + + "github.com/grafana/grafana-plugin-sdk-go/backend/status" ) // ErrorSource type defines the source of the error -type ErrorSource string +type ErrorSource = status.Source const ( // ErrorSourcePlugin error originates from plugin. - ErrorSourcePlugin ErrorSource = "plugin" + ErrorSourcePlugin = status.SourcePlugin // ErrorSourceDownstream error originates from downstream service. - ErrorSourceDownstream ErrorSource = "downstream" + ErrorSourceDownstream = status.SourceDownstream // DefaultErrorSource is the default [ErrorSource] that should be used when it is not explicitly set. - DefaultErrorSource ErrorSource = ErrorSourcePlugin + DefaultErrorSource = status.SourcePlugin ) -func (es ErrorSource) IsValid() bool { - return es == ErrorSourceDownstream || es == ErrorSourcePlugin +// ErrorSourceFromHTTPError returns an [ErrorSource] based on provided error. +func ErrorSourceFromHTTPError(err error) ErrorSource { + return status.SourceFromHTTPError(err) } -// ErrorSourceFromStatus returns an [ErrorSource] based on provided HTTP status code. +// ErrorSourceFromHTTPStatus returns an [ErrorSource] based on provided HTTP status code. func ErrorSourceFromHTTPStatus(statusCode int) ErrorSource { - switch statusCode { - case http.StatusMethodNotAllowed, - http.StatusNotAcceptable, - http.StatusPreconditionFailed, - http.StatusRequestEntityTooLarge, - http.StatusRequestHeaderFieldsTooLarge, - http.StatusRequestURITooLong, - http.StatusExpectationFailed, - http.StatusUpgradeRequired, - http.StatusRequestedRangeNotSatisfiable, - http.StatusNotImplemented: - return ErrorSourcePlugin - } - - return ErrorSourceDownstream + return status.SourceFromHTTPStatus(statusCode) } -type errorWithSourceImpl struct { - source ErrorSource - err error +// IsDownstreamError return true if provided error is an error with downstream source or +// a timeout error or a cancelled error. +func IsDownstreamError(err error) bool { + return status.IsDownstreamError(err) } -func IsDownstreamError(err error) bool { - e := errorWithSourceImpl{ - source: ErrorSourceDownstream, - } - if errors.Is(err, e) { - return true - } - - type errorWithSource interface { - ErrorSource() ErrorSource - } - - // nolint:errorlint - if errWithSource, ok := err.(errorWithSource); ok && errWithSource.ErrorSource() == ErrorSourceDownstream { - return true - } - - if isHTTPTimeoutError(err) || isCancelledError(err) { - return true - } - - return false +// IsDownstreamError return true if provided error is an error with downstream source or +// a HTTP timeout error or a cancelled error or a connection reset/refused error or dns not found error. +func IsDownstreamHTTPError(err error) bool { + return status.IsDownstreamHTTPError(err) } func DownstreamError(err error) error { - return errorWithSourceImpl{ - source: ErrorSourceDownstream, - err: err, - } + return status.DownstreamError(err) } func DownstreamErrorf(format string, a ...any) error { return DownstreamError(fmt.Errorf(format, a...)) } -func (e errorWithSourceImpl) ErrorSource() ErrorSource { - return e.source -} - -func (e errorWithSourceImpl) Error() string { - return fmt.Errorf("%s error: %w", e.source, e.err).Error() -} - -// Implements the interface used by [errors.Is]. -func (e errorWithSourceImpl) Is(err error) bool { - if errWithSource, ok := err.(errorWithSourceImpl); ok { - return errWithSource.ErrorSource() == e.source - } - - return false -} - -func (e errorWithSourceImpl) Unwrap() error { - return e.err -} - -type errorSourceCtxKey struct{} - -// errorSourceFromContext returns the error source stored in the context. -// If no error source is stored in the context, [DefaultErrorSource] is returned. func errorSourceFromContext(ctx context.Context) ErrorSource { - value, ok := ctx.Value(errorSourceCtxKey{}).(*ErrorSource) - if ok { - return *value - } - return DefaultErrorSource + return status.SourceFromContext(ctx) } -// initErrorSource initialize the status source for the context. +// initErrorSource initialize the error source for the context. func initErrorSource(ctx context.Context) context.Context { - s := DefaultErrorSource - return context.WithValue(ctx, errorSourceCtxKey{}, &s) + return status.InitSource(ctx) } // WithErrorSource mutates the provided context by setting the error source to @@ -128,12 +65,7 @@ func initErrorSource(ctx context.Context) context.Context { // will not be mutated and an error returned. This means that [initErrorSource] // has to be called before this function. func WithErrorSource(ctx context.Context, s ErrorSource) error { - v, ok := ctx.Value(errorSourceCtxKey{}).(*ErrorSource) - if !ok { - return errors.New("the provided context does not have a status source") - } - *v = s - return nil + return status.WithSource(ctx, s) } // WithDownstreamErrorSource mutates the provided context by setting the error source to @@ -141,5 +73,5 @@ func WithErrorSource(ctx context.Context, s ErrorSource) error { // will not be mutated and an error returned. This means that [initErrorSource] has to be // called before this function. func WithDownstreamErrorSource(ctx context.Context) error { - return WithErrorSource(ctx, ErrorSourceDownstream) + return status.WithDownstreamSource(ctx) } diff --git a/backend/error_source_test.go b/backend/error_source_test.go index 94ebe1354..d10926ee9 100644 --- a/backend/error_source_test.go +++ b/backend/error_source_test.go @@ -1,142 +1,18 @@ -package backend +package backend_test import ( - "context" - "errors" - "fmt" - "net" - "os" "testing" - "github.com/stretchr/testify/assert" + "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/stretchr/testify/require" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" ) func TestErrorSource(t *testing.T) { - var es ErrorSource - require.False(t, es.IsValid()) - require.True(t, ErrorSourceDownstream.IsValid()) - require.True(t, ErrorSourcePlugin.IsValid()) -} - -func TestIsDownstreamError(t *testing.T) { - tcs := []struct { - name string - err error - expected bool - }{ - { - name: "nil", - err: nil, - expected: false, - }, - { - name: "downstream error", - err: DownstreamError(nil), - expected: true, - }, - { - name: "timeout network error", - err: newFakeNetworkError(true, false), - expected: true, - }, - { - name: "wrapped timeout network error", - err: fmt.Errorf("oh no. err %w", newFakeNetworkError(true, false)), - expected: true, - }, - { - name: "temporary timeout network error", - err: newFakeNetworkError(true, true), - expected: true, - }, - { - name: "non-timeout network error", - err: newFakeNetworkError(false, false), - expected: false, - }, - { - name: "os.ErrDeadlineExceeded", - err: os.ErrDeadlineExceeded, - expected: true, - }, - { - name: "os.ErrDeadlineExceeded", - err: fmt.Errorf("error: %w", os.ErrDeadlineExceeded), - expected: true, - }, - { - name: "wrapped os.ErrDeadlineExceeded", - err: errors.Join(fmt.Errorf("oh no"), os.ErrDeadlineExceeded), - expected: true, - }, - { - name: "other error", - err: fmt.Errorf("other error"), - expected: false, - }, - { - name: "context.Canceled", - err: context.Canceled, - expected: true, - }, - { - name: "wrapped context.Canceled", - err: fmt.Errorf("error: %w", context.Canceled), - expected: true, - }, - { - name: "joined context.Canceled", - err: errors.Join(fmt.Errorf("oh no"), context.Canceled), - expected: true, - }, - { - name: "gRPC canceled error", - err: status.Error(codes.Canceled, "canceled"), - expected: true, - }, - { - name: "wrapped gRPC canceled error", - err: fmt.Errorf("error: %w", status.Error(codes.Canceled, "canceled")), - expected: true, - }, - { - name: "joined gRPC canceled error", - err: errors.Join(fmt.Errorf("oh no"), status.Error(codes.Canceled, "canceled")), - expected: true, - }, - } - for _, tc := range tcs { - t.Run(tc.name, func(t *testing.T) { - assert.Equalf(t, tc.expected, IsDownstreamError(tc.err), "IsDownstreamError(%v)", tc.err) - }) - } -} - -var _ net.Error = &fakeNetworkError{} - -type fakeNetworkError struct { - timeout bool - temporary bool -} - -func newFakeNetworkError(timeout, temporary bool) *fakeNetworkError { - return &fakeNetworkError{ - timeout: timeout, - temporary: temporary, - } -} - -func (d *fakeNetworkError) Error() string { - return "dummy timeout error" -} - -func (d *fakeNetworkError) Timeout() bool { - return d.timeout -} - -func (d *fakeNetworkError) Temporary() bool { - return d.temporary + var s backend.ErrorSource + require.False(t, s.IsValid()) + require.Equal(t, "plugin", s.String()) + require.True(t, backend.ErrorSourceDownstream.IsValid()) + require.Equal(t, "downstream", backend.ErrorSourceDownstream.String()) + require.True(t, backend.ErrorSourcePlugin.IsValid()) + require.Equal(t, "plugin", backend.ErrorSourcePlugin.String()) } diff --git a/backend/httpclient/error_source_middleware.go b/backend/httpclient/error_source_middleware.go new file mode 100644 index 000000000..17594443a --- /dev/null +++ b/backend/httpclient/error_source_middleware.go @@ -0,0 +1,24 @@ +package httpclient + +import ( + "net/http" + + "github.com/grafana/grafana-plugin-sdk-go/backend/status" +) + +// ErrorSourceMiddlewareName is the middleware name used by ErrorSourceMiddleware. +const ErrorSourceMiddlewareName = "ErrorSource" + +// ErrorSourceMiddleware inspect the response error and wraps it in a [status.DownstreamError] if [status.IsDownstreamHTTPError] returns true. +func ErrorSourceMiddleware() Middleware { + return NamedMiddlewareFunc(ErrorSourceMiddlewareName, func(_ Options, next http.RoundTripper) http.RoundTripper { + return RoundTripperFunc(func(req *http.Request) (*http.Response, error) { + res, err := next.RoundTrip(req) + if err != nil && status.IsDownstreamHTTPError(err) { + return res, status.DownstreamError(err) + } + + return res, err + }) + }) +} diff --git a/backend/httpclient/http_client.go b/backend/httpclient/http_client.go index 40a4d39d4..0d88f72e8 100644 --- a/backend/httpclient/http_client.go +++ b/backend/httpclient/http_client.go @@ -210,6 +210,7 @@ func DefaultMiddlewares() []Middleware { BasicAuthenticationMiddleware(), CustomHeadersMiddleware(), ContextualMiddleware(), + ErrorSourceMiddleware(), } } diff --git a/backend/httpclient/http_client_test.go b/backend/httpclient/http_client_test.go index 570fe0833..e8875d180 100644 --- a/backend/httpclient/http_client_test.go +++ b/backend/httpclient/http_client_test.go @@ -55,11 +55,12 @@ func TestNewClient(t *testing.T) { require.NoError(t, err) require.NotNil(t, client) - require.Len(t, usedMiddlewares, 4) + require.Len(t, usedMiddlewares, 5) require.Equal(t, TracingMiddlewareName, usedMiddlewares[0].(MiddlewareName).MiddlewareName()) require.Equal(t, BasicAuthenticationMiddlewareName, usedMiddlewares[1].(MiddlewareName).MiddlewareName()) require.Equal(t, CustomHeadersMiddlewareName, usedMiddlewares[2].(MiddlewareName).MiddlewareName()) require.Equal(t, ContextualMiddlewareName, usedMiddlewares[3].(MiddlewareName).MiddlewareName()) + require.Equal(t, ErrorSourceMiddlewareName, usedMiddlewares[4].(MiddlewareName).MiddlewareName()) }) t.Run("New() with opts middleware should return expected http.Client", func(t *testing.T) { diff --git a/backend/httpclient/provider_test.go b/backend/httpclient/provider_test.go index b5331321a..deb7525ce 100644 --- a/backend/httpclient/provider_test.go +++ b/backend/httpclient/provider_test.go @@ -24,11 +24,12 @@ func TestProvider(t *testing.T) { client, err := ctx.provider.New() require.NoError(t, err) require.NotNil(t, client) - require.Len(t, ctx.usedMiddlewares, 4) + require.Len(t, ctx.usedMiddlewares, 5) require.Equal(t, TracingMiddlewareName, ctx.usedMiddlewares[0].(MiddlewareName).MiddlewareName()) require.Equal(t, BasicAuthenticationMiddlewareName, ctx.usedMiddlewares[1].(MiddlewareName).MiddlewareName()) require.Equal(t, CustomHeadersMiddlewareName, ctx.usedMiddlewares[2].(MiddlewareName).MiddlewareName()) require.Equal(t, ContextualMiddlewareName, ctx.usedMiddlewares[3].(MiddlewareName).MiddlewareName()) + require.Equal(t, ErrorSourceMiddlewareName, ctx.usedMiddlewares[4].(MiddlewareName).MiddlewareName()) }) t.Run("Transport should use default middlewares", func(t *testing.T) { @@ -36,11 +37,12 @@ func TestProvider(t *testing.T) { transport, err := ctx.provider.GetTransport() require.NoError(t, err) require.NotNil(t, transport) - require.Len(t, ctx.usedMiddlewares, 4) + require.Len(t, ctx.usedMiddlewares, 5) require.Equal(t, TracingMiddlewareName, ctx.usedMiddlewares[0].(MiddlewareName).MiddlewareName()) require.Equal(t, BasicAuthenticationMiddlewareName, ctx.usedMiddlewares[1].(MiddlewareName).MiddlewareName()) require.Equal(t, CustomHeadersMiddlewareName, ctx.usedMiddlewares[2].(MiddlewareName).MiddlewareName()) require.Equal(t, ContextualMiddlewareName, ctx.usedMiddlewares[3].(MiddlewareName).MiddlewareName()) + require.Equal(t, ErrorSourceMiddlewareName, ctx.usedMiddlewares[4].(MiddlewareName).MiddlewareName()) }) t.Run("New() with options and no middleware should return expected http client and transport", func(t *testing.T) { @@ -81,7 +83,7 @@ func TestProvider(t *testing.T) { require.Equal(t, DefaultTimeoutOptions.Timeout, client.Timeout) t.Run("Should use configured middlewares and implement MiddlewareName", func(t *testing.T) { - require.Len(t, pCtx.usedMiddlewares, 7) + require.Len(t, pCtx.usedMiddlewares, 8) require.Equal(t, "mw1", pCtx.usedMiddlewares[0].(MiddlewareName).MiddlewareName()) require.Equal(t, "mw2", pCtx.usedMiddlewares[1].(MiddlewareName).MiddlewareName()) require.Equal(t, "mw3", pCtx.usedMiddlewares[2].(MiddlewareName).MiddlewareName()) @@ -89,6 +91,7 @@ func TestProvider(t *testing.T) { require.Equal(t, BasicAuthenticationMiddlewareName, pCtx.usedMiddlewares[4].(MiddlewareName).MiddlewareName()) require.Equal(t, CustomHeadersMiddlewareName, pCtx.usedMiddlewares[5].(MiddlewareName).MiddlewareName()) require.Equal(t, ContextualMiddlewareName, pCtx.usedMiddlewares[6].(MiddlewareName).MiddlewareName()) + require.Equal(t, ErrorSourceMiddlewareName, pCtx.usedMiddlewares[7].(MiddlewareName).MiddlewareName()) }) t.Run("When roundtrip should call expected middlewares", func(t *testing.T) { diff --git a/backend/request_status.go b/backend/request_status.go index 36280295e..e2e490633 100644 --- a/backend/request_status.go +++ b/backend/request_status.go @@ -2,14 +2,9 @@ package backend import ( "context" - "errors" - "net" - "os" "strings" - grpccodes "google.golang.org/grpc/codes" - grpcstatus "google.golang.org/grpc/status" - + "github.com/grafana/grafana-plugin-sdk-go/backend/status" "github.com/grafana/grafana-plugin-sdk-go/genproto/pluginv2" ) @@ -31,15 +26,15 @@ func (status RequestStatus) String() string { } func RequestStatusFromError(err error) RequestStatus { - status := RequestStatusOK + requestStatus := RequestStatusOK if err != nil { - status = RequestStatusError - if isCancelledError(err) { - status = RequestStatusCancelled + requestStatus = RequestStatusError + if status.IsCancelledError(err) { + requestStatus = RequestStatusCancelled } } - return status + return requestStatus } func RequestStatusFromErrorString(errString string) RequestStatus { @@ -103,16 +98,3 @@ func RequestStatusFromProtoQueryDataResponse(res *pluginv2.QueryDataResponse, er return status } - -func isCancelledError(err error) bool { - return errors.Is(err, context.Canceled) || grpcstatus.Code(err) == grpccodes.Canceled -} - -func isHTTPTimeoutError(err error) bool { - var netErr net.Error - if errors.As(err, &netErr) && netErr.Timeout() { - return true - } - - return errors.Is(err, os.ErrDeadlineExceeded) // replacement for os.IsTimeout(err) -} diff --git a/backend/status/doc.go b/backend/status/doc.go new file mode 100644 index 000000000..a148e000a --- /dev/null +++ b/backend/status/doc.go @@ -0,0 +1,2 @@ +// Package status provides utilities for status and errors. +package status diff --git a/backend/status/status_source.go b/backend/status/status_source.go new file mode 100644 index 000000000..0dedf9342 --- /dev/null +++ b/backend/status/status_source.go @@ -0,0 +1,213 @@ +package status + +import ( + "context" + "errors" + "fmt" + "net" + "net/http" + "os" + "syscall" + + grpccodes "google.golang.org/grpc/codes" + grpcstatus "google.golang.org/grpc/status" +) + +// Source type defines the status source. +type Source string + +const ( + // SourcePlugin status originates from plugin. + SourcePlugin Source = "plugin" + + // SourceDownstream status originates from downstream service. + SourceDownstream Source = "downstream" + + // DefaultSource is the default [Source] that should be used when it is not explicitly set. + DefaultSource Source = SourcePlugin +) + +// IsValid return true if es is [SourceDownstream] or [SourcePlugin]. +func (s Source) IsValid() bool { + return s == SourceDownstream || s == SourcePlugin +} + +// String returns the string representation of s. If s is not valid, [DefaultSource] is returned. +func (s Source) String() string { + if !s.IsValid() { + return string(DefaultSource) + } + + return string(s) +} + +// SourceFromHTTPError returns a [Source] based on provided error. +func SourceFromHTTPError(err error) Source { + if IsDownstreamHTTPError(err) { + return SourceDownstream + } + return SourcePlugin +} + +// ErrorSourceFromStatus returns a [Source] based on provided HTTP status code. +func SourceFromHTTPStatus(statusCode int) Source { + switch statusCode { + case http.StatusMethodNotAllowed, + http.StatusNotAcceptable, + http.StatusPreconditionFailed, + http.StatusRequestEntityTooLarge, + http.StatusRequestHeaderFieldsTooLarge, + http.StatusRequestURITooLong, + http.StatusExpectationFailed, + http.StatusUpgradeRequired, + http.StatusRequestedRangeNotSatisfiable, + http.StatusNotImplemented: + return SourcePlugin + } + + return SourceDownstream +} + +type errorWithSourceImpl struct { + source Source + err error +} + +// DownstreamError creates a new error with status [SourceDownstream]. +func DownstreamError(err error) error { + return errorWithSourceImpl{ + source: SourceDownstream, + err: err, + } +} + +// DownstreamError creates a new error with status [SourceDownstream] and formats +// according to a format specifier and returns the string as a value that satisfies error. +func DownstreamErrorf(format string, a ...any) error { + return DownstreamError(fmt.Errorf(format, a...)) +} + +func (e errorWithSourceImpl) ErrorSource() Source { + return e.source +} + +func (e errorWithSourceImpl) Error() string { + return fmt.Errorf("%s error: %w", e.source, e.err).Error() +} + +// Implements the interface used by [errors.Is]. +func (e errorWithSourceImpl) Is(err error) bool { + if errWithSource, ok := err.(errorWithSourceImpl); ok { + return errWithSource.ErrorSource() == e.source + } + + return false +} + +func (e errorWithSourceImpl) Unwrap() error { + return e.err +} + +// IsDownstreamError return true if provided error is an error with downstream source or +// a timeout error or a cancelled error. +func IsDownstreamError(err error) bool { + e := errorWithSourceImpl{ + source: SourceDownstream, + } + if errors.Is(err, e) { + return true + } + + type errorWithSource interface { + ErrorSource() Source + } + + // nolint:errorlint + if errWithSource, ok := err.(errorWithSource); ok && errWithSource.ErrorSource() == SourceDownstream { + return true + } + + return isHTTPTimeoutError(err) || IsCancelledError(err) +} + +// IsDownstreamHTTPError return true if provided error is an error with downstream source or +// a HTTP timeout error or a cancelled error or a connection reset/refused error or dns not found error. +func IsDownstreamHTTPError(err error) bool { + return IsDownstreamError(err) || + isConnectionResetOrRefusedError(err) || + isDNSNotFoundError(err) +} + +// InCancelledError returns true if err is context.Canceled or is gRPC status Canceled. +func IsCancelledError(err error) bool { + return errors.Is(err, context.Canceled) || grpcstatus.Code(err) == grpccodes.Canceled +} + +func isHTTPTimeoutError(err error) bool { + var netErr net.Error + if errors.As(err, &netErr) && netErr.Timeout() { + return true + } + + return errors.Is(err, os.ErrDeadlineExceeded) // replacement for os.IsTimeout(err) +} + +func isConnectionResetOrRefusedError(err error) bool { + var netErr *net.OpError + if errors.As(err, &netErr) { + var sysErr *os.SyscallError + if errors.As(netErr.Err, &sysErr) { + return errors.Is(sysErr.Err, syscall.ECONNRESET) || errors.Is(sysErr.Err, syscall.ECONNREFUSED) + } + } + + return false +} + +func isDNSNotFoundError(err error) bool { + var dnsError *net.DNSError + if errors.As(err, &dnsError) && dnsError.IsNotFound { + return true + } + + return false +} + +type sourceCtxKey struct{} + +// SourceFromContext returns the source stored in the context. +// If no source is stored in the context, [DefaultSource] is returned. +func SourceFromContext(ctx context.Context) Source { + value, ok := ctx.Value(sourceCtxKey{}).(*Source) + if ok { + return *value + } + return DefaultSource +} + +// InitSource initialize the source for the context. +func InitSource(ctx context.Context) context.Context { + s := DefaultSource + return context.WithValue(ctx, sourceCtxKey{}, &s) +} + +// WithSource mutates the provided context by setting the source to +// s. If the provided context does not have a source, the context +// will not be mutated and an error returned. This means that [InitSource] +// has to be called before this function. +func WithSource(ctx context.Context, s Source) error { + v, ok := ctx.Value(sourceCtxKey{}).(*Source) + if !ok { + return errors.New("the provided context does not have a status source") + } + *v = s + return nil +} + +// WithDownstreamSource mutates the provided context by setting the source to +// [SourceDownstream]. If the provided context does not have a source, the context +// will not be mutated and an error returned. This means that [InitSource] has to be +// called before this function. +func WithDownstreamSource(ctx context.Context) error { + return WithSource(ctx, SourceDownstream) +} diff --git a/backend/status/status_source_test.go b/backend/status/status_source_test.go new file mode 100644 index 000000000..91ef362a5 --- /dev/null +++ b/backend/status/status_source_test.go @@ -0,0 +1,193 @@ +package status + +import ( + "context" + "errors" + "fmt" + "net" + "os" + "syscall" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +func TestSource(t *testing.T) { + var s Source + require.False(t, s.IsValid()) + require.Equal(t, "plugin", s.String()) + require.True(t, SourceDownstream.IsValid()) + require.Equal(t, "downstream", SourceDownstream.String()) + require.True(t, SourcePlugin.IsValid()) + require.Equal(t, "plugin", SourcePlugin.String()) +} + +func TestIsDownstreamError(t *testing.T) { + tcs := []struct { + name string + err error + expected bool + }{ + { + name: "nil", + err: nil, + expected: false, + }, + { + name: "downstream error", + err: DownstreamError(nil), + expected: true, + }, + { + name: "timeout network error", + err: newFakeNetworkError(true, false), + expected: true, + }, + { + name: "temporary timeout network error", + err: newFakeNetworkError(true, true), + expected: true, + }, + { + name: "non-timeout network error", + err: newFakeNetworkError(false, false), + expected: false, + }, + { + name: "os.ErrDeadlineExceeded", + err: os.ErrDeadlineExceeded, + expected: true, + }, + { + name: "other error", + err: fmt.Errorf("other error"), + expected: false, + }, + { + name: "context.Canceled", + err: context.Canceled, + expected: true, + }, + { + name: "gRPC canceled error", + err: status.Error(codes.Canceled, "canceled"), + expected: true, + }, + } + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + wrappedErr := fmt.Errorf("error: %w", tc.err) + joinedErr := errors.Join(errors.New("oh no"), tc.err) + assert.Equalf(t, tc.expected, IsDownstreamError(tc.err), "IsDownstreamHTTPError(%v)", tc.err) + assert.Equalf(t, tc.expected, IsDownstreamError(wrappedErr), "wrapped IsDownstreamHTTPError(%v)", wrappedErr) + assert.Equalf(t, tc.expected, IsDownstreamError(joinedErr), "joined IsDownstreamHTTPError(%v)", joinedErr) + }) + } +} + +func TestIsDownstreamHTTPError(t *testing.T) { + tcs := []struct { + name string + err error + expected bool + }{ + { + name: "nil", + err: nil, + expected: false, + }, + { + name: "downstream error", + err: DownstreamError(nil), + expected: true, + }, + { + name: "timeout network error", + err: newFakeNetworkError(true, false), + expected: true, + }, + { + name: "temporary timeout network error", + err: newFakeNetworkError(true, true), + expected: true, + }, + { + name: "non-timeout network error", + err: newFakeNetworkError(false, false), + expected: false, + }, + { + name: "os.ErrDeadlineExceeded", + err: os.ErrDeadlineExceeded, + expected: true, + }, + { + name: "other error", + err: fmt.Errorf("other error"), + expected: false, + }, + { + name: "context.Canceled", + err: context.Canceled, + expected: true, + }, + { + name: "gRPC canceled error", + err: status.Error(codes.Canceled, "canceled"), + expected: true, + }, + { + name: "connection reset error", + err: &net.OpError{Err: &os.SyscallError{Err: syscall.ECONNREFUSED}}, + expected: true, + }, + { + name: "connection refused error", + err: &net.OpError{Err: &os.SyscallError{Err: syscall.ECONNREFUSED}}, + expected: true, + }, + { + name: "DNS not found error", + err: &net.DNSError{IsNotFound: true}, + expected: true, + }, + } + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + wrappedErr := fmt.Errorf("error: %w", tc.err) + joinedErr := errors.Join(errors.New("oh no"), tc.err) + assert.Equalf(t, tc.expected, IsDownstreamHTTPError(tc.err), "IsDownstreamHTTPError(%v)", tc.err) + assert.Equalf(t, tc.expected, IsDownstreamHTTPError(wrappedErr), "wrapped IsDownstreamHTTPError(%v)", wrappedErr) + assert.Equalf(t, tc.expected, IsDownstreamHTTPError(joinedErr), "joined IsDownstreamHTTPError(%v)", joinedErr) + }) + } +} + +var _ net.Error = &fakeNetworkError{} + +type fakeNetworkError struct { + timeout bool + temporary bool +} + +func newFakeNetworkError(timeout, temporary bool) *fakeNetworkError { + return &fakeNetworkError{ + timeout: timeout, + temporary: temporary, + } +} + +func (d *fakeNetworkError) Error() string { + return "dummy timeout error" +} + +func (d *fakeNetworkError) Timeout() bool { + return d.timeout +} + +func (d *fakeNetworkError) Temporary() bool { + return d.temporary +} diff --git a/experimental/errorsource/error_source_middleware.go b/experimental/errorsource/error_source_middleware.go index 95ff79b10..4245ffeeb 100644 --- a/experimental/errorsource/error_source_middleware.go +++ b/experimental/errorsource/error_source_middleware.go @@ -2,12 +2,11 @@ package errorsource import ( "errors" - "net" "net/http" - "syscall" "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" + "github.com/grafana/grafana-plugin-sdk-go/backend/status" ) // Middleware captures error source metric @@ -26,13 +25,11 @@ func RoundTripper(_ httpclient.Options, next http.RoundTripper) http.RoundTrippe } return res, Error{source: errorSource, err: err} } - if errors.Is(err, syscall.ECONNREFUSED) { - return res, Error{source: backend.ErrorSourceDownstream, err: err} - } - var dnsError *net.DNSError - if errors.As(err, &dnsError) && dnsError.IsNotFound { + + if status.IsDownstreamHTTPError(err) { return res, Error{source: backend.ErrorSourceDownstream, err: err} } + return res, err }) } From 34b775cae3c650e93243f2fec84e7c0c8f43b2c9 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 10 Oct 2024 18:27:35 +0200 Subject: [PATCH 2/9] fixes and tests for experimental error source --- backend/status/status_source.go | 25 ++++++++++-- backend/status/status_source_test.go | 59 +++++++++++++++++++--------- 2 files changed, 63 insertions(+), 21 deletions(-) diff --git a/backend/status/status_source.go b/backend/status/status_source.go index 0dedf9342..d9f42c491 100644 --- a/backend/status/status_source.go +++ b/backend/status/status_source.go @@ -120,11 +120,30 @@ func IsDownstreamError(err error) bool { type errorWithSource interface { ErrorSource() Source + Error() string } - // nolint:errorlint - if errWithSource, ok := err.(errorWithSource); ok && errWithSource.ErrorSource() == SourceDownstream { - return true + errCopy := err + + for { + // nolint:errorlint + if errWithSource, ok := errCopy.(errorWithSource); ok && errWithSource.ErrorSource() == SourceDownstream { + return true + } + + if uw, ok := errCopy.(interface{ Unwrap() []error }); ok { + errs := uw.Unwrap() + for _, joinErr := range errs { + // nolint:errorlint + if errWithSource, ok := joinErr.(errorWithSource); ok && errWithSource.ErrorSource() == SourceDownstream { + return true + } + } + } + + if errCopy = errors.Unwrap(errCopy); errCopy == nil { + break + } } return isHTTPTimeoutError(err) || IsCancelledError(err) diff --git a/backend/status/status_source_test.go b/backend/status/status_source_test.go index 91ef362a5..3eb147760 100644 --- a/backend/status/status_source_test.go +++ b/backend/status/status_source_test.go @@ -1,4 +1,4 @@ -package status +package status_test import ( "context" @@ -9,20 +9,23 @@ import ( "syscall" "testing" + "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana-plugin-sdk-go/backend/status" + "github.com/grafana/grafana-plugin-sdk-go/experimental/errorsource" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" + grpccodes "google.golang.org/grpc/codes" + grpcstatus "google.golang.org/grpc/status" ) func TestSource(t *testing.T) { - var s Source + var s status.Source require.False(t, s.IsValid()) require.Equal(t, "plugin", s.String()) - require.True(t, SourceDownstream.IsValid()) - require.Equal(t, "downstream", SourceDownstream.String()) - require.True(t, SourcePlugin.IsValid()) - require.Equal(t, "plugin", SourcePlugin.String()) + require.True(t, status.SourceDownstream.IsValid()) + require.Equal(t, "downstream", status.SourceDownstream.String()) + require.True(t, status.SourcePlugin.IsValid()) + require.Equal(t, "plugin", status.SourcePlugin.String()) } func TestIsDownstreamError(t *testing.T) { @@ -38,7 +41,7 @@ func TestIsDownstreamError(t *testing.T) { }, { name: "downstream error", - err: DownstreamError(nil), + err: status.DownstreamError(nil), expected: true, }, { @@ -73,17 +76,27 @@ func TestIsDownstreamError(t *testing.T) { }, { name: "gRPC canceled error", - err: status.Error(codes.Canceled, "canceled"), + err: grpcstatus.Error(grpccodes.Canceled, "canceled"), expected: true, }, + { + name: "experimental Error with downstream source and status", + err: errorsource.New(errors.New("test"), backend.ErrorSourceDownstream, backend.StatusUnknown), + expected: true, + }, + { + name: "experimental Error with plugin source and status", + err: errorsource.New(errors.New("test"), backend.ErrorSourcePlugin, backend.StatusUnknown), + expected: false, + }, } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { wrappedErr := fmt.Errorf("error: %w", tc.err) joinedErr := errors.Join(errors.New("oh no"), tc.err) - assert.Equalf(t, tc.expected, IsDownstreamError(tc.err), "IsDownstreamHTTPError(%v)", tc.err) - assert.Equalf(t, tc.expected, IsDownstreamError(wrappedErr), "wrapped IsDownstreamHTTPError(%v)", wrappedErr) - assert.Equalf(t, tc.expected, IsDownstreamError(joinedErr), "joined IsDownstreamHTTPError(%v)", joinedErr) + assert.Equalf(t, tc.expected, status.IsDownstreamError(tc.err), "IsDownstreamHTTPError(%v)", tc.err) + assert.Equalf(t, tc.expected, status.IsDownstreamError(wrappedErr), "wrapped IsDownstreamHTTPError(%v)", wrappedErr) + assert.Equalf(t, tc.expected, status.IsDownstreamError(joinedErr), "joined IsDownstreamHTTPError(%v)", joinedErr) }) } } @@ -101,7 +114,7 @@ func TestIsDownstreamHTTPError(t *testing.T) { }, { name: "downstream error", - err: DownstreamError(nil), + err: status.DownstreamError(nil), expected: true, }, { @@ -136,9 +149,19 @@ func TestIsDownstreamHTTPError(t *testing.T) { }, { name: "gRPC canceled error", - err: status.Error(codes.Canceled, "canceled"), + err: grpcstatus.Error(grpccodes.Canceled, "canceled"), expected: true, }, + { + name: "experimental Error with downstream source and status", + err: errorsource.New(errors.New("test"), backend.ErrorSourceDownstream, backend.StatusUnknown), + expected: true, + }, + { + name: "experimental Error with plugin source and status", + err: errorsource.New(errors.New("test"), backend.ErrorSourcePlugin, backend.StatusUnknown), + expected: false, + }, { name: "connection reset error", err: &net.OpError{Err: &os.SyscallError{Err: syscall.ECONNREFUSED}}, @@ -159,9 +182,9 @@ func TestIsDownstreamHTTPError(t *testing.T) { t.Run(tc.name, func(t *testing.T) { wrappedErr := fmt.Errorf("error: %w", tc.err) joinedErr := errors.Join(errors.New("oh no"), tc.err) - assert.Equalf(t, tc.expected, IsDownstreamHTTPError(tc.err), "IsDownstreamHTTPError(%v)", tc.err) - assert.Equalf(t, tc.expected, IsDownstreamHTTPError(wrappedErr), "wrapped IsDownstreamHTTPError(%v)", wrappedErr) - assert.Equalf(t, tc.expected, IsDownstreamHTTPError(joinedErr), "joined IsDownstreamHTTPError(%v)", joinedErr) + assert.Equalf(t, tc.expected, status.IsDownstreamHTTPError(tc.err), "IsDownstreamHTTPError(%v)", tc.err) + assert.Equalf(t, tc.expected, status.IsDownstreamHTTPError(wrappedErr), "wrapped IsDownstreamHTTPError(%v)", wrappedErr) + assert.Equalf(t, tc.expected, status.IsDownstreamHTTPError(joinedErr), "joined IsDownstreamHTTPError(%v)", joinedErr) }) } } From d8a3bee1c077017fb291a393474dad24cb323735 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 10 Oct 2024 18:30:53 +0200 Subject: [PATCH 3/9] lint fix --- backend/status/status_source.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/status/status_source.go b/backend/status/status_source.go index d9f42c491..b316a96cd 100644 --- a/backend/status/status_source.go +++ b/backend/status/status_source.go @@ -120,7 +120,6 @@ func IsDownstreamError(err error) bool { type errorWithSource interface { ErrorSource() Source - Error() string } errCopy := err @@ -131,6 +130,7 @@ func IsDownstreamError(err error) bool { return true } + // nolint:errorlint if uw, ok := errCopy.(interface{ Unwrap() []error }); ok { errs := uw.Unwrap() for _, joinErr := range errs { From e5fbc28aca8b722acb89ffbe6f7dd235c68f627d Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 10 Oct 2024 18:39:59 +0200 Subject: [PATCH 4/9] skip supporting joined errors for now --- backend/status/status_source.go | 14 ++------ backend/status/status_source_test.go | 52 +++++++++++++++++----------- 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/backend/status/status_source.go b/backend/status/status_source.go index b316a96cd..d55f2a925 100644 --- a/backend/status/status_source.go +++ b/backend/status/status_source.go @@ -122,25 +122,15 @@ func IsDownstreamError(err error) bool { ErrorSource() Source } + // temporary hack to support wrapped errors until experimental errorsource package have been removed. + // Note. Joined errors (errors.Join()) is not supported. errCopy := err - for { // nolint:errorlint if errWithSource, ok := errCopy.(errorWithSource); ok && errWithSource.ErrorSource() == SourceDownstream { return true } - // nolint:errorlint - if uw, ok := errCopy.(interface{ Unwrap() []error }); ok { - errs := uw.Unwrap() - for _, joinErr := range errs { - // nolint:errorlint - if errWithSource, ok := joinErr.(errorWithSource); ok && errWithSource.ErrorSource() == SourceDownstream { - return true - } - } - } - if errCopy = errors.Unwrap(errCopy); errCopy == nil { break } diff --git a/backend/status/status_source_test.go b/backend/status/status_source_test.go index 3eb147760..a1a5bc951 100644 --- a/backend/status/status_source_test.go +++ b/backend/status/status_source_test.go @@ -30,9 +30,10 @@ func TestSource(t *testing.T) { func TestIsDownstreamError(t *testing.T) { tcs := []struct { - name string - err error - expected bool + name string + err error + expected bool + skipJoined bool }{ { name: "nil", @@ -80,14 +81,16 @@ func TestIsDownstreamError(t *testing.T) { expected: true, }, { - name: "experimental Error with downstream source and status", - err: errorsource.New(errors.New("test"), backend.ErrorSourceDownstream, backend.StatusUnknown), - expected: true, + name: "experimental Error with downstream source and status", + err: errorsource.New(errors.New("test"), backend.ErrorSourceDownstream, backend.StatusUnknown), + skipJoined: true, + expected: true, }, { - name: "experimental Error with plugin source and status", - err: errorsource.New(errors.New("test"), backend.ErrorSourcePlugin, backend.StatusUnknown), - expected: false, + name: "experimental Error with plugin source and status", + err: errorsource.New(errors.New("test"), backend.ErrorSourcePlugin, backend.StatusUnknown), + skipJoined: true, + expected: false, }, } for _, tc := range tcs { @@ -96,16 +99,20 @@ func TestIsDownstreamError(t *testing.T) { joinedErr := errors.Join(errors.New("oh no"), tc.err) assert.Equalf(t, tc.expected, status.IsDownstreamError(tc.err), "IsDownstreamHTTPError(%v)", tc.err) assert.Equalf(t, tc.expected, status.IsDownstreamError(wrappedErr), "wrapped IsDownstreamHTTPError(%v)", wrappedErr) - assert.Equalf(t, tc.expected, status.IsDownstreamError(joinedErr), "joined IsDownstreamHTTPError(%v)", joinedErr) + + if !tc.skipJoined { + assert.Equalf(t, tc.expected, status.IsDownstreamError(joinedErr), "joined IsDownstreamHTTPError(%v)", joinedErr) + } }) } } func TestIsDownstreamHTTPError(t *testing.T) { tcs := []struct { - name string - err error - expected bool + name string + err error + expected bool + skipJoined bool }{ { name: "nil", @@ -153,14 +160,16 @@ func TestIsDownstreamHTTPError(t *testing.T) { expected: true, }, { - name: "experimental Error with downstream source and status", - err: errorsource.New(errors.New("test"), backend.ErrorSourceDownstream, backend.StatusUnknown), - expected: true, + name: "experimental Error with downstream source and status", + err: errorsource.New(errors.New("test"), backend.ErrorSourceDownstream, backend.StatusUnknown), + skipJoined: true, + expected: true, }, { - name: "experimental Error with plugin source and status", - err: errorsource.New(errors.New("test"), backend.ErrorSourcePlugin, backend.StatusUnknown), - expected: false, + name: "experimental Error with plugin source and status", + err: errorsource.New(errors.New("test"), backend.ErrorSourcePlugin, backend.StatusUnknown), + skipJoined: true, + expected: false, }, { name: "connection reset error", @@ -184,7 +193,10 @@ func TestIsDownstreamHTTPError(t *testing.T) { joinedErr := errors.Join(errors.New("oh no"), tc.err) assert.Equalf(t, tc.expected, status.IsDownstreamHTTPError(tc.err), "IsDownstreamHTTPError(%v)", tc.err) assert.Equalf(t, tc.expected, status.IsDownstreamHTTPError(wrappedErr), "wrapped IsDownstreamHTTPError(%v)", wrappedErr) - assert.Equalf(t, tc.expected, status.IsDownstreamHTTPError(joinedErr), "joined IsDownstreamHTTPError(%v)", joinedErr) + + if !tc.skipJoined { + assert.Equalf(t, tc.expected, status.IsDownstreamHTTPError(joinedErr), "joined IsDownstreamHTTPError(%v)", joinedErr) + } }) } } From 1518be5ce56df0a42c46dd3f5af398325bde219d Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 11 Oct 2024 11:39:01 +0200 Subject: [PATCH 5/9] move status package to experimental for now --- backend/data_adapter.go | 2 +- backend/error_source.go | 2 +- backend/httpclient/error_source_middleware.go | 2 +- backend/request_status.go | 2 +- experimental/errorsource/error_source_middleware.go | 2 +- {backend => experimental}/status/doc.go | 0 {backend => experimental}/status/status_source.go | 0 {backend => experimental}/status/status_source_test.go | 2 +- experimental/testdata/folder.golden.txt | 4 ++-- 9 files changed, 8 insertions(+), 8 deletions(-) rename {backend => experimental}/status/doc.go (100%) rename {backend => experimental}/status/status_source.go (100%) rename {backend => experimental}/status/status_source_test.go (98%) diff --git a/backend/data_adapter.go b/backend/data_adapter.go index 31d69f429..98919bae4 100644 --- a/backend/data_adapter.go +++ b/backend/data_adapter.go @@ -5,7 +5,7 @@ import ( "errors" "fmt" - "github.com/grafana/grafana-plugin-sdk-go/backend/status" + "github.com/grafana/grafana-plugin-sdk-go/experimental/status" "github.com/grafana/grafana-plugin-sdk-go/genproto/pluginv2" ) diff --git a/backend/error_source.go b/backend/error_source.go index 4ff4d2462..b914c8717 100644 --- a/backend/error_source.go +++ b/backend/error_source.go @@ -4,7 +4,7 @@ import ( "context" "fmt" - "github.com/grafana/grafana-plugin-sdk-go/backend/status" + "github.com/grafana/grafana-plugin-sdk-go/experimental/status" ) // ErrorSource type defines the source of the error diff --git a/backend/httpclient/error_source_middleware.go b/backend/httpclient/error_source_middleware.go index 17594443a..0d8f7bf73 100644 --- a/backend/httpclient/error_source_middleware.go +++ b/backend/httpclient/error_source_middleware.go @@ -3,7 +3,7 @@ package httpclient import ( "net/http" - "github.com/grafana/grafana-plugin-sdk-go/backend/status" + "github.com/grafana/grafana-plugin-sdk-go/experimental/status" ) // ErrorSourceMiddlewareName is the middleware name used by ErrorSourceMiddleware. diff --git a/backend/request_status.go b/backend/request_status.go index e2e490633..d81afe05b 100644 --- a/backend/request_status.go +++ b/backend/request_status.go @@ -4,7 +4,7 @@ import ( "context" "strings" - "github.com/grafana/grafana-plugin-sdk-go/backend/status" + "github.com/grafana/grafana-plugin-sdk-go/experimental/status" "github.com/grafana/grafana-plugin-sdk-go/genproto/pluginv2" ) diff --git a/experimental/errorsource/error_source_middleware.go b/experimental/errorsource/error_source_middleware.go index 4245ffeeb..4514dd4ce 100644 --- a/experimental/errorsource/error_source_middleware.go +++ b/experimental/errorsource/error_source_middleware.go @@ -6,7 +6,7 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" - "github.com/grafana/grafana-plugin-sdk-go/backend/status" + "github.com/grafana/grafana-plugin-sdk-go/experimental/status" ) // Middleware captures error source metric diff --git a/backend/status/doc.go b/experimental/status/doc.go similarity index 100% rename from backend/status/doc.go rename to experimental/status/doc.go diff --git a/backend/status/status_source.go b/experimental/status/status_source.go similarity index 100% rename from backend/status/status_source.go rename to experimental/status/status_source.go diff --git a/backend/status/status_source_test.go b/experimental/status/status_source_test.go similarity index 98% rename from backend/status/status_source_test.go rename to experimental/status/status_source_test.go index a1a5bc951..d6e3352bd 100644 --- a/backend/status/status_source_test.go +++ b/experimental/status/status_source_test.go @@ -10,8 +10,8 @@ import ( "testing" "github.com/grafana/grafana-plugin-sdk-go/backend" - "github.com/grafana/grafana-plugin-sdk-go/backend/status" "github.com/grafana/grafana-plugin-sdk-go/experimental/errorsource" + "github.com/grafana/grafana-plugin-sdk-go/experimental/status" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" grpccodes "google.golang.org/grpc/codes" diff --git a/experimental/testdata/folder.golden.txt b/experimental/testdata/folder.golden.txt index 4c305deb6..3c1cbc298 100644 --- a/experimental/testdata/folder.golden.txt +++ b/experimental/testdata/folder.golden.txt @@ -9,7 +9,7 @@ Frame[0] { "pathSeparator": "/" } Name: -Dimensions: 2 Fields by 22 Rows +Dimensions: 2 Fields by 23 Rows +----------------+------------------+ | Name: name | Name: media-type | | Labels: | Labels: | @@ -29,4 +29,4 @@ Dimensions: 2 Fields by 22 Rows ====== TEST DATA RESPONSE (arrow base64) ====== -FRAME=QVJST1cxAAD/////yAEAABAAAAAAAAoADgAMAAsABAAKAAAAFAAAAAAAAAEEAAoADAAAAAgABAAKAAAACAAAALgAAAADAAAATAAAACgAAAAEAAAAwP7//wgAAAAMAAAAAAAAAAAAAAAFAAAAcmVmSWQAAADg/v//CAAAAAwAAAAAAAAAAAAAAAQAAABuYW1lAAAAAAD///8IAAAAUAAAAEQAAAB7InR5cGUiOiJkaXJlY3RvcnktbGlzdGluZyIsInR5cGVWZXJzaW9uIjpbMCwwXSwicGF0aFNlcGFyYXRvciI6Ii8ifQAAAAAEAAAAbWV0YQAAAAACAAAAeAAAAAQAAACi////FAAAADwAAAA8AAAAAAAABTgAAAABAAAABAAAAJD///8IAAAAEAAAAAYAAABzdHJpbmcAAAYAAAB0c3R5cGUAAAAAAACI////CgAAAG1lZGlhLXR5cGUAAAAAEgAYABQAAAATAAwAAAAIAAQAEgAAABQAAABEAAAASAAAAAAAAAVEAAAAAQAAAAwAAAAIAAwACAAEAAgAAAAIAAAAEAAAAAYAAABzdHJpbmcAAAYAAAB0c3R5cGUAAAAAAAAEAAQABAAAAAQAAABuYW1lAAAAAP/////YAAAAFAAAAAAAAAAMABYAFAATAAwABAAMAAAASAIAAAAAAAAUAAAAAAAAAwQACgAYAAwACAAEAAoAAAAUAAAAeAAAABYAAAAAAAAAAAAAAAYAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABcAAAAAAAAAGAAAAAAAAAABAEAAAAAAABoAQAAAAAAAAAAAAAAAAAAaAEAAAAAAABcAAAAAAAAAMgBAAAAAAAAfgAAAAAAAAAAAAAAAgAAABYAAAAAAAAAAAAAAAAAAAAWAAAAAAAAAAAAAAAAAAAAAAAAAAkAAAAQAAAAFAAAAB4AAAAoAAAANgAAADkAAABEAAAAUgAAAF0AAABtAAAAfAAAAJAAAACqAAAAyQAAANQAAADaAAAA3gAAAOwAAAD5AAAA/AAAAAQBAAAAAAAAUkVBRE1FLm1kYWN0aW9uc2FwaXNhdXRoY2xpZW50Y29uY3VycmVudGRhdGFzb3VyY2V0ZXN0ZTJlZXJyb3Jzb3VyY2VmZWF0dXJldG9nZ2xlc2ZpbGVpbmZvLmdvZmlsZWluZm9fdGVzdC5nb2ZyYW1lX3NvcnRlci5nb2ZyYW1lX3NvcnRlcl90ZXN0LmdvZ29sZGVuX3Jlc3BvbnNlX2NoZWNrZXIuZ29nb2xkZW5fcmVzcG9uc2VfY2hlY2tlcl90ZXN0LmdvaHR0cF9sb2dnZXJtYWNyb3Ntb2NrcmVzdF9jbGllbnQuZ29zY2hlbWFidWlsZGVyc2xvdGVzdGRhdGEAAAAAAAAAAAAAAAAJAAAAEgAAABsAAAAkAAAALQAAADYAAAA/AAAASAAAAEgAAABIAAAASAAAAEgAAABIAAAASAAAAFEAAABaAAAAYwAAAGMAAABsAAAAdQAAAH4AAAAAAAAAZGlyZWN0b3J5ZGlyZWN0b3J5ZGlyZWN0b3J5ZGlyZWN0b3J5ZGlyZWN0b3J5ZGlyZWN0b3J5ZGlyZWN0b3J5ZGlyZWN0b3J5ZGlyZWN0b3J5ZGlyZWN0b3J5ZGlyZWN0b3J5ZGlyZWN0b3J5ZGlyZWN0b3J5ZGlyZWN0b3J5AAAQAAAADAAUABIADAAIAAQADAAAABAAAAAsAAAAPAAAAAAABAABAAAA2AEAAAAAAADgAAAAAAAAAEgCAAAAAAAAAAAAAAAAAAAAAAAAAAAKAAwAAAAIAAQACgAAAAgAAAC4AAAAAwAAAEwAAAAoAAAABAAAAMD+//8IAAAADAAAAAAAAAAAAAAABQAAAHJlZklkAAAA4P7//wgAAAAMAAAAAAAAAAAAAAAEAAAAbmFtZQAAAAAA////CAAAAFAAAABEAAAAeyJ0eXBlIjoiZGlyZWN0b3J5LWxpc3RpbmciLCJ0eXBlVmVyc2lvbiI6WzAsMF0sInBhdGhTZXBhcmF0b3IiOiIvIn0AAAAABAAAAG1ldGEAAAAAAgAAAHgAAAAEAAAAov///xQAAAA8AAAAPAAAAAAAAAU4AAAAAQAAAAQAAACQ////CAAAABAAAAAGAAAAc3RyaW5nAAAGAAAAdHN0eXBlAAAAAAAAiP///woAAABtZWRpYS10eXBlAAAAABIAGAAUAAAAEwAMAAAACAAEABIAAAAUAAAARAAAAEgAAAAAAAAFRAAAAAEAAAAMAAAACAAMAAgABAAIAAAACAAAABAAAAAGAAAAc3RyaW5nAAAGAAAAdHN0eXBlAAAAAAAABAAEAAQAAAAEAAAAbmFtZQAAAAD4AQAAQVJST1cx +FRAME=QVJST1cxAAD/////yAEAABAAAAAAAAoADgAMAAsABAAKAAAAFAAAAAAAAAEEAAoADAAAAAgABAAKAAAACAAAALgAAAADAAAATAAAACgAAAAEAAAAwP7//wgAAAAMAAAAAAAAAAAAAAAFAAAAcmVmSWQAAADg/v//CAAAAAwAAAAAAAAAAAAAAAQAAABuYW1lAAAAAAD///8IAAAAUAAAAEQAAAB7InR5cGUiOiJkaXJlY3RvcnktbGlzdGluZyIsInR5cGVWZXJzaW9uIjpbMCwwXSwicGF0aFNlcGFyYXRvciI6Ii8ifQAAAAAEAAAAbWV0YQAAAAACAAAAeAAAAAQAAACi////FAAAADwAAAA8AAAAAAAABTgAAAABAAAABAAAAJD///8IAAAAEAAAAAYAAABzdHJpbmcAAAYAAAB0c3R5cGUAAAAAAACI////CgAAAG1lZGlhLXR5cGUAAAAAEgAYABQAAAATAAwAAAAIAAQAEgAAABQAAABEAAAASAAAAAAAAAVEAAAAAQAAAAwAAAAIAAwACAAEAAgAAAAIAAAAEAAAAAYAAABzdHJpbmcAAAYAAAB0c3R5cGUAAAAAAAAEAAQABAAAAAQAAABuYW1lAAAAAP/////YAAAAFAAAAAAAAAAMABYAFAATAAwABAAMAAAAWAIAAAAAAAAUAAAAAAAAAwQACgAYAAwACAAEAAoAAAAUAAAAeAAAABcAAAAAAAAAAAAAAAYAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABgAAAAAAAAAGAAAAAAAAAACgEAAAAAAABwAQAAAAAAAAAAAAAAAAAAcAEAAAAAAABgAAAAAAAAANABAAAAAAAAhwAAAAAAAAAAAAAAAgAAABcAAAAAAAAAAAAAAAAAAAAXAAAAAAAAAAAAAAAAAAAAAAAAAAkAAAAQAAAAFAAAAB4AAAAoAAAANgAAADkAAABEAAAAUgAAAF0AAABtAAAAfAAAAJAAAACqAAAAyQAAANQAAADaAAAA3gAAAOwAAAD5AAAA/AAAAAIBAAAKAQAAUkVBRE1FLm1kYWN0aW9uc2FwaXNhdXRoY2xpZW50Y29uY3VycmVudGRhdGFzb3VyY2V0ZXN0ZTJlZXJyb3Jzb3VyY2VmZWF0dXJldG9nZ2xlc2ZpbGVpbmZvLmdvZmlsZWluZm9fdGVzdC5nb2ZyYW1lX3NvcnRlci5nb2ZyYW1lX3NvcnRlcl90ZXN0LmdvZ29sZGVuX3Jlc3BvbnNlX2NoZWNrZXIuZ29nb2xkZW5fcmVzcG9uc2VfY2hlY2tlcl90ZXN0LmdvaHR0cF9sb2dnZXJtYWNyb3Ntb2NrcmVzdF9jbGllbnQuZ29zY2hlbWFidWlsZGVyc2xvc3RhdHVzdGVzdGRhdGEAAAAAAAAAAAAAAAAAAAkAAAASAAAAGwAAACQAAAAtAAAANgAAAD8AAABIAAAASAAAAEgAAABIAAAASAAAAEgAAABIAAAAUQAAAFoAAABjAAAAYwAAAGwAAAB1AAAAfgAAAIcAAABkaXJlY3RvcnlkaXJlY3RvcnlkaXJlY3RvcnlkaXJlY3RvcnlkaXJlY3RvcnlkaXJlY3RvcnlkaXJlY3RvcnlkaXJlY3RvcnlkaXJlY3RvcnlkaXJlY3RvcnlkaXJlY3RvcnlkaXJlY3RvcnlkaXJlY3RvcnlkaXJlY3RvcnlkaXJlY3RvcnkAEAAAAAwAFAASAAwACAAEAAwAAAAQAAAALAAAADwAAAAAAAQAAQAAANgBAAAAAAAA4AAAAAAAAABYAgAAAAAAAAAAAAAAAAAAAAAAAAAACgAMAAAACAAEAAoAAAAIAAAAuAAAAAMAAABMAAAAKAAAAAQAAADA/v//CAAAAAwAAAAAAAAAAAAAAAUAAAByZWZJZAAAAOD+//8IAAAADAAAAAAAAAAAAAAABAAAAG5hbWUAAAAAAP///wgAAABQAAAARAAAAHsidHlwZSI6ImRpcmVjdG9yeS1saXN0aW5nIiwidHlwZVZlcnNpb24iOlswLDBdLCJwYXRoU2VwYXJhdG9yIjoiLyJ9AAAAAAQAAABtZXRhAAAAAAIAAAB4AAAABAAAAKL///8UAAAAPAAAADwAAAAAAAAFOAAAAAEAAAAEAAAAkP///wgAAAAQAAAABgAAAHN0cmluZwAABgAAAHRzdHlwZQAAAAAAAIj///8KAAAAbWVkaWEtdHlwZQAAAAASABgAFAAAABMADAAAAAgABAASAAAAFAAAAEQAAABIAAAAAAAABUQAAAABAAAADAAAAAgADAAIAAQACAAAAAgAAAAQAAAABgAAAHN0cmluZwAABgAAAHRzdHlwZQAAAAAAAAQABAAEAAAABAAAAG5hbWUAAAAA+AEAAEFSUk9XMQ== From ca6c0976a9ee4889af50f35f119cc19b162f363f Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 11 Oct 2024 12:16:38 +0200 Subject: [PATCH 6/9] middleware tests --- .../error_source_middleware_test.go | 51 +++++++++++++++++++ backend/httpclient/http_client_test.go | 6 +++ 2 files changed, 57 insertions(+) create mode 100644 backend/httpclient/error_source_middleware_test.go diff --git a/backend/httpclient/error_source_middleware_test.go b/backend/httpclient/error_source_middleware_test.go new file mode 100644 index 000000000..5c283e2cf --- /dev/null +++ b/backend/httpclient/error_source_middleware_test.go @@ -0,0 +1,51 @@ +package httpclient + +import ( + "errors" + "net" + "net/http" + "testing" + + "github.com/grafana/grafana-plugin-sdk-go/experimental/status" + "github.com/stretchr/testify/require" +) + +func TestErrorSourceMiddleware(t *testing.T) { + t.Run("With non-downstream HTTP error returned from http.RoundTripper should not be wrapped in a downstream error", func(t *testing.T) { + ctx := &testContext{} + someErr := errors.New("some error") + finalRoundTripper := ctx.createRoundTripperWithError(someErr) + mw := ErrorSourceMiddleware() + rt := mw.CreateMiddleware(Options{}, finalRoundTripper) + require.NotNil(t, rt) + middlewareName, ok := mw.(MiddlewareName) + require.True(t, ok) + require.Equal(t, ErrorSourceMiddlewareName, middlewareName.MiddlewareName()) + + req, err := http.NewRequest(http.MethodGet, "http://", nil) + require.NoError(t, err) + _, err = rt.RoundTrip(req) + require.Error(t, err) + require.False(t, status.IsDownstreamError(err)) + require.ErrorIs(t, err, someErr) + }) + + t.Run("With downstream HTTP error returned from http.RoundTripper should be wrapped in a downstream error", func(t *testing.T) { + ctx := &testContext{} + someErr := &net.DNSError{IsNotFound: true} + finalRoundTripper := ctx.createRoundTripperWithError(someErr) + mw := ErrorSourceMiddleware() + rt := mw.CreateMiddleware(Options{}, finalRoundTripper) + require.NotNil(t, rt) + middlewareName, ok := mw.(MiddlewareName) + require.True(t, ok) + require.Equal(t, ErrorSourceMiddlewareName, middlewareName.MiddlewareName()) + + req, err := http.NewRequest(http.MethodGet, "http://", nil) + require.NoError(t, err) + _, err = rt.RoundTrip(req) + require.Error(t, err) + require.True(t, status.IsDownstreamError(err)) + require.ErrorIs(t, err, someErr) + }) +} diff --git a/backend/httpclient/http_client_test.go b/backend/httpclient/http_client_test.go index e8875d180..286a9167b 100644 --- a/backend/httpclient/http_client_test.go +++ b/backend/httpclient/http_client_test.go @@ -170,6 +170,12 @@ func (c *testContext) createRoundTripper(name string) http.RoundTripper { }) } +func (c *testContext) createRoundTripperWithError(err error) http.RoundTripper { + return RoundTripperFunc(func(_ *http.Request) (*http.Response, error) { + return nil, err + }) +} + func (c *testContext) createMiddleware(name string) Middleware { return NamedMiddlewareFunc(name, func(_ Options, next http.RoundTripper) http.RoundTripper { return RoundTripperFunc(func(req *http.Request) (*http.Response, error) { From 02ca2a1260fc0024faa982c1752c148e21b3af68 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 11 Oct 2024 12:17:56 +0200 Subject: [PATCH 7/9] lint fix --- backend/httpclient/error_source_middleware_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/backend/httpclient/error_source_middleware_test.go b/backend/httpclient/error_source_middleware_test.go index 5c283e2cf..34cbb9c29 100644 --- a/backend/httpclient/error_source_middleware_test.go +++ b/backend/httpclient/error_source_middleware_test.go @@ -24,8 +24,9 @@ func TestErrorSourceMiddleware(t *testing.T) { req, err := http.NewRequest(http.MethodGet, "http://", nil) require.NoError(t, err) - _, err = rt.RoundTrip(req) + resp, err := rt.RoundTrip(req) require.Error(t, err) + require.Nil(t, resp) require.False(t, status.IsDownstreamError(err)) require.ErrorIs(t, err, someErr) }) @@ -43,8 +44,9 @@ func TestErrorSourceMiddleware(t *testing.T) { req, err := http.NewRequest(http.MethodGet, "http://", nil) require.NoError(t, err) - _, err = rt.RoundTrip(req) + resp, err := rt.RoundTrip(req) require.Error(t, err) + require.Nil(t, resp) require.True(t, status.IsDownstreamError(err)) require.ErrorIs(t, err, someErr) }) From d783f353a3e0770040a18ca2914a727dc33b9108 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 11 Oct 2024 12:25:44 +0200 Subject: [PATCH 8/9] lint fix --- backend/httpclient/error_source_middleware_test.go | 8 ++++++-- backend/httpclient/http_client_test.go | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/backend/httpclient/error_source_middleware_test.go b/backend/httpclient/error_source_middleware_test.go index 34cbb9c29..c882c8572 100644 --- a/backend/httpclient/error_source_middleware_test.go +++ b/backend/httpclient/error_source_middleware_test.go @@ -26,7 +26,9 @@ func TestErrorSourceMiddleware(t *testing.T) { require.NoError(t, err) resp, err := rt.RoundTrip(req) require.Error(t, err) - require.Nil(t, resp) + if resp.Body != nil { + require.NoError(t, resp.Body.Close()) + } require.False(t, status.IsDownstreamError(err)) require.ErrorIs(t, err, someErr) }) @@ -46,7 +48,9 @@ func TestErrorSourceMiddleware(t *testing.T) { require.NoError(t, err) resp, err := rt.RoundTrip(req) require.Error(t, err) - require.Nil(t, resp) + if resp.Body != nil { + require.NoError(t, resp.Body.Close()) + } require.True(t, status.IsDownstreamError(err)) require.ErrorIs(t, err, someErr) }) diff --git a/backend/httpclient/http_client_test.go b/backend/httpclient/http_client_test.go index 286a9167b..e75e9bb73 100644 --- a/backend/httpclient/http_client_test.go +++ b/backend/httpclient/http_client_test.go @@ -172,7 +172,7 @@ func (c *testContext) createRoundTripper(name string) http.RoundTripper { func (c *testContext) createRoundTripperWithError(err error) http.RoundTripper { return RoundTripperFunc(func(_ *http.Request) (*http.Response, error) { - return nil, err + return &http.Response{}, err }) } From 351c5a5747d04d8a50854ae53e89303892435238 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 11 Oct 2024 12:28:08 +0200 Subject: [PATCH 9/9] remove unused/not needed function --- backend/error_source.go | 5 ----- experimental/status/status_source.go | 8 -------- 2 files changed, 13 deletions(-) diff --git a/backend/error_source.go b/backend/error_source.go index b914c8717..1f03fcf56 100644 --- a/backend/error_source.go +++ b/backend/error_source.go @@ -21,11 +21,6 @@ const ( DefaultErrorSource = status.SourcePlugin ) -// ErrorSourceFromHTTPError returns an [ErrorSource] based on provided error. -func ErrorSourceFromHTTPError(err error) ErrorSource { - return status.SourceFromHTTPError(err) -} - // ErrorSourceFromHTTPStatus returns an [ErrorSource] based on provided HTTP status code. func ErrorSourceFromHTTPStatus(statusCode int) ErrorSource { return status.SourceFromHTTPStatus(statusCode) diff --git a/experimental/status/status_source.go b/experimental/status/status_source.go index d55f2a925..a484d7066 100644 --- a/experimental/status/status_source.go +++ b/experimental/status/status_source.go @@ -41,14 +41,6 @@ func (s Source) String() string { return string(s) } -// SourceFromHTTPError returns a [Source] based on provided error. -func SourceFromHTTPError(err error) Source { - if IsDownstreamHTTPError(err) { - return SourceDownstream - } - return SourcePlugin -} - // ErrorSourceFromStatus returns a [Source] based on provided HTTP status code. func SourceFromHTTPStatus(statusCode int) Source { switch statusCode {