Merge branch 'master' into 7492-keepproxy-upstream-errors

diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index 67c304deaf3ae54b2668cb8c2f2856e909da8c5a..a018eb40f787d963ce571b50f6f87655882bdf6d 100644 (file)
--- a/sdk/go/keepclient/keepclient.go
+++ b/sdk/go/keepclient/keepclient.go
@@ -22,7 +22,33 @@ import (
 // A Keep "block" is 64MB.
 const BLOCKSIZE = 64 * 1024 * 1024
 
-var BlockNotFound = errors.New("Block not found")
+// Error interface with an error and boolean indicating whether the error is temporary
+type Error interface {
+       error
+       Temporary() bool
+}
+
+// multipleResponseError is of type Error
+type multipleResponseError struct {
+       error
+       isTemp bool
+}
+
+func (e *multipleResponseError) Temporary() bool {
+       return e.isTemp
+}
+
+// BlockNotFound is a multipleResponseError where isTemp is false
+var BlockNotFound = &ErrNotFound{multipleResponseError{
+       error:  errors.New("Block not found"),
+       isTemp: false,
+}}
+
+// ErrNotFound is a multipleResponseError where isTemp can be true or false
+type ErrNotFound struct {
+       multipleResponseError
+}
+
 var InsufficientReplicasError = errors.New("Could not write sufficient replicas")
 var OversizeBlockError = errors.New("Exceeded maximum block size (" + strconv.Itoa(BLOCKSIZE) + ")")
 var MissingArvadosApiHost = errors.New("Missing required environment variable ARVADOS_API_HOST")
@@ -143,6 +169,7 @@ func (kc *KeepClient) PutR(r io.Reader) (locator string, replicas int, err error
 
 func (kc *KeepClient) getOrHead(method string, locator string) (io.ReadCloser, int64, string, error) {
        var errs []string
+       var count404 int
 
        tries_remaining := 1 + kc.Retries
        serversToTry := kc.getSortedRoots(locator)
@@ -181,6 +208,8 @@ func (kc *KeepClient) getOrHead(method string, locator string) (io.ReadCloser, i
                                        // server side failure, transient
                                        // error, can try again.
                                        retryList = append(retryList, host)
+                               } else if resp.StatusCode == 404 {
+                                       count404++
                                }
                        } else {
                                // Success.
@@ -201,7 +230,16 @@ func (kc *KeepClient) getOrHead(method string, locator string) (io.ReadCloser, i
        }
        log.Printf("DEBUG: %s %s failed: %v", method, locator, errs)
 
-       return nil, 0, "", BlockNotFound
+       var err error
+       if count404 == len(kc.getSortedRoots(locator)) {
+               err = BlockNotFound
+       } else {
+               err = &ErrNotFound{multipleResponseError{
+                       error:  fmt.Errorf("%s %s failed: %v", method, locator, errs),
+                       isTemp: len(serversToTry) > 0,
+               }}
+       }
+       return nil, 0, "", err
 }
 
 // Get() retrieves a block, given a locator. Returns a reader, the

diff --git a/sdk/go/keepclient/keepclient_test.go b/sdk/go/keepclient/keepclient_test.go
index c03ba90736868d15cdba09d0638e160e60d57998..ddfb20ec75a4485e3753779851ae91bcd93a0150 100644 (file)
--- a/sdk/go/keepclient/keepclient_test.go
+++ b/sdk/go/keepclient/keepclient_test.go
@@ -14,6 +14,7 @@ import (
        "net"
        "net/http"
        "os"
+       "strings"
        "testing"
 )
 
@@ -556,7 +557,9 @@ func (s *StandaloneSuite) TestGetFail(c *C) {
        kc.Retries = 0
 
        r, n, url2, err := kc.Get(hash)
-       c.Check(err, Equals, BlockNotFound)
+       errNotFound, _ := err.(ErrNotFound)
+       c.Check(errNotFound, NotNil)
+       c.Check(strings.Contains(err.Error(), "use of closed network connection"), Equals, true)
        c.Check(n, Equals, int64(0))
        c.Check(url2, Equals, "")
        c.Check(r, Equals, nil)
@@ -601,7 +604,9 @@ func (s *StandaloneSuite) TestGetNetError(c *C) {
        kc.SetServiceRoots(map[string]string{"x": "http://localhost:62222"}, nil, nil)
 
        r, n, url2, err := kc.Get(hash)
-       c.Check(err, Equals, BlockNotFound)
+       errNotFound, _ := err.(ErrNotFound)
+       c.Check(errNotFound, NotNil)
+       c.Check(strings.Contains(err.Error(), "connection refused"), Equals, true)
        c.Check(n, Equals, int64(0))
        c.Check(url2, Equals, "")
        c.Check(r, Equals, nil)

diff --git a/services/keepproxy/keepproxy.go b/services/keepproxy/keepproxy.go
index 7900096caf0ad9e80dfe13fde5970dbd0876db27..3cf1e28ba8ec16eb0a08b83b0b6e07af8dfcc1d5 100644 (file)
--- a/services/keepproxy/keepproxy.go
+++ b/services/keepproxy/keepproxy.go
@@ -362,7 +362,7 @@ func (this GetBlockHandler) ServeHTTP(resp http.ResponseWriter, req *http.Reques
                log.Println("Warning:", GetRemoteAddress(req), req.Method, proxiedURI, "Content-Length not provided")
        }
 
-       switch err {
+       switch respErr := err.(type) {
        case nil:
                status = http.StatusOK
                resp.Header().Set("Content-Length", fmt.Sprint(expectLength))
@@ -375,10 +375,16 @@ func (this GetBlockHandler) ServeHTTP(resp http.ResponseWriter, req *http.Reques
                                err = ContentLengthMismatch
                        }
                }
-       case keepclient.BlockNotFound:
-               status = http.StatusNotFound
+       case keepclient.Error:
+               if respErr.Error() == "Block not found" {
+                       status = http.StatusNotFound
+               } else if respErr.Temporary() {
+                       status = http.StatusBadGateway
+               } else {
+                       status = 422
+               }
        default:
-               status = http.StatusBadGateway
+               status = http.StatusInternalServerError
        }
 }
 

diff --git a/services/keepproxy/keepproxy_test.go b/services/keepproxy/keepproxy_test.go
index 7643e4b0fa2225492caae1dd0aff3428505bd86d..17caefbb216f5a187171feb667dc23ed4b499fe2 100644 (file)
--- a/services/keepproxy/keepproxy_test.go
+++ b/services/keepproxy/keepproxy_test.go
@@ -172,14 +172,18 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
 
        {
                _, _, err := kc.Ask(hash)
-               c.Check(err, Equals, keepclient.BlockNotFound)
+               errNotFound, _ := err.(keepclient.ErrNotFound)
+               c.Check(errNotFound, NotNil)
+               c.Check(strings.Contains(err.Error(), "Block not found"), Equals, true)
                log.Print("Finished Ask (expected BlockNotFound)")
        }
 
        {
                reader, _, _, err := kc.Get(hash)
                c.Check(reader, Equals, nil)
-               c.Check(err, Equals, keepclient.BlockNotFound)
+               errNotFound, _ := err.(keepclient.ErrNotFound)
+               c.Check(errNotFound, NotNil)
+               c.Check(strings.Contains(err.Error(), "Block not found"), Equals, true)
                log.Print("Finished Get (expected BlockNotFound)")
        }
 
@@ -251,7 +255,9 @@ func (s *ServerRequiredSuite) TestPutAskGetForbidden(c *C) {
 
        {
                _, _, err := kc.Ask(hash)
-               c.Check(err, Equals, keepclient.BlockNotFound)
+               errNotFound, _ := err.(keepclient.ErrNotFound)
+               c.Check(errNotFound, NotNil)
+               c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
                log.Print("Ask 1")
        }
 
@@ -265,14 +271,18 @@ func (s *ServerRequiredSuite) TestPutAskGetForbidden(c *C) {
 
        {
                blocklen, _, err := kc.Ask(hash)
-               c.Assert(err, Equals, keepclient.BlockNotFound)
+               errNotFound, _ := err.(keepclient.ErrNotFound)
+               c.Check(errNotFound, NotNil)
+               c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
                c.Check(blocklen, Equals, int64(0))
                log.Print("Ask 2")
        }
 
        {
                _, blocklen, _, err := kc.Get(hash)
-               c.Assert(err, Equals, keepclient.BlockNotFound)
+               errNotFound, _ := err.(keepclient.ErrNotFound)
+               c.Check(errNotFound, NotNil)
+               c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
                c.Check(blocklen, Equals, int64(0))
                log.Print("Get")
        }
@@ -291,7 +301,9 @@ func (s *ServerRequiredSuite) TestGetDisabled(c *C) {
 
        {
                _, _, err := kc.Ask(hash)
-               c.Check(err, Equals, keepclient.BlockNotFound)
+               errNotFound, _ := err.(keepclient.ErrNotFound)
+               c.Check(errNotFound, NotNil)
+               c.Assert(strings.Contains(err.Error(), "HTTP 400"), Equals, true)
                log.Print("Ask 1")
        }
 
@@ -305,14 +317,18 @@ func (s *ServerRequiredSuite) TestGetDisabled(c *C) {
 
        {
                blocklen, _, err := kc.Ask(hash)
-               c.Assert(err, Equals, keepclient.BlockNotFound)
+               errNotFound, _ := err.(keepclient.ErrNotFound)
+               c.Check(errNotFound, NotNil)
+               c.Assert(strings.Contains(err.Error(), "HTTP 400"), Equals, true)
                c.Check(blocklen, Equals, int64(0))
                log.Print("Ask 2")
        }
 
        {
                _, blocklen, _, err := kc.Get(hash)
-               c.Assert(err, Equals, keepclient.BlockNotFound)
+               errNotFound, _ := err.(keepclient.ErrNotFound)
+               c.Check(errNotFound, NotNil)
+               c.Assert(strings.Contains(err.Error(), "HTTP 400"), Equals, true)
                c.Check(blocklen, Equals, int64(0))
                log.Print("Get")
        }

diff --git a/tools/keep-rsync/keep-rsync_test.go b/tools/keep-rsync/keep-rsync_test.go
index 6fbb535a03a3069249b1ab22fa6aaae85740998f..60bc90acff68f56f9015a611f6e83639d7676569 100644 (file)
--- a/tools/keep-rsync/keep-rsync_test.go
+++ b/tools/keep-rsync/keep-rsync_test.go
@@ -352,7 +352,7 @@ func (s *ServerRequiredSuite) TestErrorDuringRsync_ErrorGettingBlockFromSrc(c *C
        blobSigningKey = "thisisfakeblobsigningkey"
 
        err := performKeepRsync(kcSrc, kcDst, blobSigningKey, "")
-       c.Check(strings.HasSuffix(err.Error(), "Block not found"), Equals, true)
+       c.Check(strings.Contains(err.Error(), "HTTP 403 \"Forbidden\""), Equals, true)
 }
 
 // Test rsync with error during Put to src.