7492: cleanup error checking in keepproxy

diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index a018eb40f787d963ce571b50f6f87655882bdf6d..2f809b32560b7e6072fd6242963e18b2ab9430df 100644 (file)
--- a/sdk/go/keepclient/keepclient.go
+++ b/sdk/go/keepclient/keepclient.go
@@ -169,10 +169,14 @@ 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)
+
+       numServers := len(serversToTry)
+       count404 := 0
+
        var retryList []string
 
        for tries_remaining > 0 {
@@ -231,7 +235,7 @@ func (kc *KeepClient) getOrHead(method string, locator string) (io.ReadCloser, i
        log.Printf("DEBUG: %s %s failed: %v", method, locator, errs)
 
        var err error
-       if count404 == len(kc.getSortedRoots(locator)) {
+       if count404 == numServers {
                err = BlockNotFound
        } else {
                err = &ErrNotFound{multipleResponseError{

diff --git a/sdk/go/keepclient/keepclient_test.go b/sdk/go/keepclient/keepclient_test.go
index ddfb20ec75a4485e3753779851ae91bcd93a0150..4acaf70fe46a8a28a6d6614e0b7e7b145306576d 100644 (file)
--- a/sdk/go/keepclient/keepclient_test.go
+++ b/sdk/go/keepclient/keepclient_test.go
@@ -559,7 +559,6 @@ func (s *StandaloneSuite) TestGetFail(c *C) {
        r, n, url2, err := kc.Get(hash)
        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)

diff --git a/services/keepproxy/keepproxy.go b/services/keepproxy/keepproxy.go
index 3cf1e28ba8ec16eb0a08b83b0b6e07af8dfcc1d5..d3dbeaf89e420d4546d0fedab643d9b15e9849c9 100644 (file)
--- a/services/keepproxy/keepproxy.go
+++ b/services/keepproxy/keepproxy.go
@@ -376,7 +376,7 @@ func (this GetBlockHandler) ServeHTTP(resp http.ResponseWriter, req *http.Reques
                        }
                }
        case keepclient.Error:
-               if respErr.Error() == "Block not found" {
+               if respErr == keepclient.BlockNotFound {
                        status = http.StatusNotFound
                } else if respErr.Temporary() {
                        status = http.StatusBadGateway

diff --git a/services/keepproxy/keepproxy_test.go b/services/keepproxy/keepproxy_test.go
index 17caefbb216f5a187171feb667dc23ed4b499fe2..e0d4a79c4c9e16d7b095ea90b8c475df3c75781d 100644 (file)
--- a/services/keepproxy/keepproxy_test.go
+++ b/services/keepproxy/keepproxy_test.go
@@ -172,18 +172,14 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
 
        {
                _, _, err := kc.Ask(hash)
-               errNotFound, _ := err.(keepclient.ErrNotFound)
-               c.Check(errNotFound, NotNil)
-               c.Check(strings.Contains(err.Error(), "Block not found"), Equals, true)
+               c.Check(err, Equals, keepclient.BlockNotFound)
                log.Print("Finished Ask (expected BlockNotFound)")
        }
 
        {
                reader, _, _, err := kc.Get(hash)
                c.Check(reader, Equals, nil)
-               errNotFound, _ := err.(keepclient.ErrNotFound)
-               c.Check(errNotFound, NotNil)
-               c.Check(strings.Contains(err.Error(), "Block not found"), Equals, true)
+               c.Check(err, Equals, keepclient.BlockNotFound)
                log.Print("Finished Get (expected BlockNotFound)")
        }
 
@@ -489,3 +485,36 @@ func (s *ServerRequiredSuite) TestGetIndex(c *C) {
        _, err = kc.GetIndex("proxy", "xyz")
        c.Assert((err != nil), Equals, true)
 }
+
+func (s *ServerRequiredSuite) TestPutAskGetInvalidToken(c *C) {
+       kc := runProxy(c, []string{"keepproxy"}, 28852, false)
+       waitForListener()
+       defer closeListener()
+
+       // Put a test block
+       hash, rep, err := kc.PutB([]byte("foo"))
+       c.Check(err, Equals, nil)
+       c.Check(rep, Equals, 2)
+
+       for _, token := range []string{
+               "nosuchtoken",
+               "2ym314ysp27sk7h943q6vtc378srb06se3pq6ghurylyf3pdmx", // expired
+       } {
+               // Change token to given bad token
+               kc.Arvados.ApiToken = token
+
+               // Ask should result in error
+               _, _, err = kc.Ask(hash)
+               c.Check(err, NotNil)
+               errNotFound, _ := err.(keepclient.ErrNotFound)
+               c.Check(errNotFound.Temporary(), Equals, false)
+               c.Assert(strings.Contains(err.Error(), "HTTP 403"), Equals, true)
+
+               // Get should result in error
+               _, _, _, err = kc.Get(hash)
+               c.Check(err, NotNil)
+               errNotFound, _ = err.(keepclient.ErrNotFound)
+               c.Check(errNotFound.Temporary(), Equals, false)
+               c.Assert(strings.Contains(err.Error(), "HTTP 403 \"Missing or invalid Authorization header\""), Equals, true)
+       }
+}