Skip to content

Commit

Permalink
Do not allocate empty response in case of error
Browse files Browse the repository at this point in the history
Doc string states that it's return response in case of success or an error in case of error
  • Loading branch information
moredure committed Apr 2, 2022
1 parent 333c2dc commit c23618a
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 3 deletions.
26 changes: 23 additions & 3 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,17 +179,37 @@ func (c *Client) PushWithContext(ctx Context, n *Notification) (*Response, error
}
defer httpRes.Body.Close()

apnsId := httpRes.Header.Get("apns-id")

response := &Response{}
response.StatusCode = httpRes.StatusCode
response.ApnsID = httpRes.Header.Get("apns-id")
response.ApnsID = apnsId

decoder := json.NewDecoder(httpRes.Body)
if err := decoder.Decode(&response); err != nil && err != io.EOF {
return &Response{}, err
if err := decoder.Decode(response); err != nil && err != io.EOF {
return nil, &APNSError{
Err: err,
StatusCode: httpRes.StatusCode,
ApnsID: apnsId,
}
}
return response, nil
}

type APNSError struct {
Err error
StatusCode int
ApnsID string
}

func (e *APNSError) Unwrap() error {
return e.Err
}

func (e *APNSError) Error() string {
return e.Err.Error()
}

// CloseIdleConnections closes any underlying connections which were previously
// connected from previous requests but are now sitting idle. It will not
// interrupt any connections currently in use.
Expand Down
33 changes: 33 additions & 0 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"crypto/tls"
"errors"
"fmt"
"io"
"io/ioutil"
"net"
"net/http"
Expand Down Expand Up @@ -403,9 +404,41 @@ func TestMalformedJSONResponse(t *testing.T) {
defer server.Close()
res, err := mockClient(server.URL).Push(n)
assert.Error(t, err)
assert.Nil(t, res)
assert.Equal(t, false, res.Sent())
}

func TestMalformedJSONResponseWithHeaders(t *testing.T) {
n := mockNotification()
var apnsID = "02ABC856-EF8D-4E49-8F15-7B8A61D978D6"
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json; charset=utf-8")
w.Header().Set("apns-id", apnsID)
w.WriteHeader(http.StatusBadGateway)
w.Write([]byte("{{MalformedJSON}}"))
}))
defer server.Close()
res, err := mockClient(server.URL).Push(n)
assert.Error(t, err)
var apns *apns.APNSError
assert.ErrorAs(t, err, &apns)
assert.Equal(t, apns.ApnsID, apnsID)
assert.Equal(t, apns.StatusCode, http.StatusBadGateway)
assert.Nil(t, res)
assert.Equal(t, false, res.Sent())
}

func TestAPNSError(t *testing.T) {
var err error = &apns.APNSError{
Err: io.EOF,
}
assert.Error(t, err)
var apnsErr *apns.APNSError
assert.ErrorAs(t, err, &apnsErr)
assert.ErrorIs(t, err, io.EOF)
assert.Equal(t, err.Error(), io.EOF.Error())
}

func TestCloseIdleConnections(t *testing.T) {
transport := &mockTransport{}

Expand Down
3 changes: 3 additions & 0 deletions response.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,9 @@ type Response struct {
// Sent returns whether or not the notification was successfully sent.
// This is the same as checking if the StatusCode == 200.
func (c *Response) Sent() bool {
if c == nil {
return false
}
return c.StatusCode == StatusSent
}

Expand Down

0 comments on commit c23618a

Please sign in to comment.