Skip to content

Commit 15330f1

Browse files
committed
Merge pull request #4134 from Joshua-Anderson/better-errors
feat(client-go): decode json error messages
2 parents cd9eff0 + 7900270 commit 15330f1

13 files changed

Lines changed: 143 additions & 219 deletions

File tree

client-go/controller/client/auth.go

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -129,16 +129,10 @@ func Passwd(username string, password string, newPassword string) error {
129129
return err
130130
}
131131

132-
resBody, status, err := client.BasicRequest("POST", "/v1/auth/passwd/", body)
133-
134-
if err != nil {
132+
if _, err = client.BasicRequest("POST", "/v1/auth/passwd/", body); err != nil {
135133
return err
136134
}
137135

138-
if status != 200 {
139-
return fmt.Errorf("Password change failed: %s", resBody)
140-
}
141-
142136
fmt.Println("Password change succeeded.")
143137
return nil
144138
}
@@ -151,10 +145,8 @@ func Cancel() error {
151145
return err
152146
}
153147

154-
body, status, err := client.BasicRequest("DELETE", "/v1/auth/cancel/", nil)
155-
156-
if status != 204 {
157-
return fmt.Errorf("Cancellation failed: %s", body)
148+
if _, err = client.BasicRequest("DELETE", "/v1/auth/cancel/", nil); err != nil {
149+
return err
158150
}
159151

160152
if err = deleteSettings(); err != nil {
@@ -187,16 +179,12 @@ func Regenerate(username string, all bool) error {
187179
return err
188180
}
189181

190-
resBody, status, err := client.BasicRequest("POST", "/v1/auth/tokens/", body)
182+
resBody, err := client.BasicRequest("POST", "/v1/auth/tokens/", body)
191183

192184
if err != nil {
193185
return err
194186
}
195187

196-
if status != 200 {
197-
return fmt.Errorf("Token regeneration failed: %s", resBody)
198-
}
199-
200188
// If the token regenerated is the current user's, update it.
201189
if username == "" && all == false {
202190
token := api.AuthRegenerateResponse{}

client-go/controller/client/http.go

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@ package client
33
import (
44
"bytes"
55
"crypto/tls"
6+
"encoding/json"
67
"errors"
78
"fmt"
89
"io"
910
"io/ioutil"
1011
"net/http"
1112
"net/url"
13+
"reflect"
1214
"strings"
1315

1416
"github.com/deis/deis/version"
@@ -41,7 +43,7 @@ func rawRequest(client *http.Client, method string, url string, body io.Reader,
4143
if err != nil {
4244
return nil, err
4345
}
44-
return nil, errors.New(string(resBody))
46+
return nil, checkForErrors(res, string(resBody))
4547
}
4648

4749
return res, nil
@@ -81,20 +83,57 @@ func (c Client) Request(method string, path string, body []byte) (*http.Response
8183
}
8284

8385
// BasicRequest makes a simple http request on the controller.
84-
func (c Client) BasicRequest(method string, path string, body []byte) (string, int, error) {
86+
func (c Client) BasicRequest(method string, path string, body []byte) (string, error) {
8587
res, err := c.Request(method, path, body)
8688

8789
if err != nil {
88-
return "", -1, err
90+
return "", err
8991
}
9092
defer res.Body.Close()
9193

9294
resBody, err := ioutil.ReadAll(res.Body)
9395

9496
if err != nil {
95-
return "", -1, err
97+
return "", err
9698
}
97-
return string(resBody), res.StatusCode, nil
99+
return string(resBody), checkForErrors(res, string(resBody))
100+
}
101+
102+
func checkForErrors(res *http.Response, body string) error {
103+
switch res.StatusCode {
104+
case http.StatusOK, http.StatusCreated, http.StatusNoContent:
105+
return nil
106+
}
107+
108+
bodyMap := make(map[string]interface{})
109+
110+
if err := json.Unmarshal([]byte(body), &bodyMap); err != nil {
111+
return err
112+
}
113+
114+
errorMessage := "\n"
115+
for key, value := range bodyMap {
116+
switch v := value.(type) {
117+
case string:
118+
errorMessage += key + ": " + v + "\n"
119+
case []interface{}:
120+
for _, subValue := range v {
121+
switch sv := subValue.(type) {
122+
case string:
123+
errorMessage += key + ": " + sv + "\n"
124+
default:
125+
fmt.Printf("Unexpected type in %s error message array. Contents: %v",
126+
reflect.TypeOf(value), sv)
127+
}
128+
}
129+
default:
130+
fmt.Printf("Cannot handle key %s in error message, type %s. Contents: %v",
131+
key, reflect.TypeOf(value), bodyMap[key])
132+
}
133+
}
134+
135+
errorMessage += res.Status + "\n"
136+
return errors.New(errorMessage)
98137
}
99138

100139
// CheckConection checks that the user is connected to a network and the URL points to a valid controller.

client-go/controller/client/http_test.go

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -155,20 +155,73 @@ func TestBasicRequest(t *testing.T) {
155155

156156
client := Client{HTTPClient: httpClient, ControllerURL: *u, Token: "abc"}
157157

158-
body, status, err := client.BasicRequest("POST", "/basic/", []byte("test"))
158+
body, err := client.BasicRequest("POST", "/basic/", []byte("test"))
159159

160160
if err != nil {
161161
t.Fatal(err)
162162
}
163163

164-
expectedStatus := 200
165-
166-
if status != expectedStatus {
167-
t.Errorf("Expected %d, got %d", expectedStatus, status)
168-
}
169-
170164
expected := "basic"
171165
if body != expected {
172166
t.Errorf("Expected %s, Got %s", expected, body)
173167
}
174168
}
169+
170+
func TestCheckErrors(t *testing.T) {
171+
t.Parallel()
172+
173+
expected := `
174+
error: This is an error.
175+
error_array: This is an array.
176+
error_array: Foo!
177+
404 NOT FOUND
178+
`
179+
altExpected := `
180+
error_array: This is an array.
181+
error_array: Foo!
182+
error: This is an error.
183+
404 NOT FOUND
184+
`
185+
186+
body := `
187+
{
188+
"error": "This is an error.",
189+
"error_array": [
190+
"This is an array.",
191+
"Foo!"
192+
]
193+
}`
194+
195+
res := http.Response{
196+
StatusCode: 404,
197+
Status: "404 NOT FOUND",
198+
}
199+
200+
actual := checkForErrors(&res, body).Error()
201+
202+
if actual != expected && actual != altExpected {
203+
t.Errorf("Expected %s or %s, Got %s", expected, altExpected, actual)
204+
}
205+
}
206+
207+
func TestCheckErrorsReturnsNil(t *testing.T) {
208+
t.Parallel()
209+
210+
responses := []http.Response{
211+
http.Response{
212+
StatusCode: http.StatusOK,
213+
},
214+
http.Response{
215+
StatusCode: http.StatusCreated,
216+
},
217+
http.Response{
218+
StatusCode: http.StatusNoContent,
219+
},
220+
}
221+
222+
for _, res := range responses {
223+
if err := checkForErrors(&res, ""); err != nil {
224+
t.Fatal(err)
225+
}
226+
}
227+
}

client-go/controller/models/apps/apps.go

Lines changed: 7 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package apps
22

33
import (
44
"encoding/json"
5-
"errors"
65
"fmt"
76
"strconv"
87
"strings"
@@ -13,16 +12,12 @@ import (
1312

1413
// List lists apps on a Deis controller.
1514
func List(c *client.Client) ([]api.App, error) {
16-
body, status, err := c.BasicRequest("GET", "/v1/apps/", nil)
15+
body, err := c.BasicRequest("GET", "/v1/apps/", nil)
1716

1817
if err != nil {
1918
return []api.App{}, err
2019
}
2120

22-
if status != 200 {
23-
return []api.App{}, errors.New(body)
24-
}
25-
2621
apps := api.Apps{}
2722
if err = json.Unmarshal([]byte(body), &apps); err != nil {
2823
return []api.App{}, err
@@ -45,16 +40,12 @@ func New(c *client.Client, id string) (api.App, error) {
4540
}
4641
}
4742

48-
resBody, status, err := c.BasicRequest("POST", "/v1/apps/", body)
43+
resBody, err := c.BasicRequest("POST", "/v1/apps/", body)
4944

5045
if err != nil {
5146
return api.App{}, err
5247
}
5348

54-
if status != 201 {
55-
return api.App{}, errors.New(resBody)
56-
}
57-
5849
app := api.App{}
5950
if err = json.Unmarshal([]byte(resBody), &app); err != nil {
6051
return api.App{}, err
@@ -67,16 +58,12 @@ func New(c *client.Client, id string) (api.App, error) {
6758
func Get(c *client.Client, appID string) (api.App, error) {
6859
u := fmt.Sprintf("/v1/apps/%s/", appID)
6960

70-
body, status, err := c.BasicRequest("GET", u, nil)
61+
body, err := c.BasicRequest("GET", u, nil)
7162

7263
if err != nil {
7364
return api.App{}, err
7465
}
7566

76-
if status != 200 {
77-
return api.App{}, errors.New(body)
78-
}
79-
8067
app := api.App{}
8168

8269
if err = json.Unmarshal([]byte(body), &app); err != nil {
@@ -94,16 +81,12 @@ func Logs(c *client.Client, appID string, lines int) (string, error) {
9481
u += "?log_lines=" + strconv.Itoa(lines)
9582
}
9683

97-
body, status, err := c.BasicRequest("GET", u, nil)
84+
body, err := c.BasicRequest("GET", u, nil)
9885

9986
if err != nil {
10087
return "", err
10188
}
10289

103-
if status != 200 {
104-
return body, errors.New(body)
105-
}
106-
10790
return strings.Trim(body, `"`), nil
10891
}
10992

@@ -118,16 +101,12 @@ func Run(c *client.Client, appID string, command string) (api.AppRunResponse, er
118101

119102
u := fmt.Sprintf("/v1/apps/%s/run", appID)
120103

121-
resBody, status, err := c.BasicRequest("POST", u, body)
104+
resBody, err := c.BasicRequest("POST", u, body)
122105

123106
if err != nil {
124107
return api.AppRunResponse{}, err
125108
}
126109

127-
if status != 200 {
128-
return api.AppRunResponse{}, errors.New(resBody)
129-
}
130-
131110
out := make([]interface{}, 2)
132111

133112
if err = json.Unmarshal([]byte(resBody), &out); err != nil {
@@ -141,15 +120,6 @@ func Run(c *client.Client, appID string, command string) (api.AppRunResponse, er
141120
func Delete(c *client.Client, appID string) error {
142121
u := fmt.Sprintf("/v1/apps/%s/", appID)
143122

144-
body, status, err := c.BasicRequest("DELETE", u, nil)
145-
146-
if err != nil {
147-
return err
148-
}
149-
150-
if status != 204 {
151-
return errors.New(body)
152-
}
153-
154-
return nil
123+
_, err := c.BasicRequest("DELETE", u, nil)
124+
return err
155125
}

client-go/controller/models/builds/builds.go

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package builds
22

33
import (
44
"encoding/json"
5-
"errors"
65
"fmt"
76

87
"github.com/deis/deis/client-go/controller/api"
@@ -12,16 +11,12 @@ import (
1211
// List lists an app's builds.
1312
func List(c *client.Client, appID string) ([]api.Build, error) {
1413
u := fmt.Sprintf("/v1/apps/%s/builds/", appID)
15-
body, status, err := c.BasicRequest("GET", u, nil)
14+
body, err := c.BasicRequest("GET", u, nil)
1615

1716
if err != nil {
1817
return []api.Build{}, err
1918
}
2019

21-
if status != 200 {
22-
return []api.Build{}, errors.New(body)
23-
}
24-
2520
builds := api.Builds{}
2621
if err = json.Unmarshal([]byte(body), &builds); err != nil {
2722
return []api.Build{}, err
@@ -44,16 +39,12 @@ func New(c *client.Client, appID string, image string,
4439
return api.Build{}, err
4540
}
4641

47-
resBody, status, err := c.BasicRequest("POST", u, body)
42+
resBody, err := c.BasicRequest("POST", u, body)
4843

4944
if err != nil {
5045
return api.Build{}, err
5146
}
5247

53-
if status != 201 {
54-
return api.Build{}, errors.New(resBody)
55-
}
56-
5748
build := api.Build{}
5849
if err = json.Unmarshal([]byte(resBody), &build); err != nil {
5950
return api.Build{}, err

0 commit comments

Comments
 (0)