Downgrade temporary net.Error to warning

sharedstore/memcache_client.go

@@ -3,6 +3,7 @@ package sharedstore
 import (
 	"bytes"
 	"encoding/gob"
+	"net"
 	"time"
 
 	"github.com/bradfitz/gomemcache/memcache"
@@ -53,7 +54,7 @@ func (w *memcacheClientWrapper) Get(key string) (*Item, error) {
 		if err == memcache.ErrCacheMiss {
 			err = nil
 		}
-		return nil, err
+		return nil, coalesceTimeoutError(err)
 	}
 
 	return decodeMemcacheItem(mItem)
@@ -64,7 +65,7 @@ func (w *memcacheClientWrapper) Set(key string, item *Item) error {
 	if err != nil {
 		return err
 	}
-	return w.client.Set(mItem)
+	return coalesceTimeoutError(w.client.Set(mItem))
 }
 
 func (w *memcacheClientWrapper) Add(key string, item *Item) error {
@@ -78,7 +79,7 @@ func (w *memcacheClientWrapper) Add(key string, item *Item) error {
 		// Abstract the memcache-specific error
 		return ErrNotStored
 	}
-	return err
+	return coalesceTimeoutError(err)
 }
 
 func (w *memcacheClientWrapper) Delete(key string) error {
@@ -87,5 +88,28 @@ func (w *memcacheClientWrapper) Delete(key string) error {
 		// Deleting a missing entry is not an actual issue.
 		return nil
 	}
-	return err
+	return coalesceTimeoutError(err)
+}
+
+type connectTimeoutError struct{}
+
+func (connectTimeoutError) Error() string   { return "memcache: connect timeout" }
+func (connectTimeoutError) Timeout() bool   { return true }
+func (connectTimeoutError) Temporary() bool { return true }
+
+func coalesceTimeoutError(err error) error {
+	// For some reason, gomemcache decided to replace the standard net.Error.
+	// Coalesce into a generic net.Error so that client don't have to deal with memcache-specific errors.
+	switch typed := err.(type) {
+	case *memcache.ConnectTimeoutError:
+		return &net.OpError{
+			Err:  &connectTimeoutError{},
+			Addr: typed.Addr,
+			Net:  typed.Addr.Network(),
+			Op:   "connect",
+		}
+	default:
+		// This also work if err is nil
+		return err
+	}
 }

sharedstore/memcache_client_test.go

@@ -2,6 +2,7 @@ package sharedstore
 
 import (
 	"encoding/gob"
+	"net"
 	"testing"
 	"time"
 
@@ -46,3 +47,16 @@ func Test_encodeMemcacheItem(t *testing.T) {
 		})
 	}
 }
+
+func Test_coalesceTimeoutError(t *testing.T) {
+	assert.Nil(t, coalesceTimeoutError(nil))
+
+	timeoutError := memcache.ConnectTimeoutError{Addr: &net.TCPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 1234}}
+	if err, ok := coalesceTimeoutError(&timeoutError).(net.Error); ok {
+		assert.Equal(t, "connect tcp 127.0.0.1:1234: memcache: connect timeout", err.Error())
+		assert.Equal(t, true, err.Timeout())
+		assert.Equal(t, true, err.Temporary())
+	} else {
+		assert.Fail(t, "should be a net.Error")
+	}
+}

sharedstore/store.go

@@ -2,6 +2,7 @@ package sharedstore
 
 import (
 	"context"
+	"net"
 	"time"
 
 	"github.com/pkg/errors"
@@ -59,7 +60,11 @@ func (s *store) Tomb() *tomb.Tomb {
 func (s *store) GetOrLock(ctx context.Context, key string) (Getter, Setter) {
 	item, err := s.getData(ctx, key)
 	if err != nil {
-		log(ctx, err).Error("unexpected client error")
+		if isTemporaryError(err) {
+			log(ctx, err).Warn("temporary client error")
+		} else {
+			log(ctx, err).Error("unexpected client error")
+		}
 	} else if item != nil {
 		return &resolvedGetter{
 			item: item,
@@ -84,7 +89,11 @@ func (s *store) GetOrLock(ctx context.Context, key string) (Getter, Setter) {
 	}
 
 	if ok, err := s.lock(ctx, key); err != nil {
-		log(ctx, err).Error("unable to lock item")
+		if isTemporaryError(err) {
+			log(ctx, err).Warn("temporarily unable to lock item")
+		} else {
+			log(ctx, err).Error("unable to lock item")
+		}
 		// We don't have the memcache lock, but we still have the local lock,
 		// which mitigates some of the concurrency.
 		// Proceed with the setter, to make sure threads get unlocked.
@@ -158,3 +167,10 @@ func (s *store) broadcast(ctx context.Context, key string) {
 
 	s.lockMap.Release(key)
 }
+
+func isTemporaryError(err error) bool {
+	if netErr, ok := err.(net.Error); ok {
+		return netErr.Temporary()
+	}
+	return false
+}