diff --git a/controller/api_test.go b/controller/api_test.go index 16be9fbc..c549700c 100644 --- a/controller/api_test.go +++ b/controller/api_test.go @@ -36,6 +36,13 @@ var g struct { client *client.Client } +func assertNameReuseFails(err error, name string) { + assert(err != nil, "recreating "+name+" with same name must fail") + fe, ok := err.(fission.Error) + assert(ok, "error must be a fission Error") + assert(fe.Code == fission.ErrorNameExists, "error must be a name exists error") +} + func TestFunctionApi(t *testing.T) { log.SetFormatter(&log.TextFormatter{DisableColors: true}) @@ -56,6 +63,9 @@ func TestFunctionApi(t *testing.T) { uid1 := m.Uid //log.Printf("Created function %v: %v", m.Name, m.Uid) + _, err = g.client.FunctionCreate(testFunc) + assertNameReuseFails(err, "function") + code, err := g.client.FunctionGetRaw(m) panicIf(err) assert(string(code) == testFunc.Code, "code from FunctionGetRaw must match created function") @@ -123,6 +133,9 @@ func TestHTTPTriggerApi(t *testing.T) { panicIf(err) defer g.client.HTTPTriggerDelete(m) + _, err = g.client.HTTPTriggerCreate(testTrigger) + assertNameReuseFails(err, "trigger") + tr, err := g.client.HTTPTriggerGet(m) panicIf(err) testTrigger.Metadata.Uid = m.Uid @@ -160,6 +173,9 @@ func TestEnvironmentApi(t *testing.T) { panicIf(err) defer g.client.EnvironmentDelete(m) + _, err = g.client.EnvironmentCreate(testEnv) + assertNameReuseFails(err, "environment") + tr, err := g.client.EnvironmentGet(m) panicIf(err) testEnv.Metadata.Uid = m.Uid @@ -205,6 +221,9 @@ func TestWatchApi(t *testing.T) { panicIf(err) defer g.client.WatchDelete(m) + _, err = g.client.WatchCreate(testWatch) + assertNameReuseFails(err, "watch") + w, err := g.client.WatchGet(m) panicIf(err) testWatch.Metadata.Uid = m.Uid diff --git a/controller/client/client.go b/controller/client/client.go index 3355f050..8f6fe077 100644 --- a/controller/client/client.go +++ b/controller/client/client.go @@ -80,19 +80,7 @@ func (c *Client) url(relativeUrl string) string { func (c *Client) handleResponse(resp *http.Response) ([]byte, error) { if resp.StatusCode != 200 { - var errCode int - switch resp.StatusCode { - case 403: - errCode = fission.ErrorNotAuthorized - case 404: - errCode = fission.ErrorNotFound - case 400: - errCode = fission.ErrorInvalidArgument - default: - errCode = fission.ErrorInternal - } - return nil, fission.MakeError(errCode, - fmt.Sprintf("HTTP error %v", resp.StatusCode)) + return nil, fission.MakeErrorFromHTTP(resp) } body, err := ioutil.ReadAll(resp.Body) return body, err diff --git a/controller/resourceStore.go b/controller/resourceStore.go index b43e97bd..f59ca500 100644 --- a/controller/resourceStore.go +++ b/controller/resourceStore.go @@ -25,6 +25,9 @@ import ( "github.com/coreos/etcd/client" "github.com/satori/go.uuid" "golang.org/x/net/context" + + "fmt" + "github.com/fission/fission" ) type ( @@ -93,7 +96,7 @@ func (rs *ResourceStore) create(r resource) error { _, err = rs.KeysAPI.Set(context.Background(), key, string(serialized), &client.SetOptions{PrevExist: client.PrevNoExist}) - return err + return handleEtcdError(err) } func (rs *ResourceStore) read(rkey string, res resource) error { @@ -105,7 +108,7 @@ func (rs *ResourceStore) read(rkey string, res resource) error { resp, err := rs.KeysAPI.Get(context.Background(), key, nil) if err != nil { - return err + return handleEtcdError(err) } return rs.serializer.deserialize([]byte(resp.Node.Value), res) } @@ -123,13 +126,13 @@ func (rs *ResourceStore) update(r resource) error { _, err = rs.KeysAPI.Set(context.Background(), key, string(serialized), &client.SetOptions{PrevExist: client.PrevExist}) - return err + return handleEtcdError(err) } func (rs *ResourceStore) delete(typename, rkey string) error { key := typename + "/" + rkey _, err := rs.KeysAPI.Delete(context.Background(), key, nil) // ignore response - return err + return handleEtcdError(err) } // getAll finds all entries under key. If none or found or key @@ -140,7 +143,7 @@ func (rs *ResourceStore) getAll(key string) ([]string, error) { if client.IsKeyNotFound(err) { return []string{}, nil } - return nil, err + return nil, handleEtcdError(err) } res := make([]string, 0, len(resp.Node.Nodes)) @@ -162,7 +165,7 @@ func (rs *ResourceStore) writeFile(parentKey string, contents []byte) (string, s resp, err := rs.KeysAPI.CreateInOrder(context.Background(), parentKey, uid, nil) if err != nil { _ = rs.FileStore.delete(uid) - return "", "", err + return "", "", handleEtcdError(err) } return resp.Node.Key, uid, nil @@ -172,7 +175,7 @@ func (rs *ResourceStore) readFile(key string, uid *string) ([]byte, error) { key = "file/" + key resp, err := rs.KeysAPI.Get(context.Background(), key, &client.GetOptions{Sort: true}) if err != nil { - return nil, err + return nil, handleEtcdError(err) } if uid == nil { @@ -194,14 +197,14 @@ func (rs *ResourceStore) readFile(key string, uid *string) ([]byte, error) { } contents, err := rs.FileStore.read(*uid) - return contents, err + return contents, handleEtcdError(err) } func (rs *ResourceStore) deleteFile(key string, uid string) error { key = "file/" + key resp, err := rs.KeysAPI.Get(context.Background(), key, &client.GetOptions{Sort: true}) if err != nil { - return err + return handleEtcdError(err) } var node *client.Node @@ -222,12 +225,12 @@ func (rs *ResourceStore) deleteFile(key string, uid string) error { _, err = rs.KeysAPI.Delete(context.Background(), node.Key, nil) if err != nil { - return err + return handleEtcdError(err) } if len(resp.Node.Nodes) == 1 { _, err = rs.KeysAPI.Delete(context.Background(), key, &client.DeleteOptions{Dir: true}) - return err + return handleEtcdError(err) } return nil } @@ -236,7 +239,7 @@ func (rs *ResourceStore) deleteAllFiles(key string) error { key = "file/" + key resp, err := rs.KeysAPI.Get(context.Background(), key, &client.GetOptions{Sort: true}) if err != nil { - return err + return handleEtcdError(err) } for _, u := range resp.Node.Nodes { err = rs.FileStore.delete(u.Value) @@ -246,9 +249,26 @@ func (rs *ResourceStore) deleteAllFiles(key string) error { _, err = rs.KeysAPI.Delete(context.Background(), u.Key, nil) if err != nil { - return err + return handleEtcdError(err) } } _, err = rs.KeysAPI.Delete(context.Background(), key, &client.DeleteOptions{Dir: true}) - return err + return handleEtcdError(err) +} + +func handleEtcdError(e error) error { + ee, ok := e.(client.Error) + if !ok { + return e + } + code := fission.ErrorInternal + msg := ee.Error() + + //TODO: handle any other etcd error codes we care about + switch ee.Code { + case client.ErrorCodeNodeExist: + code = fission.ErrorNameExists + msg = fmt.Sprintf("%v (%v)", ee.Message, ee.Cause) + } + return fission.MakeError(code, msg) } diff --git a/error.go b/error.go index 7f8f04e6..dfba53f3 100644 --- a/error.go +++ b/error.go @@ -18,7 +18,9 @@ package fission import ( "fmt" + "io/ioutil" "net/http" + "strings" ) func (e Error) Error() string { @@ -36,29 +38,39 @@ func MakeErrorFromHTTP(resp *http.Response) error { var errCode int switch resp.StatusCode { + case 400: + errCode = ErrorInvalidArgument case 403: errCode = ErrorNotAuthorized case 404: errCode = ErrorNotFound - case 400: - errCode = ErrorInvalidArgument + case 409: + errCode = ErrorNameExists default: errCode = ErrorInternal } - return MakeError(errCode, resp.Status) + + msg := resp.Status + defer resp.Body.Close() + body, err := ioutil.ReadAll(resp.Body) + if err == nil && len(body) > 0 { + msg = strings.TrimSpace(string(body)) + } + + return MakeError(errCode, msg) } func (err Error) HTTPStatus() int { var code int switch err.Code { - case ErrorNotFound: - code = 404 case ErrorInvalidArgument: code = 400 - case ErrorNoSpace: - code = 500 case ErrorNotAuthorized: code = 403 + case ErrorNotFound: + code = 404 + case ErrorNameExists: + code = 409 default: code = 500 } @@ -70,8 +82,8 @@ func GetHTTPError(err error) (int, string) { var code int fe, ok := err.(Error) if ok { - msg = fe.Message code = fe.HTTPStatus() + msg = fe.Message } else { code = 500 msg = err.Error()