Merge pull request #666 from kzys/har-error

Make http-address-resolver's errors more descriptive

Author: Kazuyoshi Kato

snapshotter/demux/proxy/address/http_resolver.go

@@ -47,7 +47,9 @@ func requestURL(url string, namespace string) string {
 
 // Get queries the proxy network type and address for the specified namespace.
 func (h HTTPResolver) Get(namespace string) (Response, error) {
-	httpResponse, err := h.client.Get(requestURL(h.url, namespace))
+	url := requestURL(h.url, namespace)
+
+	httpResponse, err := h.client.Get(url)
 	if err != nil {
 		return Response{}, err
 	}
@@ -58,11 +60,15 @@ func (h HTTPResolver) Get(namespace string) (Response, error) {
 		return Response{}, err
 	}
 
+	code := httpResponse.StatusCode
+	if code != 200 {
+		return Response{}, fmt.Errorf("failed to GET %s: status=%d, body=%s", url, code, body)
+	}
+
 	var response Response
 	err = json.Unmarshal(body, &response)
 	if err != nil {
 		return Response{}, err
 	}
-
 	return response, nil
 }

snapshotter/demux/proxy/address/http_resolver_test.go

@@ -69,13 +69,12 @@ func happyPath() error {
 				"test2": "label2"
 				}
 			}`), err: io.EOF}
-	client := mockClient{getError: nil, getResponse: http.Response{Body: &reader}}
+	client := mockClient{getError: nil, getResponse: http.Response{StatusCode: 200, Body: &reader}}
 	uut := HTTPResolver{url: "localhost:10001", client: &client}
 
 	actual, err := uut.Get("namespace")
 	if err != nil {
-		fmt.Println(err)
-		return errors.New("expected no error from HTTP resolver")
+		return fmt.Errorf("expected no error from HTTP resolver, but got %+v", err)
 	}
 
 	if actual.Network != "unix" {

snapshotter/internal/http_address_resolver.go

@@ -107,45 +107,44 @@ func queryAddress(writ http.ResponseWriter, req *http.Request) {
 	writ.Header().Set("Content-Type", "application/json")
 
 	keys, ok := req.URL.Query()["namespace"]
-
 	if !ok {
 		http.Error(writ, "Missing 'namespace' query", http.StatusBadRequest)
 		return
 	}
 
-	namespace := keys[0]
-
-	fcClient, err := client.New(containerdSockPath + ".ttrpc")
+	sock := containerdSockPath + ".ttrpc"
+	fcClient, err := client.New(sock)
 	if err != nil {
 		logger.WithError(err).Error("could not create firecracker client")
-		http.Error(writ, "Internal server error", http.StatusInternalServerError)
+		http.Error(writ, fmt.Sprintf("failed to connect %q", sock), http.StatusInternalServerError)
 		return
 	}
 	defer fcClient.Close()
 
+	namespace := keys[0]
 	ctx := namespaces.WithNamespace(req.Context(), namespace)
 	vmInfo, err := fcClient.GetVMInfo(ctx, &proto.GetVMInfoRequest{VMID: namespace})
 	if err != nil {
 		logger.WithField("VMID", namespace).WithError(err).Error("unable to retrieve VM Info")
-		http.Error(writ, "Internal server error", http.StatusInternalServerError)
+		http.Error(writ, fmt.Sprintf("failed to get VM %q", namespace), http.StatusNotFound)
 		return
 	}
 
 	writ.WriteHeader(http.StatusOK)
 
-	response, err := json.Marshal(proxyaddress.Response{
+	result := proxyaddress.Response{
 		Network:         "unix",
 		Address:         vmInfo.VSockPath,
 		SnapshotterPort: strconv.Itoa(remotePort),
 		MetricsPort:     strconv.Itoa(metricsRemotePort),
 		Labels: map[string]string{
 			"VMID": namespace,
 		},
-	})
+	}
+	serialized, err := json.Marshal(&result)
 	if err != nil {
-		http.Error(writ, "Internal server error", http.StatusInternalServerError)
+		http.Error(writ, fmt.Sprintf("failed to marshal %+v", result), http.StatusInternalServerError)
 		return
 	}
-
-	writ.Write(response)
+	writ.Write(serialized)
 }