TUN-3443: Decode as v4api response on non-200 status
This commit is contained in:
parent
be7b7c7149
commit
812244d79f
|
@ -376,16 +376,12 @@ func (r *RESTClient) sendRequest(method string, url url.URL, body interface{}) (
|
||||||
func parseResponse(reader io.Reader, data interface{}) error {
|
func parseResponse(reader io.Reader, data interface{}) error {
|
||||||
// Schema for Tunnelstore responses in the v1 API.
|
// Schema for Tunnelstore responses in the v1 API.
|
||||||
// Roughly, it's a wrapper around a particular result that adds failures/errors/etc
|
// Roughly, it's a wrapper around a particular result that adds failures/errors/etc
|
||||||
var result struct {
|
var result response
|
||||||
Result json.RawMessage `json:"result"`
|
|
||||||
Success bool `json:"success"`
|
|
||||||
Errors []string `json:"errors"`
|
|
||||||
}
|
|
||||||
// First, parse the wrapper and check the API call succeeded
|
// First, parse the wrapper and check the API call succeeded
|
||||||
if err := json.NewDecoder(reader).Decode(&result); err != nil {
|
if err := json.NewDecoder(reader).Decode(&result); err != nil {
|
||||||
return errors.Wrap(err, "failed to decode response")
|
return errors.Wrap(err, "failed to decode response")
|
||||||
}
|
}
|
||||||
if err := checkErrors(result.Errors); err != nil {
|
if err := result.checkErrors(); err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
if !result.Success {
|
if !result.Success {
|
||||||
|
@ -402,24 +398,43 @@ func unmarshalTunnel(reader io.Reader) (*Tunnel, error) {
|
||||||
return &tunnel, err
|
return &tunnel, err
|
||||||
}
|
}
|
||||||
|
|
||||||
func checkErrors(errs []string) error {
|
type response struct {
|
||||||
if len(errs) == 0 {
|
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
|
return nil
|
||||||
}
|
}
|
||||||
if len(errs) == 1 {
|
if len(r.Errors) == 1 {
|
||||||
return fmt.Errorf("API error: %s", errs[0])
|
return r.Errors[0]
|
||||||
}
|
}
|
||||||
allErrs := strings.Join(errs, "; ")
|
var messages string
|
||||||
return fmt.Errorf("API errors: %s", allErrs)
|
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 {
|
func (r *RESTClient) statusCodeToError(op string, resp *http.Response) error {
|
||||||
if resp.Header.Get("Content-Type") == "application/json" {
|
if resp.Header.Get("Content-Type") == "application/json" {
|
||||||
var errorsResp struct {
|
var errorsResp response
|
||||||
Error string `json:"error"`
|
if json.NewDecoder(resp.Body).Decode(&errorsResp) == nil {
|
||||||
}
|
if err := errorsResp.checkErrors(); err != nil {
|
||||||
if json.NewDecoder(resp.Body).Decode(&errorsResp) == nil && errorsResp.Error != "" {
|
return errors.Errorf("Failed to %s: %s", op, err)
|
||||||
return errors.Errorf("Failed to %s: %s", op, errorsResp.Error)
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -30,8 +30,8 @@ func TestDNSRouteUnmarshalResult(t *testing.T) {
|
||||||
badJSON := []string{
|
badJSON := []string{
|
||||||
`abc`,
|
`abc`,
|
||||||
`{"success": false, "result": {"cname": "new"}}`,
|
`{"success": false, "result": {"cname": "new"}}`,
|
||||||
`{"errors": ["foo"], "result": {"cname": "new"}}`,
|
`{"errors": [{"code": 1003, "message":"An A, AAAA or CNAME record already exists with that host"}], "result": {"cname": "new"}}`,
|
||||||
`{"errors": ["foo", "bar"], "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"}}`,
|
||||||
`{"result": {"cname": "new"}}`,
|
`{"result": {"cname": "new"}}`,
|
||||||
}
|
}
|
||||||
|
@ -60,8 +60,8 @@ func TestLBRouteUnmarshalResult(t *testing.T) {
|
||||||
badJSON := []string{
|
badJSON := []string{
|
||||||
`abc`,
|
`abc`,
|
||||||
`{"success": false, "result": {"pool": "unchanged", "load_balancer": "updated"}}`,
|
`{"success": false, "result": {"pool": "unchanged", "load_balancer": "updated"}}`,
|
||||||
`{"errors": ["foo"], "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": ["foo", "bar"], "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"}}`,
|
`{"result": {"pool": "unchanged", "load_balancer": "updated"}}`,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -127,7 +127,7 @@ func Test_parseListTunnels(t *testing.T) {
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "errors are present",
|
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,
|
wantErr: true,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
|
@ -197,7 +197,7 @@ func TestUnmarshalTunnelErr(t *testing.T) {
|
||||||
`abc`,
|
`abc`,
|
||||||
`{"success": true, "result": abc}`,
|
`{"success": true, "result": abc}`,
|
||||||
`{"success": false, "result": {"id": "00000000-0000-0000-0000-000000000000","name":"test","created_at":"0001-01-01T00:00:00Z","connections":[]}}}`,
|
`{"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 {
|
for i, test := range tests {
|
||||||
|
|
Loading…
Reference in New Issue