[ARVADOS] updated: 239093640609a2267eb49a90d2f96ffcb2a462fe

git at public.curoverse.com git at public.curoverse.com
Sat Apr 12 14:25:57 EDT 2014


Summary of changes:
 services/keep/keep.go      |   31 ++++++++++++++++---------------
 services/keep/keep_test.go |   34 ++++++++++++----------------------
 2 files changed, 28 insertions(+), 37 deletions(-)

       via  239093640609a2267eb49a90d2f96ffcb2a462fe (commit)
      from  0f5d86d25f1e66723bbfe1f912d892f8a66321b8 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 239093640609a2267eb49a90d2f96ffcb2a462fe
Author: Tim Pierce <twp at curoverse.com>
Date:   Sat Apr 12 14:24:49 2014 -0400

    Clean up error handling (refs #2292).

diff --git a/services/keep/keep.go b/services/keep/keep.go
index bd9360f..5113727 100644
--- a/services/keep/keep.go
+++ b/services/keep/keep.go
@@ -44,20 +44,21 @@ var KeepVolumes []string
 //
 type KeepError struct {
 	HTTPCode int
-	Err      error
+	ErrMsg   string
 }
 
-const (
-	ErrCollision = 400
-	ErrMD5Fail   = 401
-	ErrCorrupt   = 402
-	ErrNotFound  = 404
-	ErrOther     = 500
-	ErrFull      = 503
+var (
+	CollisionError = &KeepError{400, "Collision"}
+	MD5Error       = &KeepError{401, "MD5 Failure"}
+	CorruptError   = &KeepError{402, "Corruption"}
+	NotFoundError  = &KeepError{404, "Not Found"}
+	GenericError   = &KeepError{500, "Fail"}
+	FullError      = &KeepError{503, "Full"}
+	TooLongError   = &KeepError{504, "Too Long"}
 )
 
 func (e *KeepError) Error() string {
-	return fmt.Sprintf("Error %d: %s", e.HTTPCode, e.Err.Error())
+	return e.ErrMsg
 }
 
 // This error is returned by ReadAtMost if the available
@@ -242,7 +243,7 @@ func GetBlock(hash string) ([]byte, error) {
 			//
 			log.Printf("%s: checksum mismatch: %s (actual hash %s)\n",
 				vol, blockFilename, filehash)
-			return buf, &KeepError{ErrCorrupt, errors.New("Corrupt")}
+			return buf, CorruptError
 		}
 
 		// Success!
@@ -250,7 +251,7 @@ func GetBlock(hash string) ([]byte, error) {
 	}
 
 	log.Printf("%s: not found on any volumes, giving up\n", hash)
-	return buf, &KeepError{ErrNotFound, errors.New("not found: " + hash)}
+	return buf, NotFoundError
 }
 
 /* PutBlock(block, hash)
@@ -284,7 +285,7 @@ func PutBlock(block []byte, hash string) error {
 	blockhash := fmt.Sprintf("%x", md5.Sum(block))
 	if blockhash != hash {
 		log.Printf("%s: MD5 checksum %s did not match request", hash, blockhash)
-		return &KeepError{ErrMD5Fail, errors.New("MD5Fail")}
+		return MD5Error
 	}
 
 	// If we already have a block on disk under this identifier, return
@@ -296,7 +297,7 @@ func PutBlock(block []byte, hash string) error {
 		if bytes.Compare(block, oldblock) == 0 {
 			return nil
 		} else {
-			return &KeepError{ErrCollision, errors.New("Collision")}
+			return CollisionError
 		}
 	}
 
@@ -340,10 +341,10 @@ func PutBlock(block []byte, hash string) error {
 
 	if allFull {
 		log.Printf("all Keep volumes full")
-		return &KeepError{ErrFull, errors.New("Full")}
+		return FullError
 	} else {
 		log.Printf("all Keep volumes failed")
-		return &KeepError{ErrOther, errors.New("Fail")}
+		return GenericError
 	}
 }
 
diff --git a/services/keep/keep_test.go b/services/keep/keep_test.go
index c1f4e65..348445e 100644
--- a/services/keep/keep_test.go
+++ b/services/keep/keep_test.go
@@ -62,13 +62,8 @@ func TestGetBlockMissing(t *testing.T) {
 
 	// Check that GetBlock returns failure.
 	result, err := GetBlock(TEST_HASH)
-	if err == nil {
-		t.Errorf("GetBlock incorrectly returned success: ", result)
-	} else {
-		ke := err.(*KeepError)
-		if ke.HTTPCode != ErrNotFound {
-			t.Errorf("GetBlock: %v", ke)
-		}
+	if err != NotFoundError {
+		t.Errorf("Expected NotFoundError, got %v", result)
 	}
 }
 
@@ -88,8 +83,8 @@ func TestGetBlockCorrupt(t *testing.T) {
 
 	// Check that GetBlock returns failure.
 	result, err := GetBlock(TEST_HASH)
-	if err == nil {
-		t.Errorf("GetBlock incorrectly returned success: %s", result)
+	if err != CorruptError {
+		t.Errorf("Expected CorruptError, got %v", result)
 	}
 }
 
@@ -113,7 +108,7 @@ func TestPutBlockOK(t *testing.T) {
 
 	result, err := GetBlock(TEST_HASH)
 	if err != nil {
-		t.Fatalf("GetBlock: %s", err.Error())
+		t.Fatalf("GetBlock returned error: %v", err)
 	}
 	if string(result) != string(TEST_BLOCK) {
 		t.Error("PutBlock/GetBlock mismatch")
@@ -140,7 +135,7 @@ func TestPutBlockOneVol(t *testing.T) {
 
 	result, err := GetBlock(TEST_HASH)
 	if err != nil {
-		t.Fatalf("GetBlock: %s", err.Error())
+		t.Fatalf("GetBlock: %v", err)
 	}
 	if string(result) != string(TEST_BLOCK) {
 		t.Error("PutBlock/GetBlock mismatch")
@@ -161,19 +156,14 @@ func TestPutBlockMD5Fail(t *testing.T) {
 
 	// Check that PutBlock returns the expected error when the hash does
 	// not match the block.
-	if err := PutBlock(BAD_BLOCK, TEST_HASH); err == nil {
-		t.Error("PutBlock succeeded despite a block mismatch")
-	} else {
-		ke := err.(*KeepError)
-		if ke.HTTPCode != ErrMD5Fail {
-			t.Errorf("PutBlock returned the wrong error (%v)", ke)
-		}
+	if err := PutBlock(BAD_BLOCK, TEST_HASH); err != MD5Error {
+		t.Error("Expected MD5Error, got %v", err)
 	}
 
 	// Confirm that GetBlock fails to return anything.
-	if result, err := GetBlock(TEST_HASH); err == nil {
-		t.Errorf("GetBlock succeded after a corrupt block store, returned '%s'",
-			string(result))
+	if result, err := GetBlock(TEST_HASH); err != NotFoundError {
+		t.Errorf("GetBlock succeeded after a corrupt block store (result = %s, err = %v)",
+			string(result), err)
 	}
 }
 
@@ -220,7 +210,7 @@ func TestPutBlockCollision(t *testing.T) {
 
 	if err := PutBlock(b2, locator); err == nil {
 		t.Error("PutBlock did not report a collision")
-	} else if err.(*KeepError).HTTPCode != ErrCollision {
+	} else if err != CollisionError {
 		t.Errorf("PutBlock returned %v", err)
 	}
 }

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list