TUN-3443: Decode as v4api response on non-200 status

Author: chungthuang

tunnelstore/client.go

@@ -376,16 +376,12 @@ func (r *RESTClient) sendRequest(method string, url url.URL, body interface{}) (
 func parseResponse(reader io.Reader, data interface{}) error {
 	// Schema for Tunnelstore responses in the v1 API.
 	// Roughly, it's a wrapper around a particular result that adds failures/errors/etc
-	var result struct {
-		Result  json.RawMessage `json:"result"`
-		Success bool            `json:"success"`
-		Errors  []string        `json:"errors"`
-	}
+	var result response
 	// First, parse the wrapper and check the API call succeeded
 	if err := json.NewDecoder(reader).Decode(&result); err != nil {
 		return errors.Wrap(err, "failed to decode response")
 	}
-	if err := checkErrors(result.Errors); err != nil {
+	if err := result.checkErrors(); err != nil {
 		return err
 	}
 	if !result.Success {
@@ -402,24 +398,43 @@ func unmarshalTunnel(reader io.Reader) (*Tunnel, error) {
 	return &tunnel, err
 }
 
-func checkErrors(errs []string) error {
-	if len(errs) == 0 {
+type response struct {
+	Success  bool            `json:"success,omitempty"`
+	Errors   []apiErr        `json:"errors,omitempty"`
+	Messages []string        `json:"messages,omitempty"`
+	Result   json.RawMessage `json:"result,omitempty"`
+}
+
+func (r *response) checkErrors() error {
+	if len(r.Errors) == 0 {
 		return nil
 	}
-	if len(errs) == 1 {
-		return fmt.Errorf("API error: %s", errs[0])
+	if len(r.Errors) == 1 {
+		return r.Errors[0]
 	}
-	allErrs := strings.Join(errs, "; ")
-	return fmt.Errorf("API errors: %s", allErrs)
+	var messages string
+	for _, e := range r.Errors {
+		messages += fmt.Sprintf("%s; ", e)
+	}
+	return fmt.Errorf("API errors: %s", messages)
+}
+
+type apiErr struct {
+	Code    json.Number `json:"code,omitempty"`
+	Message string      `json:"message,omitempty"`
+}
+
+func (e apiErr) Error() string {
+	return fmt.Sprintf("code: %v, reason: %s", e.Code, e.Message)
 }
 
 func (r *RESTClient) statusCodeToError(op string, resp *http.Response) error {
 	if resp.Header.Get("Content-Type") == "application/json" {
-		var errorsResp struct {
-			Error string `json:"error"`
-		}
-		if json.NewDecoder(resp.Body).Decode(&errorsResp) == nil && errorsResp.Error != "" {
-			return errors.Errorf("Failed to %s: %s", op, errorsResp.Error)
+		var errorsResp response
+		if json.NewDecoder(resp.Body).Decode(&errorsResp) == nil {
+			if err := errorsResp.checkErrors(); err != nil {
+				return errors.Errorf("Failed to %s: %s", op, err)
+			}
 		}
 	}
 

tunnelstore/client_test.go

@@ -30,8 +30,8 @@ func TestDNSRouteUnmarshalResult(t *testing.T) {
 	badJSON := []string{
 		`abc`,
 		`{"success": false, "result": {"cname": "new"}}`,
-		`{"errors": ["foo"], "result": {"cname": "new"}}`,
-		`{"errors": ["foo", "bar"], "result": {"cname": "new"}}`,
+		`{"errors": [{"code": 1003, "message":"An A, AAAA or CNAME record already exists with that host"}], "result": {"cname": "new"}}`,
+		`{"errors": [{"code": 1003, "message":"An A, AAAA or CNAME record already exists with that host"}, {"code": 1004, "message":"Cannot use tunnel as origin for non-proxied load balancer"}], "result": {"cname": "new"}}`,
 		`{"result": {"cname": "new"}}`,
 		`{"result": {"cname": "new"}}`,
 	}
@@ -60,8 +60,8 @@ func TestLBRouteUnmarshalResult(t *testing.T) {
 	badJSON := []string{
 		`abc`,
 		`{"success": false, "result": {"pool": "unchanged", "load_balancer": "updated"}}`,
-		`{"errors": ["foo"], "result": {"pool": "unchanged", "load_balancer": "updated"}}`,
-		`{"errors": ["foo", "bar"], "result": {"pool": "unchanged", "load_balancer": "updated"}}`,
+		`{"errors": [{"code": 1003, "message":"An A, AAAA or CNAME record already exists with that host"}], "result": {"pool": "unchanged", "load_balancer": "updated"}}`,
+		`{"errors": [{"code": 1003, "message":"An A, AAAA or CNAME record already exists with that host"}, {"code": 1004, "message":"Cannot use tunnel as origin for non-proxied load balancer"}], "result": {"pool": "unchanged", "load_balancer": "updated"}}`,
 		`{"result": {"pool": "unchanged", "load_balancer": "updated"}}`,
 	}
 
@@ -127,7 +127,7 @@ func Test_parseListTunnels(t *testing.T) {
 		},
 		{
 			name:    "errors are present",
-			args:    args{body: `{"errors": ["foo"], "result": []}`},
+			args:    args{body: `{"errors": [{"code": 1003, "message":"An A, AAAA or CNAME record already exists with that host"}], "result": []}`},
 			wantErr: true,
 		},
 		{
@@ -197,7 +197,7 @@ func TestUnmarshalTunnelErr(t *testing.T) {
 		`abc`,
 		`{"success": true, "result": abc}`,
 		`{"success": false, "result": {"id": "00000000-0000-0000-0000-000000000000","name":"test","created_at":"0001-01-01T00:00:00Z","connections":[]}}}`,
-		`{"errors": ["foo"], "result": {"id": "00000000-0000-0000-0000-000000000000","name":"test","created_at":"0001-01-01T00:00:00Z","connections":[]}}}`,
+		`{"errors": [{"code": 1003, "message":"An A, AAAA or CNAME record already exists with that host"}], "result": {"id": "00000000-0000-0000-0000-000000000000","name":"test","created_at":"0001-01-01T00:00:00Z","connections":[]}}}`,
 	}
 
 	for i, test := range tests {