Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Delete unused error handling code #3914

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Delete unused error handling code #3914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 0 additions & 122 deletions internal/common/armadaerrors/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,9 @@ package armadaerrors
import (
"context"
"fmt"
"io"
"net"
"regexp"
"strings"
"syscall"

"github.com/apache/pulsar-client-go/pulsar"
"github.com/pkg/errors"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -190,90 +186,6 @@ func CodeFromError(err error) codes.Code {
return codes.Unknown
}

var PULSAR_CONNECTION_ERRORS = []pulsar.Result{
pulsar.TimeoutError,
pulsar.LookupError,
pulsar.ConnectError,
pulsar.ReadError,
pulsar.NotConnectedError,
pulsar.TooManyLookupRequestException,
pulsar.ServiceUnitNotReady,
pulsar.ProducerQueueIsFull,
}

// IsNetworkError returns true if err is a network-related error.
// If err is an error chain, this function returns true if any error in the chain is a network error.
//
// For details, see
// https://stackoverflow.com/questions/22761562/portable-way-to-detect-different-kinds-of-network-error
func IsNetworkError(err error) bool {
// Return immediately on nil.
if err == nil {
return false
}

// Because deadline exceeded is typically caused by a network timeout, we consider it a network error.
if ok := errors.Is(err, context.DeadlineExceeded); ok {
return true
}

// EOF indicates a network termination
if errors.Is(err, io.EOF) {
return true
}

// Generic network errors in the net package. Redis returns these.
{
var e net.Error
if ok := errors.As(err, &e); ok {
return true
}
}
{
var e *net.OpError
if ok := errors.As(err, &e); ok {
return true
}
}

// Generic syscall errors.
// Not sure if anything returns this, but it seems proper to check.
{
var e syscall.Errno
if ok := errors.As(err, &e); ok {
if e == syscall.ECONNREFUSED {
return true
} else if e == syscall.ECONNRESET {
return true
} else if e == syscall.ECONNABORTED {
return true
}
}
}

// Errors associated with connection problems with Pulsar.
{
var e *pulsar.Error
if ok := errors.As(err, &e); ok {
for _, result := range PULSAR_CONNECTION_ERRORS {
if e.Result() == result {
return true
}
}
}
}

// Pulsar subscribe returns an errors.errorString with a particular message
// (as opposed to using its internal error type).
if e := errors.Cause(err); e != nil {
if strings.Contains(e.Error(), "connection error") { // Pulsar subscribe
return true
}
}

return false
}

// Add the action to the error if possible.
func addAction(err error, action string) {
{
Expand Down Expand Up @@ -446,40 +358,6 @@ func (err *ErrPodUnschedulable) Error() string {
return b.String()
}

// NewCombinedErrPodUnschedulable returns a new ErrPodUnschedulable with
// countFromReasons aggregated over all arguments.
func NewCombinedErrPodUnschedulable(errs ...error) *ErrPodUnschedulable {
if len(errs) == 0 {
return nil
}

result := &ErrPodUnschedulable{
countFromReason: make(map[string]int),
}
for _, err := range errs {
if err == nil {
continue
}

// If the error is of type *ErrPodUnschedulable, merge the reasons.
if e, ok := err.(*ErrPodUnschedulable); ok {
if len(e.countFromReason) == 0 {
continue
}
for reason, count := range e.countFromReason {
result.countFromReason[reason] += count
}
} else { // Otherwise, add the error message as a reason.
result.countFromReason[err.Error()] += 1
}
}

if len(result.countFromReason) == 0 {
return nil
}
return result
}

// ErrUnauthenticated represents an error that occurs when a client tries to
// perform some action through the gRPC API for which it cannot authenticate.
//
Expand Down
47 changes: 0 additions & 47 deletions internal/common/armadaerrors/errors_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,12 @@ package armadaerrors

import (
"context"
goerrors "errors"
"fmt"
"regexp"
"testing"
"time"

"github.com/apache/pulsar-client-go/pulsar"
grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
"github.com/pkg/errors"
"github.com/redis/go-redis/v9"
"github.com/stretchr/testify/assert"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -177,49 +173,6 @@ func TestStreamServerInterceptor(t *testing.T) {
assert.Contains(t, st.Message(), "testMethod")
}

func TestIsNetworkErrorRedis(t *testing.T) {
client := redis.NewClient(&redis.Options{
Addr: "localhost:637", // Assume nothing is listening on this port
})
cmd := client.Ping(context.TODO())

err := errors.Wrap(cmd.Err(), "foo")
assert.True(t, IsNetworkError(err))

err = fmt.Errorf("%w", cmd.Err())
assert.True(t, IsNetworkError(err))
}

func TestIsNetworkErrorPulsar(t *testing.T) {
// Set the timeout really short to immediately get a network error.
// Setting the timeout to a nanosecond results in a panic in the Pulsar client.
client, err := pulsar.NewClient(pulsar.ClientOptions{
URL: "pulsar://pulsar:665", // Assume nothing is listening on this port
ConnectionTimeout: time.Millisecond,
OperationTimeout: time.Millisecond,
})
if ok := assert.NoError(t, err); !ok {
t.FailNow()
}
_, err = client.Subscribe(pulsar.ConsumerOptions{
Topic: "foo",
SubscriptionName: "foo",
})

assert.True(t, IsNetworkError(errors.Wrap(err, "foo")))
assert.True(t, IsNetworkError(fmt.Errorf("%w", err)))
}

func TestIsNotNetworkError(t *testing.T) {
assert.False(t, IsNetworkError(errors.New("foo")))
assert.False(t, IsNetworkError(goerrors.New("foo")))
assert.False(t, IsNetworkError(&ErrNotFound{}))
assert.False(t, IsNetworkError(&ErrAlreadyExists{}))
assert.False(t, IsNetworkError(&ErrInvalidArgument{}))
assert.False(t, IsNetworkError(errors.Wrap(&ErrNotFound{}, "foo")))
assert.False(t, IsNetworkError(fmt.Errorf("%w", &ErrNotFound{})))
}

func TestIsRetryablePostgresError(t *testing.T) {
fatalErrors := []*regexp.Regexp{
regexp.MustCompile(".* some error .*"),
Expand Down