Skip to content

Commit

Permalink
Set HTTP timeouts as downstream errors (#1063)
Browse files Browse the repository at this point in the history
* add timeout checks for downstream errors

* simplify

* add tests

* add wrapped error cases

* update tests

* Fix typo in comment

Co-authored-by: Giuseppe Guerra <giuseppe.guerra@grafana.com>

---------

Co-authored-by: Giuseppe Guerra <giuseppe.guerra@grafana.com>
  • Loading branch information
wbrowne and xnyo authored Aug 28, 2024
1 parent f1cb16c commit e1e82c0
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 0 deletions.
16 changes: 16 additions & 0 deletions backend/data_adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,16 @@ func (a *dataSDKAdapter) QueryData(ctx context.Context, req *pluginv2.QueryDataR
return RequestStatusCancelled, nil
}

if isHTTPTimeoutError(innerErr) {
return RequestStatusError, nil
}

// Set downstream status source in the context if there's at least one response with downstream status source,
// and if there's no plugin error
var hasPluginError bool
var hasDownstreamError bool
var hasCancelledError bool
var hasHTTPTimeoutError bool
for _, r := range resp.Responses {
if r.Error == nil {
continue
Expand All @@ -50,6 +55,10 @@ func (a *dataSDKAdapter) QueryData(ctx context.Context, req *pluginv2.QueryDataR
if isCancelledError(r.Error) {
hasCancelledError = true
}
if isHTTPTimeoutError(r.Error) {
hasHTTPTimeoutError = true
}

if r.ErrorSource == ErrorSourceDownstream {
hasDownstreamError = true
} else {
Expand All @@ -64,6 +73,13 @@ func (a *dataSDKAdapter) QueryData(ctx context.Context, req *pluginv2.QueryDataR
return RequestStatusCancelled, nil
}

if hasHTTPTimeoutError {
if err := WithDownstreamErrorSource(ctx); err != nil {
return RequestStatusError, fmt.Errorf("failed to set downstream status source: %w", errors.Join(innerErr, err))
}
return RequestStatusError, nil
}

// A plugin error has higher priority than a downstream error,
// so set to downstream only if there's no plugin error
if hasDownstreamError && !hasPluginError {
Expand Down
4 changes: 4 additions & 0 deletions backend/error_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ func IsDownstreamError(err error) bool {
return true
}

if isHTTPTimeoutError(err) {
return true
}

return false
}

Expand Down
101 changes: 101 additions & 0 deletions backend/error_source_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package backend

import (
"errors"
"fmt"
"net"
"os"
"testing"

"github.com/stretchr/testify/assert"
)

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,
},
}
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
}
11 changes: 11 additions & 0 deletions backend/request_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package backend
import (
"context"
"errors"
"net"
"os"
"strings"

grpccodes "google.golang.org/grpc/codes"
Expand Down Expand Up @@ -105,3 +107,12 @@ func RequestStatusFromProtoQueryDataResponse(res *pluginv2.QueryDataResponse, er
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)
}

0 comments on commit e1e82c0

Please sign in to comment.