From 9f0025d371481fc71a07d93d654c1d73d517a5a0 Mon Sep 17 00:00:00 2001
From: Keunwoo Lee <keunwoo@flux.io>
Date: Thu, 9 Jul 2015 16:55:58 -0700
Subject: Make errors more distinguishable

Prior to this commit, this library raised errors either mostly using
errors.New() or directly passing through error values from underlying
libraries.  This made it difficult for clients to respond correctly to
the errors that were returned.

This becomes particularly problematic when securecookie is used together
with gorilla/sessions.  From an operations standpoint, you often want to
log different errors when the client simply provides an invalid auth
cookie, versus an I/O error fetching data from the session store.  The
former probably indicates an expired timestamp or similar client error;
the latter indicates a possible failure in a backend database.

This commit introduces a public Error interface, which is now returned
consistently on all errors, and can be used to distinguish between
implementation errors (IsUsage() and IsInternal()) and failed validation
of user input (IsDecode()).

See also discussion on pull requests #9 and #24:
https://github.com/gorilla/securecookie/pull/9
https://github.com/gorilla/securecookie/pull/24

Some interface comments on other API functions have been clarified and
updated to harmonize with the new error interfaces.
---
 securecookie_test.go | 43 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

(limited to 'securecookie_test.go')

diff --git a/securecookie_test.go b/securecookie_test.go
index e482397..5778000 100644
--- a/securecookie_test.go
+++ b/securecookie_test.go
@@ -52,6 +52,21 @@ func TestSecureCookie(t *testing.T) {
 		if err3 == nil {
 			t.Fatalf("Expected failure decoding.")
 		}
+		err4, ok := err3.(Error)
+		if !ok {
+			t.Fatalf("Expected error to implement Error, got: %#v", err3)
+		}
+		if !err4.IsDecode() {
+			t.Fatalf("Expected DecodeError, got: %#v", err4)
+		}
+
+		// Test other error type flags.
+		if err4.IsUsage() {
+			t.Fatalf("Expected IsUsage() == false, got: %#v", err4)
+		}
+		if err4.IsInternal() {
+			t.Fatalf("Expected IsInternal() == false, got: %#v", err4)
+		}
 	}
 }
 
@@ -69,9 +84,18 @@ func TestDecodeInvalid(t *testing.T) {
 	s := New([]byte("12345"), nil)
 	var dst string
 	for i, v := range invalidCookies {
-		err := s.Decode("name", base64.StdEncoding.EncodeToString([]byte(v)), &dst)
-		if err == nil {
-			t.Fatalf("%d: expected failure decoding", i)
+		for _, enc := range []*base64.Encoding{
+			base64.StdEncoding,
+			base64.URLEncoding,
+		} {
+			err := s.Decode("name", enc.EncodeToString([]byte(v)), &dst)
+			if err == nil {
+				t.Fatalf("%d: expected failure decoding", i)
+			}
+			err2, ok := err.(Error)
+			if !ok || !err2.IsDecode() {
+				t.Fatalf("%d: Expected IsDecode(), got: %#v", i, err)
+			}
 		}
 	}
 }
@@ -174,6 +198,16 @@ func TestMultiError(t *testing.T) {
 		if strings.Index(err.Error(), "hash key is not set") == -1 {
 			t.Errorf("Expected missing hash key error, got %s.", err.Error())
 		}
+		ourErr, ok := err.(Error)
+		if !ok || !ourErr.IsUsage() {
+			t.Fatalf("Expected error to be a usage error; got %#v", err)
+		}
+		if ourErr.IsDecode() {
+			t.Errorf("Expected error NOT to be a decode error; got %#v", ourErr)
+		}
+		if ourErr.IsInternal() {
+			t.Errorf("Expected error NOT to be an internal error; got %#v", ourErr)
+		}
 	}
 }
 
@@ -198,6 +232,9 @@ func TestMissingKey(t *testing.T) {
 	if err != errHashKeyNotSet {
 		t.Fatalf("Expected %#v, got %#v", errHashKeyNotSet, err)
 	}
+	if err2, ok := err.(Error); !ok || !err2.IsUsage() {
+		t.Errorf("Expected missing hash key to be IsUsage(); was %#v", err)
+	}
 }
 
 // ----------------------------------------------------------------------------
-- 
cgit v1.2.3