Merge pull request #434 from fkautz/pr_out_adding_better_error_support_to_api

pkg/api/api_bucket_handlers.go

@@ -20,6 +20,7 @@ import (
 	"net/http"
 
 	"github.com/gorilla/mux"
+	"github.com/minio-io/iodine"
 	"github.com/minio-io/minio/pkg/drivers"
 	"github.com/minio-io/minio/pkg/utils/log"
 )
@@ -69,10 +70,6 @@ func (server *minioAPI) listObjectsHandler(w http.ResponseWriter, req *http.Requ
 		{
 			writeErrorResponse(w, req, NoSuchBucket, acceptsContentType, req.URL.Path)
 		}
-	case drivers.ImplementationError:
-		{
-			writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
-		}
 	case drivers.BucketNameInvalid:
 		{
 			writeErrorResponse(w, req, InvalidBucketName, acceptsContentType, req.URL.Path)
@@ -83,7 +80,7 @@ func (server *minioAPI) listObjectsHandler(w http.ResponseWriter, req *http.Requ
 		}
 	default:
 		{
-			log.Error.Println(err)
+			log.Error.Println(iodine.New(err, nil))
 			writeErrorResponse(w, req, InternalError, acceptsContentType, req.URL.Path)
 		}
 	}

pkg/api/api_router.go

@@ -21,6 +21,7 @@ import (
 	"net/http"
 
 	router "github.com/gorilla/mux"
+	"github.com/minio-io/iodine"
 	"github.com/minio-io/minio/pkg/api/config"
 	"github.com/minio-io/minio/pkg/drivers"
 )
@@ -83,7 +84,7 @@ func HTTPHandler(domain string, driver drivers.Driver) http.Handler {
 
 	var conf = config.Config{}
 	if err := conf.SetupConfig(); err != nil {
-		log.Fatal(err)
+		log.Fatal(iodine.New(err, map[string]string{"domain": domain}))
 	}
 
 	return validateHandler(conf, ignoreResourcesHandler(mux))

pkg/api/api_test.go

@@ -893,6 +893,66 @@ func (s *MySuite) TestPartialContent(c *C) {
 	c.Assert(string(partialObject), Equals, "wo")
 }
 
+func (s *MySuite) TestListObjectsHandlerErrors(c *C) {
+	switch driver := s.Driver.(type) {
+	case *mocks.Driver:
+		{
+			driver.AssertExpectations(c)
+		}
+	default:
+		{
+			// We mock failures here to test
+			return
+		}
+	}
+	driver := s.Driver
+	typedDriver := s.MockDriver
+
+	httpHandler := api.HTTPHandler("", driver)
+	testServer := httptest.NewServer(httpHandler)
+	defer testServer.Close()
+	client := http.Client{}
+
+	typedDriver.On("ListObjects", "foo", mock.Anything).Return(make([]drivers.ObjectMetadata, 0), drivers.BucketResourcesMetadata{}, drivers.BucketNotFound{}).Once()
+
+	request, err := http.NewRequest("GET", testServer.URL+"/foo", bytes.NewBufferString(""))
+	c.Assert(err, IsNil)
+	response, err := client.Do(request)
+	verifyError(c, response, "NoSuchBucket", "The specified bucket does not exist.", http.StatusNotFound)
+
+	typedDriver.On("ListObjects", "foo", mock.Anything).Return(make([]drivers.ObjectMetadata, 0), drivers.BucketResourcesMetadata{}, drivers.BucketNameInvalid{}).Once()
+	request, err = http.NewRequest("GET", testServer.URL+"/foo", bytes.NewBufferString(""))
+	c.Assert(err, IsNil)
+	response, err = client.Do(request)
+	c.Assert(err, IsNil)
+	verifyError(c, response, "InvalidBucketName", "The specified bucket is not valid.", http.StatusBadRequest)
+
+	typedDriver.On("ListObjects", "foo", mock.Anything).Return(make([]drivers.ObjectMetadata, 0), drivers.BucketResourcesMetadata{}, drivers.ObjectNameInvalid{}).Once()
+	request, err = http.NewRequest("GET", testServer.URL+"/foo", bytes.NewBufferString(""))
+	c.Assert(err, IsNil)
+	response, err = client.Do(request)
+	c.Assert(err, IsNil)
+	verifyError(c, response, "NoSuchKey", "The specified key does not exist.", http.StatusNotFound)
+
+	typedDriver.On("ListObjects", "foo", mock.Anything).Return(make([]drivers.ObjectMetadata, 0), drivers.BucketResourcesMetadata{}, drivers.BackendCorrupted{}).Once()
+	request, err = http.NewRequest("GET", testServer.URL+"/foo", bytes.NewBufferString(""))
+	c.Assert(err, IsNil)
+	response, err = client.Do(request)
+	c.Assert(err, IsNil)
+	verifyError(c, response, "InternalError", "We encountered an internal error, please try again.", http.StatusInternalServerError)
+}
+
+func verifyError(c *C, response *http.Response, code, description string, statusCode int) {
+	data, err := ioutil.ReadAll(response.Body)
+	c.Assert(err, IsNil)
+	errorResponse := api.ErrorResponse{}
+	err = xml.Unmarshal(data, &errorResponse)
+	c.Assert(err, IsNil)
+	c.Assert(errorResponse.Code, Equals, code)
+	c.Assert(errorResponse.Message, Equals, description)
+	c.Assert(response.StatusCode, Equals, statusCode)
+}
+
 func startMockDriver() *mocks.Driver {
 	return &mocks.Driver{
 		ObjectWriterData: make(map[string][]byte),

pkg/api/config/config.go

@@ -18,6 +18,7 @@ package config
 
 import (
 	"encoding/json"
+	"github.com/minio-io/iodine"
 	"io"
 	"os"
 	"os/user"
@@ -44,12 +45,12 @@ type User struct {
 func (c *Config) SetupConfig() error {
 	u, err := user.Current()
 	if err != nil {
-		return err
+		return iodine.New(err, nil)
 	}
 
 	confPath := path.Join(u.HomeDir, ".minio")
 	if err := os.MkdirAll(confPath, os.ModeDir); err != nil {
-		return err
+		return iodine.New(err, nil)
 	}
 
 	c.ConfigPath = confPath
@@ -57,7 +58,7 @@ func (c *Config) SetupConfig() error {
 	if _, err := os.Stat(c.ConfigFile); os.IsNotExist(err) {
 		_, err = os.Create(c.ConfigFile)
 		if err != nil {
-			return err
+			return iodine.New(err, nil)
 		}
 	}
 
@@ -113,7 +114,7 @@ func (c *Config) WriteConfig() error {
 	file, err = os.OpenFile(c.ConfigFile, os.O_WRONLY, 0666)
 	defer file.Close()
 	if err != nil {
-		return err
+		return iodine.New(err, nil)
 	}
 
 	encoder := json.NewEncoder(file)
@@ -132,7 +133,7 @@ func (c *Config) ReadConfig() error {
 	file, err = os.OpenFile(c.ConfigFile, os.O_RDONLY, 0666)
 	defer file.Close()
 	if err != nil {
-		return err
+		return iodine.New(err, nil)
 	}
 
 	users := make(map[string]User)
@@ -145,6 +146,6 @@ func (c *Config) ReadConfig() error {
 		c.Users = users
 		return nil
 	default:
-		return err
+		return iodine.New(err, nil)
 	}
 }

pkg/api/web/web.go

@@ -19,13 +19,14 @@ package web
 import (
 	"bytes"
 	"encoding/json"
-	"log"
 	"net/http"
 	"path"
 
 	"github.com/gorilla/mux"
+	"github.com/minio-io/iodine"
 	"github.com/minio-io/minio/pkg/api/config"
 	"github.com/minio-io/minio/pkg/utils/crypto/keys"
+	"github.com/minio-io/minio/pkg/utils/log"
 )
 
 const (
@@ -48,7 +49,7 @@ func HTTPHandler() http.Handler {
 	var api = webAPI{}
 
 	if err := api.conf.SetupConfig(); err != nil {
-		log.Fatal(err)
+		log.Fatal(iodine.New(err, nil))
 	}
 
 	api.webPath = path.Join(api.conf.GetConfigPath(), defaultWeb)
@@ -79,6 +80,7 @@ func (web *webAPI) accessHandler(w http.ResponseWriter, req *http.Request) {
 
 	err = web.conf.ReadConfig()
 	if err != nil {
+		log.Error.Println(iodine.New(err, nil))
 		w.WriteHeader(http.StatusInternalServerError)
 		w.Write([]byte(err.Error()))
 		return
@@ -94,6 +96,7 @@ func (web *webAPI) accessHandler(w http.ResponseWriter, req *http.Request) {
 
 	accesskey, err = keys.GenerateRandomAlphaNumeric(keys.MinioAccessID)
 	if err != nil {
+		log.Error.Println(iodine.New(err, nil))
 		w.WriteHeader(http.StatusInternalServerError)
 		w.Write([]byte(err.Error()))
 		return
@@ -102,6 +105,7 @@ func (web *webAPI) accessHandler(w http.ResponseWriter, req *http.Request) {
 
 	secretkey, err = keys.GenerateRandomBase64(keys.MinioSecretID)
 	if err != nil {
+		log.Error.Println(iodine.New(err, nil))
 		w.WriteHeader(http.StatusInternalServerError)
 		w.Write([]byte(err.Error()))
 		return
@@ -111,6 +115,7 @@ func (web *webAPI) accessHandler(w http.ResponseWriter, req *http.Request) {
 	web.conf.AddUser(user)
 	err = web.conf.WriteConfig()
 	if err != nil {
+		log.Error.Println(iodine.New(err, nil))
 		w.WriteHeader(http.StatusInternalServerError)
 		w.Write([]byte(err.Error()))
 		return
@@ -118,6 +123,7 @@ func (web *webAPI) accessHandler(w http.ResponseWriter, req *http.Request) {
 
 	err = web.conf.ReadConfig()
 	if err != nil {
+		log.Error.Println(iodine.New(err, nil))
 		w.WriteHeader(http.StatusInternalServerError)
 		w.Write([]byte(err.Error()))
 		return