From 5e3e7cd03dd58118c0dca99cde84b236ce711aef Mon Sep 17 00:00:00 2001 From: Ruslan Efanov Date: Wed, 26 Oct 2022 18:07:04 +0300 Subject: [PATCH] fix review --- v1/client.go | 36 ++++++++++++++++-------------------- v1/client_test.go | 6 +----- v1/errors.go | 46 ++++++++++++++++++++++------------------------ v1/errors_test.go | 16 ++++++++++++++-- v1/helpers.go | 3 +-- 5 files changed, 54 insertions(+), 53 deletions(-) diff --git a/v1/client.go b/v1/client.go index c7a9b24..619ae86 100644 --- a/v1/client.go +++ b/v1/client.go @@ -71,7 +71,7 @@ func (c *MgClient) TransportTemplates() ([]Template, int, error) { } if status > http.StatusCreated || status < http.StatusOK { - return resp, status, c.Error(data) + return resp, status, NewAPIClientError(data) } return resp, status, err @@ -116,7 +116,7 @@ func (c *MgClient) ActivateTemplate(channelID uint64, request ActivateTemplateRe } if status > http.StatusCreated || status < http.StatusOK { - return status, c.Error(data) + return status, NewAPIClientError(data) } return status, err @@ -165,7 +165,7 @@ func (c *MgClient) UpdateTemplate(request Template) (int, error) { } if status != http.StatusOK { - return status, c.Error(data) + return status, NewAPIClientError(data) } return status, err @@ -190,7 +190,7 @@ func (c *MgClient) DeactivateTemplate(channelID uint64, templateCode string) (in } if status > http.StatusCreated || status < http.StatusOK { - return status, c.Error(data) + return status, NewAPIClientError(data) } return status, err @@ -224,7 +224,7 @@ func (c *MgClient) TransportChannels(request Channels) ([]ChannelListItem, int, } if status > http.StatusCreated || status < http.StatusOK { - return resp, status, c.Error(data) + return resp, status, NewAPIClientError(data) } return resp, status, err @@ -283,7 +283,7 @@ func (c *MgClient) ActivateTransportChannel(request Channel) (ActivateResponse, } if status > http.StatusCreated || status < http.StatusOK { - return resp, status, c.Error(data) + return resp, status, NewAPIClientError(data) } return resp, status, err @@ -342,7 +342,7 @@ func (c *MgClient) UpdateTransportChannel(request Channel) (UpdateResponse, int, } if status != http.StatusOK { - return resp, status, c.Error(data) + return resp, status, NewAPIClientError(data) } return resp, status, err @@ -378,7 +378,7 @@ func (c *MgClient) DeactivateTransportChannel(id uint64) (DeleteResponse, int, e } if status != http.StatusOK { - return resp, status, c.Error(data) + return resp, status, NewAPIClientError(data) } return resp, status, err @@ -427,7 +427,7 @@ func (c *MgClient) Messages(request SendData) (MessagesResponse, int, error) { } if status != http.StatusOK { - return resp, status, c.Error(data) + return resp, status, NewAPIClientError(data) } return resp, status, err @@ -471,7 +471,7 @@ func (c *MgClient) UpdateMessages(request EditMessageRequest) (MessagesResponse, } if status != http.StatusOK { - return resp, status, c.Error(data) + return resp, status, NewAPIClientError(data) } return resp, status, err @@ -510,7 +510,7 @@ func (c *MgClient) MarkMessageRead(request MarkMessageReadRequest) (MarkMessageR } if status != http.StatusOK { - return resp, status, c.Error(data) + return resp, status, NewAPIClientError(data) } return resp, status, err @@ -541,7 +541,7 @@ func (c *MgClient) AckMessage(request AckMessageRequest) (int, error) { } if status != http.StatusOK { - return status, c.Error(data) + return status, NewAPIClientError(data) } return status, err @@ -579,7 +579,7 @@ func (c *MgClient) DeleteMessage(request DeleteData) (*MessagesResponse, int, er return nil, status, err } if status != http.StatusOK { - return nil, status, c.Error(data) + return nil, status, NewAPIClientError(data) } var previousChatMessage *MessagesResponse @@ -618,7 +618,7 @@ func (c *MgClient) GetFile(request string) (FullFileResponse, int, error) { } if status != http.StatusOK { - return resp, status, c.Error(data) + return resp, status, NewAPIClientError(data) } return resp, status, err @@ -638,7 +638,7 @@ func (c *MgClient) UploadFile(request io.Reader) (UploadFileResponse, int, error } if status != http.StatusOK { - return resp, status, c.Error(data) + return resp, status, NewAPIClientError(data) } return resp, status, err @@ -659,16 +659,12 @@ func (c *MgClient) UploadFileByURL(request UploadFileByUrlRequest) (UploadFileRe } if status != http.StatusOK { - return resp, status, c.Error(data) + return resp, status, NewAPIClientError(data) } return resp, status, err } -func (c *MgClient) Error(info []byte) error { - return NewAPIClientError(info) -} - // MakeTimestamp returns current unix timestamp. func MakeTimestamp() int64 { return time.Now().UnixNano() / (int64(time.Millisecond) / int64(time.Nanosecond)) diff --git a/v1/client_test.go b/v1/client_test.go index 5317f75..a7dc299 100644 --- a/v1/client_test.go +++ b/v1/client_test.go @@ -723,10 +723,6 @@ func (t *MGClientTest) Test_UploadFile() { func (t *MGClientTest) Test_SuccessHandleError() { client := t.client() json := `{"errors": ["Channel not found"]}` - handleError := client.Error([]byte(json)) - - t.Assert().IsType(new(httpClientError), handleError) - t.Assert().Equal(handleError.Error(), "Channel not found") defer gock.Off() t.gock(). @@ -745,7 +741,7 @@ func (t *MGClientTest) Test_SuccessHandleError() { t.Assert().Equal(internalServerError, err.Error()) var serverErr *httpClientError if errors.As(err, &serverErr) { - t.Assert().NotNil(serverErr.LimitedResponse) + t.Assert().Nil(serverErr.Response) } else { t.Fail("Unexpected type of error") } diff --git a/v1/errors.go b/v1/errors.go index 0cbed00..08ca6ea 100644 --- a/v1/errors.go +++ b/v1/errors.go @@ -9,28 +9,29 @@ import ( "net/http" ) -var defaultErrorMessage = "internal http client error" +var defaultErrorMessage = "http client error" var internalServerError = "internal server error" +var marshalError = "cannot unmarshal response body" + +type MGErrors struct { + Errors []string +} type httpClientError struct { - ErrorMsg string - BaseError error - LimitedResponse io.Reader + ErrorMsg string + BaseError error + Response io.Reader } func (err *httpClientError) Unwrap() error { return err.BaseError } -func (err *httpClientError) Is(target error) bool { - return errors.As(target, &err) -} - func (err *httpClientError) Error() string { message := defaultErrorMessage if err.BaseError != nil { - message = fmt.Sprintf("%s - %s", defaultErrorMessage, err.BaseError.Error()) + message = fmt.Sprintf("%s: %s", defaultErrorMessage, err.BaseError.Error()) } if len(err.ErrorMsg) > 0 { @@ -45,35 +46,32 @@ func NewCriticalHTTPError(err error) error { } func NewAPIClientError(responseBody []byte) error { - var data map[string]interface{} + var data MGErrors var message string if len(responseBody) == 0 { message = internalServerError } else { - if err := json.Unmarshal(responseBody, &data); err != nil { - return err - } + err := json.Unmarshal(responseBody, &data) - values := data["errors"].([]interface{}) - message = values[0].(string) + if err != nil { + message = marshalError + } else if len(data.Errors) > 0 { + message = data.Errors[0] + } } return &httpClientError{ErrorMsg: message} } func NewServerError(response *http.Response) error { - var data []byte - body, err := buildLimitedRawResponse(response) - if err == nil { - data = body - } - - err = NewAPIClientError(data) var serverError *httpClientError - if errors.As(err, &serverError) { - serverError.LimitedResponse = bytes.NewBuffer(body) + body, _ := buildLimitedRawResponse(response) + err := NewAPIClientError(body) + + if errors.As(err, &serverError) && len(body) > 0 { + serverError.Response = bytes.NewBuffer(body) return serverError } diff --git a/v1/errors_test.go b/v1/errors_test.go index 6b7edcc..924b469 100644 --- a/v1/errors_test.go +++ b/v1/errors_test.go @@ -19,7 +19,7 @@ func TestNewCriticalHTTPError(t *testing.T) { assert.IsType(t, new(httpClientError), httpErr) assert.IsType(t, new(url.Error), errors.Unwrap(httpErr)) assert.IsType(t, new(url.Error), errors.Unwrap(httpErr)) - assert.Equal(t, httpErr.Error(), fmt.Sprintf("%s - %s", defaultErrorMessage, err.Error())) + assert.Equal(t, httpErr.Error(), fmt.Sprintf("%s: %s", defaultErrorMessage, err.Error())) } func TestNewApiClientError(t *testing.T) { @@ -47,8 +47,20 @@ func TestNewServerError(t *testing.T) { var err *httpClientError if errors.As(serverErr, &err) { - assert.NotNil(t, err.LimitedResponse) + assert.NotNil(t, err.Response) } else { t.Fatal("Unexpected type of error") } + + body = []byte(`{"invalid_json"`) + response = new(http.Response) + response.Body = io.NopCloser(bytes.NewReader(body)) + serverErr = NewServerError(response) + + assert.IsType(t, new(httpClientError), serverErr) + assert.Equal(t, serverErr.Error(), marshalError) + + if errors.As(serverErr, &err) { + assert.NotNil(t, err.Response) + } } diff --git a/v1/helpers.go b/v1/helpers.go index b6ece4c..67ea32c 100644 --- a/v1/helpers.go +++ b/v1/helpers.go @@ -6,12 +6,11 @@ import ( ) const MB = 1 << 20 -const MaxSizeBody = MB * 0.5 func buildLimitedRawResponse(resp *http.Response) ([]byte, error) { defer resp.Body.Close() - limitReader := io.LimitReader(resp.Body, MaxSizeBody) + limitReader := io.LimitReader(resp.Body, MB) body, err := io.ReadAll(limitReader) if err != nil {