7491: Fix error handling/reporting in keepclient/GET

[arvados.git] / sdk / go / keepclient / keepclient.go

diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index a7522eb2e6b4a2b4deec5bd019cd9658fb9b9e4b..8b7cf419ead2f56c7c124799eadd0479dc69eac6 100644 (file)
--- a/sdk/go/keepclient/keepclient.go
+++ b/sdk/go/keepclient/keepclient.go
@@ -2,6 +2,7 @@
 package keepclient
 
 import (
+       "bytes"
        "crypto/md5"
        "crypto/tls"
        "errors"
@@ -28,9 +29,12 @@ var OversizeBlockError = errors.New("Exceeded maximum block size (" + strconv.It
 var MissingArvadosApiHost = errors.New("Missing required environment variable ARVADOS_API_HOST")
 var MissingArvadosApiToken = errors.New("Missing required environment variable ARVADOS_API_TOKEN")
 var InvalidLocatorError = errors.New("Invalid locator")
-var NoSuchKeepServer = errors.New("No keep server matching the given UUID is found")
-var GetIndexError = errors.New("Error getting index")
-var IncompleteIndexError = errors.New("Got incomplete index")
+
+// ErrNoSuchKeepServer is returned when GetIndex is invoked with a UUID with no matching keep server
+var ErrNoSuchKeepServer = errors.New("No keep server matching the given UUID is found")
+
+// ErrIncompleteIndex is returned when the Index response does not end with a new empty line
+var ErrIncompleteIndex = errors.New("Got incomplete index")
 
 const X_Keep_Desired_Replicas = "X-Keep-Desired-Replicas"
 const X_Keep_Replicas_Stored = "X-Keep-Replicas-Stored"
@@ -136,21 +140,19 @@ func (kc *KeepClient) Get(locator string) (io.ReadCloser, int64, string, error)
                url := host + "/" + locator
                req, err := http.NewRequest("GET", url, nil)
                if err != nil {
+                       errs = append(errs, fmt.Sprintf("%s: %v", url, err))
                        continue
                }
                req.Header.Add("Authorization", fmt.Sprintf("OAuth2 %s", kc.Arvados.ApiToken))
                resp, err := kc.Client.Do(req)
-               if err != nil || resp.StatusCode != http.StatusOK {
-                       if resp != nil {
-                               var respbody []byte
-                               if resp.Body != nil {
-                                       respbody, _ = ioutil.ReadAll(&io.LimitedReader{resp.Body, 4096})
-                               }
-                               errs = append(errs, fmt.Sprintf("%s: %d %s",
-                                       url, resp.StatusCode, strings.TrimSpace(string(respbody))))
-                       } else {
-                               errs = append(errs, fmt.Sprintf("%s: %v", url, err))
-                       }
+               if err != nil {
+                       errs = append(errs, fmt.Sprintf("%s: %v", url, err))
+                       continue
+               } else if resp.StatusCode != http.StatusOK {
+                       respbody, _ := ioutil.ReadAll(&io.LimitedReader{resp.Body, 4096})
+                       resp.Body.Close()
+                       errs = append(errs, fmt.Sprintf("%s: HTTP %d %q",
+                               url, resp.StatusCode, bytes.TrimSpace(respbody)))
                        continue
                }
                return HashCheckingReader{
@@ -187,14 +189,15 @@ func (kc *KeepClient) Ask(locator string) (int64, string, error) {
 
 // GetIndex retrieves a list of blocks stored on the given server whose hashes
 // begin with the given prefix. The returned reader will return an error (other
-// than EOF) if the complete index cannot be retrieved. This should only be
-// expected to return useful results if the client is using a "data manager token"
+// than EOF) if the complete index cannot be retrieved.
+//
+// This is meant to be used only by system components and admin tools.
+// It will return an error unless the client is using a "data manager token"
 // recognized by the Keep services.
 func (kc *KeepClient) GetIndex(keepServiceUUID, prefix string) (io.Reader, error) {
        url := kc.LocalRoots()[keepServiceUUID]
        if url == "" {
-               log.Printf("No such keep server found: %v", keepServiceUUID)
-               return nil, NoSuchKeepServer
+               return nil, ErrNoSuchKeepServer
        }
 
        url += "/index"
@@ -204,34 +207,36 @@ func (kc *KeepClient) GetIndex(keepServiceUUID, prefix string) (io.Reader, error
 
        req, err := http.NewRequest("GET", url, nil)
        if err != nil {
-               log.Printf("GET index error: %v", err)
-               return nil, GetIndexError
+               return nil, err
        }
 
        req.Header.Add("Authorization", fmt.Sprintf("OAuth2 %s", kc.Arvados.ApiToken))
        resp, err := kc.Client.Do(req)
-       if err != nil || resp.StatusCode != http.StatusOK {
-               log.Printf("GET index error: %v; status code: %v", err, resp.StatusCode)
-               return nil, GetIndexError
+       if err != nil {
+               return nil, err
        }
 
-       var respbody []byte
-       if resp.Body != nil {
-               respbody, err = ioutil.ReadAll(resp.Body)
-               if err != nil {
-                       log.Printf("GET index error: %v", err)
-                       return nil, GetIndexError
-               }
+       defer resp.Body.Close()
 
-               // Got index; verify that it is complete
-               if !strings.HasSuffix(string(respbody), "\n\n") {
-                       log.Printf("Got incomplete index for %v", url)
-                       return nil, IncompleteIndexError
-               }
+       if resp.StatusCode != http.StatusOK {
+               return nil, fmt.Errorf("Got http status code: %d", resp.StatusCode)
+       }
+
+       var respBody []byte
+       respBody, err = ioutil.ReadAll(resp.Body)
+       if err != nil {
+               return nil, err
+       }
+
+       // Got index; verify that it is complete
+       // The response should be "\n" if no locators matched the prefix
+       // Else, it should be a list of locators followed by a blank line
+       if !bytes.Equal(respBody, []byte("\n")) && !bytes.HasSuffix(respBody, []byte("\n\n")) {
+               return nil, ErrIncompleteIndex
        }
 
-       // Got complete index or "" if no locators matching prefix
-       return strings.NewReader(string(respbody)), nil
+       // Got complete index; strip the trailing newline and send
+       return bytes.NewReader(respBody[0 : len(respBody)-1]), nil
 }
 
 // LocalRoots() returns the map of local (i.e., disk and proxy) Keep