Return remote API errors as JSON

Signed-off-by: Ben Firshman <[email protected]>

Author: bfirsh

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)
 }

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
 }
 

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.
-

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,

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) {

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))
 }

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)
 	}
 }
 

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)
 
 }

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) {

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