From 4dfc1ca25e7e875197503a1ec65b6155a6c84e94 Mon Sep 17 00:00:00 2001 From: Zachary Wasserman Date: Wed, 9 May 2018 16:54:23 -0700 Subject: [PATCH] Improve client error messages with unexpected server errors (#1776) --- server/service/client_errors.go | 27 +++++++++++++++++++++++++++ server/service/client_labels.go | 26 ++++++++++++++++++++------ server/service/client_packs.go | 26 ++++++++++++++++++++------ server/service/client_queries.go | 26 ++++++++++++++++++++------ server/service/client_sessions.go | 13 ++++++++++--- server/service/client_setup.go | 7 +++++++ 6 files changed, 104 insertions(+), 21 deletions(-) diff --git a/server/service/client_errors.go b/server/service/client_errors.go index 8fefb5637e..2debc10a59 100644 --- a/server/service/client_errors.go +++ b/server/service/client_errors.go @@ -1,5 +1,10 @@ package service +import ( + "encoding/json" + "io" +) + type SetupAlreadyErr interface { SetupAlready() bool Error() string @@ -59,3 +64,25 @@ func (e notFoundErr) Error() string { func (n notFoundErr) NotFound() bool { return true } + +type serverError struct { + Message string `json:"message"` + Errors []struct { + Name string `json:"name"` + Reason string `json:"reason"` + } `json:"errors"` +} + +func extractServerErrorText(body io.Reader) string { + var serverErr serverError + if err := json.NewDecoder(body).Decode(&serverErr); err != nil { + return "unknown" + } + + errText := serverErr.Message + if len(serverErr.Errors) > 0 { + errText += ": " + serverErr.Errors[0].Reason + } + + return errText +} diff --git a/server/service/client_labels.go b/server/service/client_labels.go index 4700f68856..4cdba3618d 100644 --- a/server/service/client_labels.go +++ b/server/service/client_labels.go @@ -20,7 +20,11 @@ func (c *Client) ApplyLabels(specs []*kolide.LabelSpec) error { defer response.Body.Close() if response.StatusCode != http.StatusOK { - return errors.Errorf("apply label spec got HTTP %d, expected 200", response.StatusCode) + return errors.Errorf( + "apply labels received status %d %s", + response.StatusCode, + extractServerErrorText(response.Body), + ) } var responseBody applyLabelSpecsResponse @@ -49,9 +53,12 @@ func (c *Client) GetLabel(name string) (*kolide.LabelSpec, error) { case http.StatusNotFound: return nil, notFoundErr{} } - if response.StatusCode != http.StatusOK { - return nil, errors.Errorf("get label spec got HTTP %d, expected 200", response.StatusCode) + return nil, errors.Errorf( + "get label received status %d %s", + response.StatusCode, + extractServerErrorText(response.Body), + ) } var responseBody getLabelSpecResponse @@ -76,7 +83,11 @@ func (c *Client) GetLabels() ([]*kolide.LabelSpec, error) { defer response.Body.Close() if response.StatusCode != http.StatusOK { - return nil, errors.Errorf("get label spec got HTTP %d, expected 200", response.StatusCode) + return nil, errors.Errorf( + "get labels received status %d %s", + response.StatusCode, + extractServerErrorText(response.Body), + ) } var responseBody getLabelSpecsResponse @@ -105,9 +116,12 @@ func (c *Client) DeleteLabel(name string) error { case http.StatusNotFound: return notFoundErr{} } - if response.StatusCode != http.StatusOK { - return errors.Errorf("get label spec got HTTP %d, expected 200", response.StatusCode) + return errors.Errorf( + "delete label received status %d %s", + response.StatusCode, + extractServerErrorText(response.Body), + ) } var responseBody deleteLabelResponse diff --git a/server/service/client_packs.go b/server/service/client_packs.go index 5c756b6eca..35791f9ae6 100644 --- a/server/service/client_packs.go +++ b/server/service/client_packs.go @@ -20,7 +20,11 @@ func (c *Client) ApplyPacks(specs []*kolide.PackSpec) error { defer response.Body.Close() if response.StatusCode != http.StatusOK { - return errors.Errorf("apply pack spec got HTTP %d, expected 200", response.StatusCode) + return errors.Errorf( + "apply packs received status %d %s", + response.StatusCode, + extractServerErrorText(response.Body), + ) } var responseBody applyPackSpecsResponse @@ -49,9 +53,12 @@ func (c *Client) GetPack(name string) (*kolide.PackSpec, error) { case http.StatusNotFound: return nil, notFoundErr{} } - if response.StatusCode != http.StatusOK { - return nil, errors.Errorf("get pack spec got HTTP %d, expected 200", response.StatusCode) + return nil, errors.Errorf( + "get pack received status %d %s", + response.StatusCode, + extractServerErrorText(response.Body), + ) } var responseBody getPackSpecResponse @@ -76,7 +83,11 @@ func (c *Client) GetPacks() ([]*kolide.PackSpec, error) { defer response.Body.Close() if response.StatusCode != http.StatusOK { - return nil, errors.Errorf("get pack spec got HTTP %d, expected 200", response.StatusCode) + return nil, errors.Errorf( + "get packs received status %d %s", + response.StatusCode, + extractServerErrorText(response.Body), + ) } var responseBody getPackSpecsResponse @@ -105,9 +116,12 @@ func (c *Client) DeletePack(name string) error { case http.StatusNotFound: return notFoundErr{} } - if response.StatusCode != http.StatusOK { - return errors.Errorf("get pack spec got HTTP %d, expected 200", response.StatusCode) + return errors.Errorf( + "delete pack received status %d %s", + response.StatusCode, + extractServerErrorText(response.Body), + ) } var responseBody deletePackResponse diff --git a/server/service/client_queries.go b/server/service/client_queries.go index 14917a4766..e72d7f45cc 100644 --- a/server/service/client_queries.go +++ b/server/service/client_queries.go @@ -20,7 +20,11 @@ func (c *Client) ApplyQueries(specs []*kolide.QuerySpec) error { defer response.Body.Close() if response.StatusCode != http.StatusOK { - return errors.Errorf("apply query spec got HTTP %d, expected 200", response.StatusCode) + return errors.Errorf( + "apply queries received status %d %s", + response.StatusCode, + extractServerErrorText(response.Body), + ) } var responseBody applyQuerySpecsResponse @@ -49,9 +53,12 @@ func (c *Client) GetQuery(name string) (*kolide.QuerySpec, error) { case http.StatusNotFound: return nil, notFoundErr{} } - if response.StatusCode != http.StatusOK { - return nil, errors.Errorf("get query spec got HTTP %d, expected 200", response.StatusCode) + return nil, errors.Errorf( + "get query received status %d %s", + response.StatusCode, + extractServerErrorText(response.Body), + ) } var responseBody getQuerySpecResponse @@ -76,7 +83,11 @@ func (c *Client) GetQueries() ([]*kolide.QuerySpec, error) { defer response.Body.Close() if response.StatusCode != http.StatusOK { - return nil, errors.Errorf("get query spec got HTTP %d, expected 200", response.StatusCode) + return nil, errors.Errorf( + "get queries received status %d %s", + response.StatusCode, + extractServerErrorText(response.Body), + ) } var responseBody getQuerySpecsResponse @@ -105,9 +116,12 @@ func (c *Client) DeleteQuery(name string) error { case http.StatusNotFound: return notFoundErr{} } - if response.StatusCode != http.StatusOK { - return errors.Errorf("get query spec got HTTP %d, expected 200", response.StatusCode) + return errors.Errorf( + "delete query received status %d %s", + response.StatusCode, + extractServerErrorText(response.Body), + ) } var responseBody deleteQueryResponse diff --git a/server/service/client_sessions.go b/server/service/client_sessions.go index bf96de64b1..d4b52d64a0 100644 --- a/server/service/client_sessions.go +++ b/server/service/client_sessions.go @@ -27,9 +27,12 @@ func (c *Client) Login(email, password string) (string, error) { case http.StatusUnauthorized: return "", invalidLoginErr{} } - if response.StatusCode != http.StatusOK { - return "", errors.Errorf("login got HTTP %d, expected 200", response.StatusCode) + return "", errors.Errorf( + "login received status %d %s", + response.StatusCode, + extractServerErrorText(response.Body), + ) } var responseBody loginResponse @@ -54,7 +57,11 @@ func (c *Client) Logout() error { defer response.Body.Close() if response.StatusCode != http.StatusOK { - return errors.Errorf("logout got HTTP %d, expected 200", response.StatusCode) + return errors.Errorf( + "logout received status %d %s", + response.StatusCode, + extractServerErrorText(response.Body), + ) } var responseBody logoutResponse diff --git a/server/service/client_setup.go b/server/service/client_setup.go index a5fd69ed19..f6e3166588 100644 --- a/server/service/client_setup.go +++ b/server/service/client_setup.go @@ -36,6 +36,13 @@ func (c *Client) Setup(email, password, org string) (string, error) { if response.StatusCode == http.StatusNotFound { return "", setupAlreadyErr{} } + if response.StatusCode != http.StatusOK { + return "", errors.Errorf( + "setup received status %d %s", + response.StatusCode, + extractServerErrorText(response.Body), + ) + } if response.StatusCode != http.StatusOK { return "", errors.Errorf("setup got HTTP %d, expected 200", response.StatusCode)