Provide helpers and tests for common error types

Unify error handling in apiserver into a single path - RESTStorage
objects must provide appropriate errors individually.  Ensure ALL
errors which can be traced to logical faults with RESTStorage are
returned as api.Status objects.
pull/6/head
Clayton Coleman 2014-07-31 14:26:34 -04:00
parent 71c6e082d4
commit 0083fae453
6 changed files with 202 additions and 113 deletions

View File

@ -82,7 +82,7 @@ func New(storage map[string]RESTStorage, codec Codec, prefix string) *APIServer
// Watch API handlers
watchPrefix := path.Join(prefix, "watch") + "/"
mux.Handle(watchPrefix, http.StripPrefix(watchPrefix, &WatchHandler{storage}))
mux.Handle(watchPrefix, http.StripPrefix(watchPrefix, &WatchHandler{storage, codec}))
// Support services for the apiserver
logsPrefix := "/logs/"
@ -167,23 +167,19 @@ func (s *APIServer) handleRESTStorage(parts []string, req *http.Request, w http.
case 1:
selector, err := labels.ParseSelector(req.URL.Query().Get("labels"))
if err != nil {
internalError(err, w)
errorJSON(err, s.codec, w)
return
}
list, err := storage.List(selector)
if err != nil {
internalError(err, w)
errorJSON(err, s.codec, w)
return
}
writeJSON(http.StatusOK, s.codec, list, w)
case 2:
item, err := storage.Get(parts[1])
if IsNotFound(err) {
notFound(w, req)
return
}
if err != nil {
internalError(err, w)
errorJSON(err, s.codec, w)
return
}
writeJSON(http.StatusOK, s.codec, item, w)
@ -198,26 +194,18 @@ func (s *APIServer) handleRESTStorage(parts []string, req *http.Request, w http.
}
body, err := readBody(req)
if err != nil {
internalError(err, w)
errorJSON(err, s.codec, w)
return
}
obj := storage.New()
err = s.codec.DecodeInto(body, obj)
if IsNotFound(err) {
notFound(w, req)
return
}
if err != nil {
internalError(err, w)
errorJSON(err, s.codec, w)
return
}
out, err := storage.Create(obj)
if IsNotFound(err) {
notFound(w, req)
return
}
if err != nil {
internalError(err, w)
errorJSON(err, s.codec, w)
return
}
op := s.createOperation(out, sync, timeout)
@ -229,12 +217,8 @@ func (s *APIServer) handleRESTStorage(parts []string, req *http.Request, w http.
return
}
out, err := storage.Delete(parts[1])
if IsNotFound(err) {
notFound(w, req)
return
}
if err != nil {
internalError(err, w)
errorJSON(err, s.codec, w)
return
}
op := s.createOperation(out, sync, timeout)
@ -247,26 +231,18 @@ func (s *APIServer) handleRESTStorage(parts []string, req *http.Request, w http.
}
body, err := readBody(req)
if err != nil {
internalError(err, w)
errorJSON(err, s.codec, w)
return
}
obj := storage.New()
err = s.codec.DecodeInto(body, obj)
if IsNotFound(err) {
notFound(w, req)
return
}
if err != nil {
internalError(err, w)
errorJSON(err, s.codec, w)
return
}
out, err := storage.Update(obj)
if IsNotFound(err) {
notFound(w, req)
return
}
if err != nil {
internalError(err, w)
errorJSON(err, s.codec, w)
return
}
op := s.createOperation(out, sync, timeout)
@ -320,7 +296,7 @@ func (s *APIServer) finishReq(op *Operation, w http.ResponseWriter) {
func writeJSON(statusCode int, codec Codec, object interface{}, w http.ResponseWriter) {
output, err := codec.Encode(object)
if err != nil {
internalError(err, w)
errorJSON(err, codec, w)
return
}
w.Header().Set("Content-Type", "application/json")
@ -328,11 +304,17 @@ func writeJSON(statusCode int, codec Codec, object interface{}, w http.ResponseW
w.Write(output)
}
// errorJSON renders an error to the response
func errorJSON(err error, codec Codec, w http.ResponseWriter) {
status := errToAPIStatus(err)
writeJSON(status.Code, codec, status, w)
}
// writeRawJSON writes a non-API object in JSON.
func writeRawJSON(statusCode int, object interface{}, w http.ResponseWriter) {
output, err := json.Marshal(object)
if err != nil {
internalError(err, w)
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
w.Header().Set("Content-Type", "application/json")

View File

@ -25,6 +25,7 @@ import (
"net/http"
"net/http/httptest"
"reflect"
"strings"
"sync"
"testing"
"time"
@ -433,26 +434,6 @@ func TestUpdateMissing(t *testing.T) {
}
}
func TestBadPath(t *testing.T) {
handler := New(map[string]RESTStorage{}, codec, "/prefix/version")
server := httptest.NewServer(handler)
client := http.Client{}
request, err := http.NewRequest("GET", server.URL+"/foobar", nil)
if err != nil {
t.Errorf("unexpected error: %v", err)
}
response, err := client.Do(request)
if err != nil {
t.Errorf("unexpected error: %v", err)
}
if response.StatusCode != http.StatusNotFound {
t.Errorf("Unexpected response %#v", response)
}
}
func TestCreate(t *testing.T) {
simpleStorage := &SimpleRESTStorage{}
handler := New(map[string]RESTStorage{
@ -588,33 +569,64 @@ func expectApiStatus(t *testing.T, method, url string, data []byte, code int) *a
}
response, err := client.Do(request)
if err != nil {
t.Fatalf("unexpected error %#v", err)
t.Fatalf("unexpected error on %s %s: %v", method, url, err)
return nil
}
var status api.Status
_, err = extractBody(response, &status)
if err != nil {
t.Fatalf("unexpected error %#v", err)
t.Fatalf("unexpected error on %s %s: %v", method, url, err)
return nil
}
if code != response.StatusCode {
t.Fatalf("Expected %d, Got %d", code, response.StatusCode)
t.Fatalf("Expected %s %s to return %d, Got %d", method, url, code, response.StatusCode)
}
return &status
}
func TestErrorsToAPIStatus(t *testing.T) {
cases := map[error]api.Status{
NewAlreadyExistsErr("foo", "bar"): api.Status{
Status: api.StatusFailure,
Code: http.StatusConflict,
Reason: "already_exists",
Message: "foo \"bar\" already exists",
Details: &api.StatusDetails{
Kind: "foo",
ID: "bar",
},
},
NewConflictErr("foo", "bar", errors.New("failure")): api.Status{
Status: api.StatusFailure,
Code: http.StatusConflict,
Reason: "conflict",
Message: "foo \"bar\" cannot be updated: failure",
Details: &api.StatusDetails{
Kind: "foo",
ID: "bar",
},
},
}
for k, v := range cases {
actual := errToAPIStatus(k)
if !reflect.DeepEqual(actual, &v) {
t.Errorf("Expected %#v, Got %#v", v, actual)
}
}
}
func TestAsyncDelayReturnsError(t *testing.T) {
storage := SimpleRESTStorage{
injectedFunction: func(obj interface{}) (interface{}, error) {
return nil, errors.New("error")
return nil, NewAlreadyExistsErr("foo", "bar")
},
}
handler := New(map[string]RESTStorage{"foo": &storage}, codec, "/prefix/version")
handler.asyncOpWait = time.Millisecond / 2
server := httptest.NewServer(handler)
status := expectApiStatus(t, "DELETE", fmt.Sprintf("%s/prefix/version/foo/bar", server.URL), nil, http.StatusInternalServerError)
if status.Status != api.StatusFailure || status.Message != "error" || status.Details != nil {
status := expectApiStatus(t, "DELETE", fmt.Sprintf("%s/prefix/version/foo/bar", server.URL), nil, http.StatusConflict)
if status.Status != api.StatusFailure || status.Message == "" || status.Details == nil || status.Reason != api.ReasonTypeAlreadyExists {
t.Errorf("Unexpected status %#v", status)
}
}
@ -624,7 +636,7 @@ func TestAsyncCreateError(t *testing.T) {
storage := SimpleRESTStorage{
injectedFunction: func(obj interface{}) (interface{}, error) {
<-ch
return nil, errors.New("error")
return nil, NewAlreadyExistsErr("foo", "bar")
},
}
handler := New(map[string]RESTStorage{"foo": &storage}, codec, "/prefix/version")
@ -648,13 +660,22 @@ func TestAsyncCreateError(t *testing.T) {
time.Sleep(time.Millisecond)
finalStatus := expectApiStatus(t, "GET", fmt.Sprintf("%s/prefix/version/operations/%s?after=1", server.URL, status.Details.ID), []byte{}, http.StatusOK)
expectedErr := NewAlreadyExistsErr("foo", "bar")
expectedStatus := &api.Status{
Code: http.StatusInternalServerError,
Message: "error",
Status: api.StatusFailure,
Code: http.StatusConflict,
Reason: "already_exists",
Message: expectedErr.Error(),
Details: &api.StatusDetails{
Kind: "foo",
ID: "bar",
},
}
if !reflect.DeepEqual(expectedStatus, finalStatus) {
t.Errorf("Expected %#v, Got %#v", expectedStatus, finalStatus)
if finalStatus.Details != nil {
t.Logf("Details %#v, Got %#v", *expectedStatus.Details, *finalStatus.Details)
}
}
}
@ -665,14 +686,12 @@ func TestWriteJSONDecodeError(t *testing.T) {
}
writeJSON(http.StatusOK, api.Codec, &T{"Undecodable"}, w)
}))
client := http.Client{}
resp, err := client.Get(server.URL)
if err != nil {
t.Errorf("unexpected error: %v", err)
status := expectApiStatus(t, "GET", server.URL, nil, http.StatusInternalServerError)
if status.Reason != api.ReasonTypeUnknown {
t.Errorf("unexpected reason %#v", status)
}
if resp.StatusCode != http.StatusInternalServerError {
t.Errorf("unexpected status code %d", resp.StatusCode)
if !strings.Contains(status.Message, "type apiserver.T is not registered") {
t.Errorf("unexpected message %#v", status)
}
}

View File

@ -17,10 +17,6 @@ limitations under the License.
package apiserver
import (
"net/http"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/tools"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
)
@ -37,16 +33,7 @@ func MakeAsync(fn WorkFunc) <-chan interface{} {
defer util.HandleCrash()
obj, err := fn()
if err != nil {
status := http.StatusInternalServerError
switch {
case tools.IsEtcdTestFailed(err):
status = http.StatusConflict
}
channel <- &api.Status{
Status: api.StatusFailure,
Message: err.Error(),
Code: status,
}
channel <- errToAPIStatus(err)
} else {
channel <- obj
}

View File

@ -19,50 +19,108 @@ package apiserver
import (
"fmt"
"net/http"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/tools"
)
// errNotFound is an error which indicates that a specified resource is not found.
type errNotFound string
// Error returns a string representation of the err.
func (err errNotFound) Error() string {
return string(err)
// apiServerError is an error intended for consumption by a REST API server
type apiServerError struct {
api.Status
}
// IsNotFound determines if the err is an error which indicates that a specified resource was not found.
func IsNotFound(err error) bool {
_, ok := err.(errNotFound)
return ok
// Error implements the Error interface.
func (e *apiServerError) Error() string {
return e.Status.Message
}
// NewNotFoundErr returns a new error which indicates that the resource of the kind and the name was not found.
func NewNotFoundErr(kind, name string) error {
return errNotFound(fmt.Sprintf("%s %q not found", kind, name))
return &apiServerError{api.Status{
Status: api.StatusFailure,
Code: http.StatusNotFound,
Reason: api.ReasonTypeNotFound,
Details: &api.StatusDetails{
Kind: kind,
ID: name,
},
Message: fmt.Sprintf("%s %q not found", kind, name),
}}
}
// errAlreadyExists is an error which indicates that a specified resource already exists.
type errAlreadyExists string
// NewAlreadyExistsErr returns an error indicating the item requested exists by that identifier
func NewAlreadyExistsErr(kind, name string) error {
return &apiServerError{api.Status{
Status: api.StatusFailure,
Code: http.StatusConflict,
Reason: api.ReasonTypeAlreadyExists,
Details: &api.StatusDetails{
Kind: kind,
ID: name,
},
Message: fmt.Sprintf("%s %q already exists", kind, name),
}}
}
// Error returns a string representation of the err.
func (err errAlreadyExists) Error() string {
return string(err)
// NewConflictErr returns an error indicating the item can't be updated as provided.
func NewConflictErr(kind, name string, err error) error {
return &apiServerError{api.Status{
Status: api.StatusFailure,
Code: http.StatusConflict,
Reason: api.ReasonTypeConflict,
Details: &api.StatusDetails{
Kind: kind,
ID: name,
},
Message: fmt.Sprintf("%s %q cannot be updated: %s", kind, name, err),
}}
}
// IsNotFound returns true if the specified error was created by NewNotFoundErr
func IsNotFound(err error) bool {
return reasonForError(err) == api.ReasonTypeNotFound
}
// IsAlreadyExists determines if the err is an error which indicates that a specified resource already exists.
func IsAlreadyExists(err error) bool {
_, ok := err.(errAlreadyExists)
return ok
return reasonForError(err) == api.ReasonTypeAlreadyExists
}
// NewAlreadyExistsErr returns a new error which indicates that the resource of the kind and the name was not found.
func NewAlreadyExistsErr(kind, name string) error {
return errAlreadyExists(fmt.Sprintf("%s %q already exists", kind, name))
// IsConflict determines if the err is an error which indicates the provided update conflicts
func IsConflict(err error) bool {
return reasonForError(err) == api.ReasonTypeConflict
}
// internalError renders a generic error to the response
func internalError(err error, w http.ResponseWriter) {
w.WriteHeader(http.StatusInternalServerError)
fmt.Fprintf(w, "Internal Error: %#v", err)
func reasonForError(err error) api.ReasonType {
switch t := err.(type) {
case *apiServerError:
return t.Status.Reason
}
return api.ReasonTypeUnknown
}
// errToAPIStatus converts an error to an api.Status object.
func errToAPIStatus(err error) *api.Status {
switch t := err.(type) {
case *apiServerError:
status := t.Status
status.Status = api.StatusFailure
//TODO: check for invalid responses
return &status
default:
status := http.StatusInternalServerError
switch {
//TODO: replace me with NewUpdateConflictErr
case tools.IsEtcdTestFailed(err):
status = http.StatusConflict
}
return &api.Status{
Status: api.StatusFailure,
Code: status,
Reason: api.ReasonTypeUnknown,
Message: err.Error(),
}
}
}
// notFound renders a simple not found error

View File

@ -0,0 +1,42 @@
/*
Copyright 2014 Google Inc. All rights reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package apiserver
import (
"errors"
"testing"
)
func TestErrorNew(t *testing.T) {
err := NewAlreadyExistsErr("test", "1")
if !IsAlreadyExists(err) {
t.Errorf("expected to be already_exists")
}
if IsConflict(err) {
t.Errorf("expected to not be confict")
}
if IsNotFound(err) {
t.Errorf("expected to not be not_found")
}
if !IsConflict(NewConflictErr("test", "2", errors.New("message"))) {
t.Errorf("expected to be confict")
}
if !IsNotFound(NewNotFoundErr("test", "3")) {
t.Errorf("expected to be not found")
}
}

View File

@ -31,6 +31,7 @@ import (
type WatchHandler struct {
storage map[string]RESTStorage
codec Codec
}
func getWatchParams(query url.Values) (label, field labels.Selector, resourceVersion uint64) {
@ -64,7 +65,7 @@ func (h *WatchHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
label, field, resourceVersion := getWatchParams(req.URL.Query())
watching, err := watcher.Watch(label, field, resourceVersion)
if err != nil {
internalError(err, w)
errorJSON(err, h.codec, w)
return
}