Skip to content

Commit

Permalink
Feature/improve error and requester (#44)
Browse files Browse the repository at this point in the history
* Remove pointer from responses

* Update unit tests

* Update integration tests

* Update examples

* Update customer struct

* Order struct fields

* Change int64 to int

* Switch params order

* Externalize Requester interface

* Improve internal tests

* Externalize response error
  • Loading branch information
gdeandradero authored Mar 18, 2024
1 parent 4e788fd commit ca46968
Show file tree
Hide file tree
Showing 10 changed files with 81 additions and 38 deletions.
1 change: 0 additions & 1 deletion .testcoverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,4 @@ threshold:

exclude:
paths:
- ^pkg/internal
- ^pkg/config
37 changes: 37 additions & 0 deletions examples/mp_error_parse/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package main

import (
"context"
"errors"
"fmt"

"github.com/mercadopago/sdk-go/pkg/config"
"github.com/mercadopago/sdk-go/pkg/mperror"
"github.com/mercadopago/sdk-go/pkg/payment"
)

func main() {
accessToken := "{{ACCESS_TOKEN}}"

cfg, err := config.New(accessToken)
if err != nil {
fmt.Println(err)
return
}

invalidRequest := payment.Request{}

client := payment.NewClient(cfg)
payment, err := client.Create(context.Background(), invalidRequest)
if err != nil {
var mpErr *mperror.ResponseError
if errors.As(err, &mpErr) {
fmt.Printf("\nheaders: %s\nmessage: %s\nstatus code: %d", mpErr.Headers, mpErr.Message, mpErr.StatusCode)
return
}
fmt.Println(err)
return
}

fmt.Println(payment)
}
5 changes: 3 additions & 2 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ package config
import (
"fmt"

"github.com/mercadopago/sdk-go/pkg/internal/requester"
"github.com/mercadopago/sdk-go/pkg/internal/defaultrequester"
"github.com/mercadopago/sdk-go/pkg/requester"
)

// Config allows you to send custom settings and API authentication
Expand All @@ -20,7 +21,7 @@ type Config struct {
func New(accessToken string, opts ...Option) (*Config, error) {
cfg := &Config{
AccessToken: accessToken,
Requester: requester.Default(),
Requester: defaultrequester.New(),
}

// Apply all the functional options to configure the client.
Expand Down
2 changes: 1 addition & 1 deletion pkg/config/config_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package config
import (
"fmt"

"github.com/mercadopago/sdk-go/pkg/internal/requester"
"github.com/mercadopago/sdk-go/pkg/requester"
)

type Option func(*Config) error
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package requester
package defaultrequester

import (
"context"
"io"
"net/http"
"time"

"github.com/mercadopago/sdk-go/pkg/requester"
)

var (
Expand All @@ -21,11 +23,6 @@ var (
defaultBackoffStrategy = constantBackoff(time.Second * 2)
)

// Requester has the minimum required method to send http requests.
type Requester interface {
Do(req *http.Request) (*http.Response, error)
}

// defaultRequester provides an immutable implementation of option.Requester.
type defaultRequester struct{}

Expand All @@ -34,8 +31,8 @@ type defaultRequester struct{}
// pass before trying again.
type backoffFunc func(attempt int) time.Duration

// Default return the default implementation of Requester interface.
func Default() Requester {
// New return the default implementation of Requester interface.
func New() requester.Requester {
return &defaultRequester{}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package requester
package defaultrequester

import (
"io"
Expand All @@ -8,13 +8,13 @@ import (
)

func TestDo(t *testing.T) {
server, req := NewRequestWithHTTPServerUnavailableMock()
server, req := newRequestWithHTTPServerUnavailableMock()
defer server.Close()

serverOK, reqOK := NewRequestWithHTTPServerOKMock()
serverOK, reqOK := newRequestWithHTTPServerOKMock()
defer serverOK.Close()

reqWithDeadline, cancel := NewRequestMockWithDeadlineContextAndServerError()
reqWithDeadline, cancel := newRequestMockWithDeadlineContextAndServerError()
defer cancel()

type args struct {
Expand Down Expand Up @@ -43,7 +43,7 @@ func TestDo(t *testing.T) {
{
name: "should_return_error_when_context_is_canceled",
args: args{
req: NewRequestMockWithCanceledContext(),
req: newRequestMockWithCanceledContext(),
},
wantErr: "context canceled",
},
Expand All @@ -57,21 +57,21 @@ func TestDo(t *testing.T) {
{
name: "should_return_error_when_retry_is_enabled_and_request_fails",
args: args{
req: NewRequestMock(),
req: newRequestMock(),
},
wantErr: "Get \"\": unsupported protocol scheme \"\"",
},
{
name: "should_return_error_when_request_is_nil",
args: args{
req: NewInvalidRequestMock(),
req: newInvalidRequestMock(),
},
wantErr: "error getting body",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
d := Default()
d := New()
got, err := d.Do(tt.args.req)

gotError := ""
Expand Down Expand Up @@ -108,14 +108,14 @@ func TestRequestFromInternal(t *testing.T) {
{
name: "should_copy_and_return_request_with_body",
args: args{
req: NewRequestMockWithBody(),
req: newRequestMockWithBody(),
},
want: "{id:1}",
},
{
name: "should_copy_and_return_request_with_body_nil",
args: args{
req: NewRequestMock(),
req: newRequestMock(),
},
want: "",
},
Expand Down Expand Up @@ -147,10 +147,10 @@ func TestRequestFromInternal(t *testing.T) {
}

func TestCloseResponseBody(t *testing.T) {
server, req := NewRequestWithHTTPServerOKMock()
server, req := newRequestWithHTTPServerOKMock()
defer server.Close()

s, reqWithResAndCancel, cancel := NewRequestWithHTTPServerUnavailableAndCanceledContext()
s, reqWithResAndCancel, cancel := newRequestWithHTTPServerUnavailableAndCanceledContext()
defer s.Close()
defer cancel()

Expand Down Expand Up @@ -192,7 +192,7 @@ func TestCloseResponseBody(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
d := Default()
d := New()
got, _ := d.Do(tt.args.req)

tt.args.close(got)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package requester
package defaultrequester

import (
"bytes"
Expand All @@ -10,19 +10,19 @@ import (
"time"
)

func NewRequestMock() *http.Request {
func newRequestMock() *http.Request {
req, _ := http.NewRequestWithContext(context.Background(), http.MethodGet, "", nil)

return req
}

func NewRequestMockWithBody() *http.Request {
func newRequestMockWithBody() *http.Request {
req, _ := http.NewRequest(http.MethodPost, "http://test", bytes.NewBuffer([]byte(`{id:1}`)))

return req
}

func NewInvalidRequestMock() *http.Request {
func newInvalidRequestMock() *http.Request {
req, _ := http.NewRequest(http.MethodGet, "http://test", nil)
req.GetBody = func() (io.ReadCloser, error) {
return nil, fmt.Errorf("error getting body")
Expand All @@ -31,7 +31,7 @@ func NewInvalidRequestMock() *http.Request {
return req
}

func NewRequestMockWithCanceledContext() *http.Request {
func newRequestMockWithCanceledContext() *http.Request {
ctx, cancel := context.WithTimeout(context.Background(), time.Second*1)
defer cancel()

Expand All @@ -40,14 +40,14 @@ func NewRequestMockWithCanceledContext() *http.Request {
return req
}

func NewRequestMockWithDeadlineContextAndServerError() (*http.Request, context.CancelFunc) {
func newRequestMockWithDeadlineContextAndServerError() (*http.Request, context.CancelFunc) {
ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(time.Second*7))
req, _ := http.NewRequestWithContext(ctx, http.MethodGet, "", nil)

return req, cancel
}

func NewRequestWithHTTPServerUnavailableMock() (*httptest.Server, *http.Request) {
func newRequestWithHTTPServerUnavailableMock() (*httptest.Server, *http.Request) {
s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusServiceUnavailable)
}))
Expand All @@ -57,7 +57,7 @@ func NewRequestWithHTTPServerUnavailableMock() (*httptest.Server, *http.Request)
return s, request
}

func NewRequestWithHTTPServerOKMock() (*httptest.Server, *http.Request) {
func newRequestWithHTTPServerOKMock() (*httptest.Server, *http.Request) {
s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
// we should make this to pass in the lint pileline
Expand All @@ -69,7 +69,7 @@ func NewRequestWithHTTPServerOKMock() (*httptest.Server, *http.Request) {
return s, request
}

func NewRequestWithHTTPServerUnavailableAndCanceledContext() (*httptest.Server, *http.Request, context.CancelFunc) {
func newRequestWithHTTPServerUnavailableAndCanceledContext() (*httptest.Server, *http.Request, context.CancelFunc) {
ctx, cancel := context.WithTimeout(context.Background(), time.Second*1)
s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusServiceUnavailable)
Expand Down
7 changes: 4 additions & 3 deletions pkg/internal/httpclient/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import (
"io"
"net/http"

"github.com/mercadopago/sdk-go/pkg/internal/requester"
"github.com/mercadopago/sdk-go/pkg/mperror"
"github.com/mercadopago/sdk-go/pkg/requester"
)

func Send(requester requester.Requester, req *http.Request) ([]byte, error) {
Expand All @@ -18,15 +19,15 @@ func Send(requester requester.Requester, req *http.Request) ([]byte, error) {

response, err := io.ReadAll(result.Body)
if err != nil {
return nil, &ResponseError{
return nil, &mperror.ResponseError{
StatusCode: result.StatusCode,
Message: "error reading response body: " + err.Error(),
Headers: result.Header,
}
}

if result.StatusCode > 399 {
return nil, &ResponseError{
return nil, &mperror.ResponseError{
StatusCode: result.StatusCode,
Message: string(response),
Headers: result.Header,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package httpclient
package mperror

import "net/http"

Expand Down
8 changes: 8 additions & 0 deletions pkg/requester/requester.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package requester

import "net/http"

// Requester has the minimum required method to send http requests.
type Requester interface {
Do(req *http.Request) (*http.Response, error)
}

0 comments on commit ca46968

Please sign in to comment.