From 5865295f5d55cd8c95f9b2bfa3beef5e01051aad Mon Sep 17 00:00:00 2001 From: "Frederick F. Kautz IV" Date: Fri, 3 Apr 2015 18:06:30 -0700 Subject: [PATCH] Adding better error support to api --- pkg/api/api_bucket_handlers.go | 7 ++-- pkg/api/api_router.go | 3 +- pkg/api/api_test.go | 60 ++++++++++++++++++++++++++++++++++ pkg/api/config/config.go | 13 ++++---- pkg/api/web/web.go | 10 ++++-- 5 files changed, 79 insertions(+), 14 deletions(-) diff --git a/pkg/api/api_bucket_handlers.go b/pkg/api/api_bucket_handlers.go index 61b6ee423..a81874181 100644 --- a/pkg/api/api_bucket_handlers.go +++ b/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) } } diff --git a/pkg/api/api_router.go b/pkg/api/api_router.go index 4910eea2e..cca623892 100644 --- a/pkg/api/api_router.go +++ b/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)) diff --git a/pkg/api/api_test.go b/pkg/api/api_test.go index 92e04db56..2a04f9517 100644 --- a/pkg/api/api_test.go +++ b/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), diff --git a/pkg/api/config/config.go b/pkg/api/config/config.go index 6197e1440..8db6218ad 100644 --- a/pkg/api/config/config.go +++ b/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) } } diff --git a/pkg/api/web/web.go b/pkg/api/web/web.go index 4728d374a..2117fdb3e 100644 --- a/pkg/api/web/web.go +++ b/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