diff --git a/api/server/httputils/errors.go b/api/server/httputils/errors.go index e3eaba9290..0f89dd90fc 100644 --- a/api/server/httputils/errors.go +++ b/api/server/httputils/errors.go @@ -5,6 +5,9 @@ import ( "strings" "github.com/Sirupsen/logrus" + "github.com/docker/engine-api/types" + "github.com/docker/engine-api/types/versions" + "github.com/gorilla/mux" ) // httpStatusError is an interface @@ -70,13 +73,19 @@ func GetHTTPErrorStatusCode(err error) int { return statusCode } -// WriteError decodes a specific docker error and sends it in the response. -func WriteError(w http.ResponseWriter, err error) { - if err == nil || w == nil { - logrus.WithFields(logrus.Fields{"error": err, "writer": w}).Error("unexpected HTTP error handling") - return +// MakeErrorHandler makes an HTTP handler that decodes a Docker error and +// returns it in the response. +func MakeErrorHandler(err error) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + statusCode := GetHTTPErrorStatusCode(err) + vars := mux.Vars(r) + if vars["version"] == "" || versions.GreaterThan(vars["version"], "1.23") { + response := &types.ErrorResponse{ + Message: err.Error(), + } + WriteJSON(w, statusCode, response) + } else { + http.Error(w, err.Error(), statusCode) + } } - - statusCode := GetHTTPErrorStatusCode(err) - http.Error(w, err.Error(), statusCode) } diff --git a/api/server/server.go b/api/server/server.go index f406aeeba1..391aa5d567 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -2,6 +2,7 @@ package server import ( "crypto/tls" + "fmt" "net" "net/http" "strings" @@ -10,6 +11,7 @@ import ( "github.com/docker/docker/api/server/httputils" "github.com/docker/docker/api/server/middleware" "github.com/docker/docker/api/server/router" + "github.com/docker/docker/errors" "github.com/gorilla/mux" "golang.org/x/net/context" ) @@ -136,7 +138,7 @@ func (s *Server) makeHTTPHandler(handler httputils.APIFunc) http.HandlerFunc { if err := handlerFunc(ctx, w, r, vars); err != nil { logrus.Errorf("Handler for %s %s returned error: %v", r.Method, r.URL.Path, err) - httputils.WriteError(w, err) + httputils.MakeErrorHandler(err)(w, r) } } } @@ -172,6 +174,11 @@ func (s *Server) createMux() *mux.Router { } } + err := errors.NewRequestNotFoundError(fmt.Errorf("page not found")) + notFoundHandler := httputils.MakeErrorHandler(err) + m.HandleFunc(versionMatcher+"/{path:.*}", notFoundHandler) + m.NotFoundHandler = notFoundHandler + return m } diff --git a/docs/reference/api/docker_remote_api.md b/docs/reference/api/docker_remote_api.md index 8b87496f2a..34e19cd239 100644 --- a/docs/reference/api/docker_remote_api.md +++ b/docs/reference/api/docker_remote_api.md @@ -131,6 +131,7 @@ This section lists each version from latest to oldest. Each listing includes a * `POST /images/(name)/tag` no longer has a `force` query parameter. * `GET /images/search` now supports maximum returned search results `limit`. * `POST /containers/{name:.*}/copy` is now removed and errors out starting from this API version. +* API errors are now returned as JSON instead of plain text. ### v1.23 API changes @@ -262,4 +263,3 @@ end point now returns the new boolean fields `CpuCfsPeriod`, `CpuCfsQuota`, and * `CgroupParent` can be passed in the host config to setup container cgroups under a specific cgroup. * `POST /build` closing the HTTP request cancels the build * `POST /containers/(id)/exec` includes `Warnings` field to response. - diff --git a/docs/reference/api/docker_remote_api_v1.24.md b/docs/reference/api/docker_remote_api_v1.24.md index d525a71944..4d44796307 100644 --- a/docs/reference/api/docker_remote_api_v1.24.md +++ b/docs/reference/api/docker_remote_api_v1.24.md @@ -22,9 +22,19 @@ weight=-5 - When the client API version is newer than the daemon's, these calls return an HTTP `400 Bad Request` error message. -# 2. Endpoints +# 2. Errors -## 2.1 Containers +The Remote API uses standard HTTP status codes to indicate the success or failure of the API call. The body of the response will be JSON in the following format: + + { + "message": "page not found" + } + +The status codes that are returned for each endpoint are specified in the endpoint documentation below. + +# 3. Endpoints + +## 3.1 Containers ### List containers @@ -1504,7 +1514,7 @@ Status Codes: - no such file or directory (**path** resource does not exist) - **500** – server error -## 2.2 Images +## 3.2 Images ### List Images @@ -2112,7 +2122,7 @@ Status Codes: - **200** – no error - **500** – server error -## 2.3 Misc +## 3.3 Misc ### Check auth configuration @@ -2834,7 +2844,7 @@ Status Codes: - **404** – no such exec instance - **500** - server error -## 2.4 Volumes +## 3.4 Volumes ### List volumes @@ -2972,7 +2982,7 @@ Status Codes - **409** - volume is in use and cannot be removed - **500** - server error -## 2.5 Networks +## 3.5 Networks ### List networks @@ -3296,9 +3306,9 @@ Status Codes - **404** - no such network - **500** - server error -# 3. Going further +# 4. Going further -## 3.1 Inside `docker run` +## 4.1 Inside `docker run` As an example, the `docker run` command line makes the following API calls: @@ -3316,7 +3326,7 @@ As an example, the `docker run` command line makes the following API calls: - If in detached mode or only `stdin` is attached, display the container's id. -## 3.2 Hijacking +## 4.2 Hijacking In this version of the API, `/attach`, uses hijacking to transport `stdin`, `stdout`, and `stderr` on the same socket. @@ -3331,7 +3341,7 @@ When Docker daemon detects the `Upgrade` header, it switches its status code from **200 OK** to **101 UPGRADED** and resends the same headers. -## 3.3 CORS Requests +## 4.3 CORS Requests To set cross origin requests to the remote api please give values to `--api-cors-header` when running Docker in daemon mode. Set * (asterisk) allows all, diff --git a/integration-cli/docker_api_attach_test.go b/integration-cli/docker_api_attach_test.go index c0fa675bb0..a9fcc962b6 100644 --- a/integration-cli/docker_api_attach_test.go +++ b/integration-cli/docker_api_attach_test.go @@ -85,8 +85,8 @@ func (s *DockerSuite) TestGetContainersWsAttachContainerNotFound(c *check.C) { status, body, err := sockRequest("GET", "/containers/doesnotexist/attach/ws", nil) c.Assert(status, checker.Equals, http.StatusNotFound) c.Assert(err, checker.IsNil) - expected := "No such container: doesnotexist\n" - c.Assert(string(body), checker.Contains, expected) + expected := "No such container: doesnotexist" + c.Assert(getErrorMessage(c, body), checker.Contains, expected) } func (s *DockerSuite) TestPostContainersAttach(c *check.C) { diff --git a/integration-cli/docker_api_auth_test.go b/integration-cli/docker_api_auth_test.go index 798c3b5eaa..d73c61d411 100644 --- a/integration-cli/docker_api_auth_test.go +++ b/integration-cli/docker_api_auth_test.go @@ -16,9 +16,10 @@ func (s *DockerSuite) TestAuthApi(c *check.C) { Password: "no-password", } - expected := "Get https://registry-1.docker.io/v2/: unauthorized: incorrect username or password\n" + expected := "Get https://registry-1.docker.io/v2/: unauthorized: incorrect username or password" status, body, err := sockRequest("POST", "/auth", config) c.Assert(err, check.IsNil) c.Assert(status, check.Equals, http.StatusUnauthorized) - c.Assert(string(body), checker.Contains, expected, check.Commentf("Expected: %v, got: %v", expected, string(body))) + msg := getErrorMessage(c, body) + c.Assert(msg, checker.Contains, expected, check.Commentf("Expected: %v, got: %v", expected, msg)) } diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index 52e3a02326..58f0b40fed 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/integration-cli/docker_api_containers_test.go @@ -480,10 +480,10 @@ func (s *DockerSuite) TestContainerApiBadPort(c *check.C) { jsonData := bytes.NewBuffer(nil) json.NewEncoder(jsonData).Encode(config) - status, b, err := sockRequest("POST", "/containers/create", config) + status, body, err := sockRequest("POST", "/containers/create", config) c.Assert(err, checker.IsNil) c.Assert(status, checker.Equals, http.StatusInternalServerError) - c.Assert(strings.TrimSpace(string(b)), checker.Equals, `Invalid port specification: "aa80"`, check.Commentf("Incorrect error msg: %s", string(b))) + c.Assert(getErrorMessage(c, body), checker.Equals, `Invalid port specification: "aa80"`, check.Commentf("Incorrect error msg: %s", body)) } func (s *DockerSuite) TestContainerApiCreate(c *check.C) { @@ -509,12 +509,12 @@ func (s *DockerSuite) TestContainerApiCreate(c *check.C) { func (s *DockerSuite) TestContainerApiCreateEmptyConfig(c *check.C) { config := map[string]interface{}{} - status, b, err := sockRequest("POST", "/containers/create", config) + status, body, err := sockRequest("POST", "/containers/create", config) c.Assert(err, checker.IsNil) c.Assert(status, checker.Equals, http.StatusInternalServerError) - expected := "Config cannot be empty in order to create a container\n" - c.Assert(string(b), checker.Equals, expected) + expected := "Config cannot be empty in order to create a container" + c.Assert(getErrorMessage(c, body), checker.Equals, expected) } func (s *DockerSuite) TestContainerApiCreateMultipleNetworksConfig(c *check.C) { @@ -530,14 +530,15 @@ func (s *DockerSuite) TestContainerApiCreateMultipleNetworksConfig(c *check.C) { }, } - status, b, err := sockRequest("POST", "/containers/create", config) + status, body, err := sockRequest("POST", "/containers/create", config) c.Assert(err, checker.IsNil) c.Assert(status, checker.Equals, http.StatusBadRequest) + msg := getErrorMessage(c, body) // network name order in error message is not deterministic - c.Assert(string(b), checker.Contains, "Container cannot be connected to network endpoints") - c.Assert(string(b), checker.Contains, "net1") - c.Assert(string(b), checker.Contains, "net2") - c.Assert(string(b), checker.Contains, "net3") + c.Assert(msg, checker.Contains, "Container cannot be connected to network endpoints") + c.Assert(msg, checker.Contains, "net1") + c.Assert(msg, checker.Contains, "net2") + c.Assert(msg, checker.Contains, "net3") } func (s *DockerSuite) TestContainerApiCreateWithHostName(c *check.C) { @@ -997,7 +998,7 @@ func (s *DockerSuite) TestContainerApiDeleteNotExist(c *check.C) { status, body, err := sockRequest("DELETE", "/containers/doesnotexist", nil) c.Assert(err, checker.IsNil) c.Assert(status, checker.Equals, http.StatusNotFound) - c.Assert(string(body), checker.Matches, "No such container: doesnotexist\n") + c.Assert(getErrorMessage(c, body), checker.Matches, "No such container: doesnotexist") } func (s *DockerSuite) TestContainerApiDeleteForce(c *check.C) { @@ -1247,8 +1248,8 @@ func (s *DockerSuite) TestPostContainersCreateWithWrongCpusetValues(c *check.C) status, body, err := sockRequest("POST", "/containers/create?name="+name, c1) c.Assert(err, checker.IsNil) c.Assert(status, checker.Equals, http.StatusInternalServerError) - expected := "Invalid value 1-42,, for cpuset cpus\n" - c.Assert(string(body), checker.Equals, expected) + expected := "Invalid value 1-42,, for cpuset cpus" + c.Assert(getErrorMessage(c, body), checker.Equals, expected) c2 := struct { Image string @@ -1258,8 +1259,8 @@ func (s *DockerSuite) TestPostContainersCreateWithWrongCpusetValues(c *check.C) status, body, err = sockRequest("POST", "/containers/create?name="+name, c2) c.Assert(err, checker.IsNil) c.Assert(status, checker.Equals, http.StatusInternalServerError) - expected = "Invalid value 42-3,1-- for cpuset mems\n" - c.Assert(string(body), checker.Equals, expected) + expected = "Invalid value 42-3,1-- for cpuset mems" + c.Assert(getErrorMessage(c, body), checker.Equals, expected) } func (s *DockerSuite) TestPostContainersCreateShmSizeNegative(c *check.C) { @@ -1273,7 +1274,7 @@ func (s *DockerSuite) TestPostContainersCreateShmSizeNegative(c *check.C) { status, body, err := sockRequest("POST", "/containers/create", config) c.Assert(err, check.IsNil) c.Assert(status, check.Equals, http.StatusInternalServerError) - c.Assert(string(body), checker.Contains, "SHM size must be greater than 0") + c.Assert(getErrorMessage(c, body), checker.Contains, "SHM size must be greater than 0") } func (s *DockerSuite) TestPostContainersCreateShmSizeHostConfigOmitted(c *check.C) { @@ -1409,9 +1410,11 @@ func (s *DockerSuite) TestPostContainersCreateWithOomScoreAdjInvalidRange(c *che status, b, err := sockRequest("POST", "/containers/create?name="+name, config) c.Assert(err, check.IsNil) c.Assert(status, check.Equals, http.StatusInternalServerError) + expected := "Invalid value 1001, range for oom score adj is [-1000, 1000]" - if !strings.Contains(string(b), expected) { - c.Fatalf("Expected output to contain %q, got %q", expected, string(b)) + msg := getErrorMessage(c, b) + if !strings.Contains(msg, expected) { + c.Fatalf("Expected output to contain %q, got %q", expected, msg) } config = struct { @@ -1423,8 +1426,9 @@ func (s *DockerSuite) TestPostContainersCreateWithOomScoreAdjInvalidRange(c *che c.Assert(err, check.IsNil) c.Assert(status, check.Equals, http.StatusInternalServerError) expected = "Invalid value -1001, range for oom score adj is [-1000, 1000]" - if !strings.Contains(string(b), expected) { - c.Fatalf("Expected output to contain %q, got %q", expected, string(b)) + msg = getErrorMessage(c, b) + if !strings.Contains(msg, expected) { + c.Fatalf("Expected output to contain %q, got %q", expected, msg) } } diff --git a/integration-cli/docker_api_create_test.go b/integration-cli/docker_api_create_test.go index d29b35501b..355e8f8749 100644 --- a/integration-cli/docker_api_create_test.go +++ b/integration-cli/docker_api_create_test.go @@ -2,7 +2,6 @@ package main import ( "net/http" - "strings" "github.com/docker/docker/pkg/integration/checker" "github.com/go-check/check" @@ -15,31 +14,31 @@ func (s *DockerSuite) TestApiCreateWithNotExistImage(c *check.C) { "Volumes": map[string]struct{}{"/tmp": {}}, } - status, resp, err := sockRequest("POST", "/containers/create?name="+name, config) + status, body, err := sockRequest("POST", "/containers/create?name="+name, config) c.Assert(err, check.IsNil) c.Assert(status, check.Equals, http.StatusNotFound) expected := "No such image: test456:v1" - c.Assert(strings.TrimSpace(string(resp)), checker.Contains, expected) + c.Assert(getErrorMessage(c, body), checker.Contains, expected) config2 := map[string]interface{}{ "Image": "test456", "Volumes": map[string]struct{}{"/tmp": {}}, } - status, resp, err = sockRequest("POST", "/containers/create?name="+name, config2) + status, body, err = sockRequest("POST", "/containers/create?name="+name, config2) c.Assert(err, check.IsNil) c.Assert(status, check.Equals, http.StatusNotFound) expected = "No such image: test456:latest" - c.Assert(strings.TrimSpace(string(resp)), checker.Equals, expected) + c.Assert(getErrorMessage(c, body), checker.Equals, expected) config3 := map[string]interface{}{ "Image": "sha256:0cb40641836c461bc97c793971d84d758371ed682042457523e4ae701efeaaaa", } - status, resp, err = sockRequest("POST", "/containers/create?name="+name, config3) + status, body, err = sockRequest("POST", "/containers/create?name="+name, config3) c.Assert(err, check.IsNil) c.Assert(status, check.Equals, http.StatusNotFound) expected = "No such image: sha256:0cb40641836c461bc97c793971d84d758371ed682042457523e4ae701efeaaaa" - c.Assert(strings.TrimSpace(string(resp)), checker.Equals, expected) + c.Assert(getErrorMessage(c, body), checker.Equals, expected) } diff --git a/integration-cli/docker_api_exec_test.go b/integration-cli/docker_api_exec_test.go index f16582f40f..ec33637aa8 100644 --- a/integration-cli/docker_api_exec_test.go +++ b/integration-cli/docker_api_exec_test.go @@ -24,7 +24,7 @@ func (s *DockerSuite) TestExecApiCreateNoCmd(c *check.C) { c.Assert(status, checker.Equals, http.StatusInternalServerError) comment := check.Commentf("Expected message when creating exec command with no Cmd specified") - c.Assert(string(body), checker.Contains, "No exec command specified", comment) + c.Assert(getErrorMessage(c, body), checker.Contains, "No exec command specified", comment) } func (s *DockerSuite) TestExecApiCreateNoValidContentType(c *check.C) { @@ -44,7 +44,7 @@ func (s *DockerSuite) TestExecApiCreateNoValidContentType(c *check.C) { c.Assert(err, checker.IsNil) comment := check.Commentf("Expected message when creating exec command with invalid Content-Type specified") - c.Assert(string(b), checker.Contains, "Content-Type specified", comment) + c.Assert(getErrorMessage(c, b), checker.Contains, "Content-Type specified", comment) } func (s *DockerSuite) TestExecApiCreateContainerPaused(c *check.C) { @@ -59,7 +59,7 @@ func (s *DockerSuite) TestExecApiCreateContainerPaused(c *check.C) { c.Assert(status, checker.Equals, http.StatusConflict) comment := check.Commentf("Expected message when creating exec command with Container %s is paused", name) - c.Assert(string(body), checker.Contains, "Container "+name+" is paused, unpause the container before exec", comment) + c.Assert(getErrorMessage(c, body), checker.Contains, "Container "+name+" is paused, unpause the container before exec", comment) } func (s *DockerSuite) TestExecApiStart(c *check.C) { diff --git a/integration-cli/docker_api_logs_test.go b/integration-cli/docker_api_logs_test.go index 2ff27f8ecc..fb34a2636f 100644 --- a/integration-cli/docker_api_logs_test.go +++ b/integration-cli/docker_api_logs_test.go @@ -60,9 +60,7 @@ func (s *DockerSuite) TestLogsApiNoStdoutNorStderr(c *check.C) { c.Assert(err, checker.IsNil) expected := "Bad parameters: you must choose at least one stream" - if !bytes.Contains(body, []byte(expected)) { - c.Fatalf("Expected %s, got %s", expected, string(body[:])) - } + c.Assert(getErrorMessage(c, body), checker.Contains, expected) } // Regression test for #12704 diff --git a/integration-cli/docker_api_resize_test.go b/integration-cli/docker_api_resize_test.go index 73023dd2dc..041df6cb94 100644 --- a/integration-cli/docker_api_resize_test.go +++ b/integration-cli/docker_api_resize_test.go @@ -40,5 +40,5 @@ func (s *DockerSuite) TestResizeApiResponseWhenContainerNotStarted(c *check.C) { c.Assert(status, check.Equals, http.StatusInternalServerError) c.Assert(err, check.IsNil) - c.Assert(string(body), checker.Contains, "is not running", check.Commentf("resize should fail with message 'Container is not running'")) + c.Assert(getErrorMessage(c, body), checker.Contains, "is not running", check.Commentf("resize should fail with message 'Container is not running'")) } diff --git a/integration-cli/docker_api_test.go b/integration-cli/docker_api_test.go index dc3f54ce6a..35ab1ee459 100644 --- a/integration-cli/docker_api_test.go +++ b/integration-cli/docker_api_test.go @@ -60,7 +60,7 @@ func (s *DockerSuite) TestApiClientVersionNewerThanServer(c *check.C) { c.Assert(err, checker.IsNil) c.Assert(status, checker.Equals, http.StatusBadRequest) expected := fmt.Sprintf("client is newer than server (client API version: %s, server API version: %s)", version, api.DefaultVersion) - c.Assert(strings.TrimSpace(string(body)), checker.Equals, expected) + c.Assert(getErrorMessage(c, body), checker.Equals, expected) } func (s *DockerSuite) TestApiClientVersionOldNotSupported(c *check.C) { @@ -99,3 +99,44 @@ func (s *DockerSuite) TestApiDockerApiVersion(c *check.C) { c.Fatalf("Out didn't have 'xxx' for the API version, had:\n%s", out) } } + +func (s *DockerSuite) TestApiErrorJSON(c *check.C) { + httpResp, body, err := sockRequestRaw("POST", "/containers/create", strings.NewReader(`{}`), "application/json") + c.Assert(err, checker.IsNil) + c.Assert(httpResp.StatusCode, checker.Equals, http.StatusInternalServerError) + c.Assert(httpResp.Header.Get("Content-Type"), checker.Equals, "application/json") + b, err := readBody(body) + c.Assert(err, checker.IsNil) + c.Assert(getErrorMessage(c, b), checker.Equals, "Config cannot be empty in order to create a container") +} + +func (s *DockerSuite) TestApiErrorPlainText(c *check.C) { + httpResp, body, err := sockRequestRaw("POST", "/v1.23/containers/create", strings.NewReader(`{}`), "application/json") + c.Assert(err, checker.IsNil) + c.Assert(httpResp.StatusCode, checker.Equals, http.StatusInternalServerError) + c.Assert(httpResp.Header.Get("Content-Type"), checker.Contains, "text/plain") + b, err := readBody(body) + c.Assert(err, checker.IsNil) + c.Assert(strings.TrimSpace(string(b)), checker.Equals, "Config cannot be empty in order to create a container") +} + +func (s *DockerSuite) TestApiErrorNotFoundJSON(c *check.C) { + // 404 is a different code path to normal errors, so test separately + httpResp, body, err := sockRequestRaw("GET", "/notfound", nil, "application/json") + c.Assert(err, checker.IsNil) + c.Assert(httpResp.StatusCode, checker.Equals, http.StatusNotFound) + c.Assert(httpResp.Header.Get("Content-Type"), checker.Equals, "application/json") + b, err := readBody(body) + c.Assert(err, checker.IsNil) + c.Assert(getErrorMessage(c, b), checker.Equals, "page not found") +} + +func (s *DockerSuite) TestApiErrorNotFoundPlainText(c *check.C) { + httpResp, body, err := sockRequestRaw("GET", "/v1.23/notfound", nil, "application/json") + c.Assert(err, checker.IsNil) + c.Assert(httpResp.StatusCode, checker.Equals, http.StatusNotFound) + c.Assert(httpResp.Header.Get("Content-Type"), checker.Contains, "text/plain") + b, err := readBody(body) + c.Assert(err, checker.IsNil) + c.Assert(strings.TrimSpace(string(b)), checker.Equals, "page not found") +} diff --git a/integration-cli/docker_utils.go b/integration-cli/docker_utils.go index c2d3fcb153..67a14c6abe 100644 --- a/integration-cli/docker_utils.go +++ b/integration-cli/docker_utils.go @@ -1507,3 +1507,10 @@ func waitForGoroutines(expected int) error { } } } + +// getErrorMessage returns the error message from an error API response +func getErrorMessage(c *check.C, body []byte) string { + var resp types.ErrorResponse + c.Assert(json.Unmarshal(body, &resp), check.IsNil) + return strings.TrimSpace(resp.Message) +}