Skip to content

Commit

Permalink
Control http.Client creation instead of using http.DefaultClient
Browse files Browse the repository at this point in the history
  • Loading branch information
alanhamlett committed May 20, 2021
1 parent 5c6ee49 commit 2052676
Show file tree
Hide file tree
Showing 11 changed files with 56 additions and 41 deletions.
3 changes: 1 addition & 2 deletions cmd/legacy/heartbeat/heartbeat.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package heartbeat
import (
"errors"
"fmt"
"net/http"
"os"
"strings"

Expand Down Expand Up @@ -130,7 +129,7 @@ func SendHeartbeats(v *viper.Viper, queueFilepath string) error {
clientOpts = append(clientOpts, api.WithUserAgentUnknownPlugin())
}

c := api.NewClient(params.API.URL, http.DefaultClient, clientOpts...)
c := api.NewClient(params.API.URL, clientOpts...)

heartbeats := []heartbeat.Heartbeat{
heartbeat.New(
Expand Down
3 changes: 1 addition & 2 deletions cmd/legacy/today/today.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package today
import (
"errors"
"fmt"
"net/http"
"os"
"strings"
"time"
Expand Down Expand Up @@ -123,7 +122,7 @@ func Summary(v *viper.Viper) (string, error) {
url = params.API.URL
}

c := api.NewClient(url, http.DefaultClient, opts...)
c := api.NewClient(url, opts...)

now := time.Now()
todayStart := time.Date(now.Year(), now.Month(), now.Day(), 0, 0, 0, 0, now.Location())
Expand Down
3 changes: 1 addition & 2 deletions cmd/legacy/todaygoal/todaygoal.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package todaygoal
import (
"errors"
"fmt"
"net/http"
"os"
"regexp"
"strings"
Expand Down Expand Up @@ -126,7 +125,7 @@ func Goal(v *viper.Viper) (string, error) {
url = params.API.URL
}

c := api.NewClient(url, http.DefaultClient, opts...)
c := api.NewClient(url, opts...)

goal, err := c.Goal(params.GoalID)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ require (
github.com/sirupsen/logrus v1.8.1
github.com/slongfield/pyfmt v0.0.0-20180124071345-020a7cb18bca
github.com/spf13/cobra v1.1.1
github.com/spf13/jwalterweatherman v1.0.0
github.com/spf13/jwalterweatherman v1.1.0
github.com/spf13/pflag v1.0.5
github.com/spf13/viper v1.7.1
github.com/stretchr/testify v1.7.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@ github.com/spf13/cobra v1.1.1 h1:KfztREH0tPxJJ+geloSLaAkaPkr4ki2Er5quFV1TDo4=
github.com/spf13/cobra v1.1.1/go.mod h1:WnodtKOvamDL/PwE2M4iKs8aMDBZ5Q5klgD3qfVJQMI=
github.com/spf13/jwalterweatherman v1.0.0 h1:XHEdyB+EcvlqZamSM4ZOMGlc93t6AcsBEu9Gc1vn7yk=
github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb68N+wFjFa4jdeBTo=
github.com/spf13/jwalterweatherman v1.1.0 h1:ue6voC5bR5F8YxI5S67j9i582FU4Qvo2bmqnqMYADFk=
github.com/spf13/jwalterweatherman v1.1.0/go.mod h1:aNWZUN0dPAAO/Ljvb5BEdw96iTZ0EXowPYD95IqWIGo=
github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4=
github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
Expand Down
13 changes: 10 additions & 3 deletions pkg/api/api.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package api

import "net/http"
import (
"net/http"
"time"
)

// BaseURL is the base url of the wakatime api.
const BaseURL = "https://api.wakatime.com/api"
Expand All @@ -25,10 +28,14 @@ type Client struct {
}

// NewClient creates a new Client. Any number of Options can be provided.
func NewClient(baseURL string, client *http.Client, opts ...Option) *Client {
func NewClient(baseURL string, opts ...Option) *Client {
c := &Client{
baseURL: baseURL,
client: client,
client: &http.Client{
Transport: NewTransport(),
CheckRedirect: nil, // defaults to following up to 10 redirects
Timeout: 30 * time.Second,
},
doFunc: func(c *Client, req *http.Request) (*http.Response, error) {
req.Header.Set("Accept", "application/json")
return c.client.Do(req)
Expand Down
10 changes: 5 additions & 5 deletions pkg/api/goal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func TestClient_Goal(t *testing.T) {
require.NoError(t, err)
})

c := api.NewClient(u, http.DefaultClient)
c := api.NewClient(u)
goal, err := c.Goal("00000000-0000-4000-8000-000000000000")

require.NoError(t, err)
Expand All @@ -65,7 +65,7 @@ func TestClient_GoalWithTimeout(t *testing.T) {
})

opts := []api.Option{api.WithTimeout(20 * time.Millisecond)}
c := api.NewClient(u, http.DefaultClient, opts...)
c := api.NewClient(u, opts...)
_, err := c.Goal("00000000-0000-4000-8000-000000000000")
require.Error(t, err)

Expand Down Expand Up @@ -93,7 +93,7 @@ func TestClient_Goal_Err(t *testing.T) {
w.WriteHeader(http.StatusInternalServerError)
})

c := api.NewClient(u, http.DefaultClient)
c := api.NewClient(u)
_, err := c.Goal("00000000-0000-4000-8000-000000000000")

var apierr api.Err
Expand All @@ -114,7 +114,7 @@ func TestClient_Goal_ErrAuth(t *testing.T) {
w.WriteHeader(http.StatusUnauthorized)
})

c := api.NewClient(u, http.DefaultClient)
c := api.NewClient(u)
_, err := c.Goal("00000000-0000-4000-8000-000000000000")

var autherr api.ErrAuth
Expand All @@ -124,7 +124,7 @@ func TestClient_Goal_ErrAuth(t *testing.T) {
}

func TestClient_Goal_ErrRequest(t *testing.T) {
c := api.NewClient("invalid-url", http.DefaultClient)
c := api.NewClient("invalid-url")
_, err := c.Goal("00000000-0000-4000-8000-000000000000")

var reqerr api.ErrRequest
Expand Down
8 changes: 4 additions & 4 deletions pkg/api/heartbeat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestClient_Send(t *testing.T) {
require.NoError(t, err)
})

c := api.NewClient(url, http.DefaultClient)
c := api.NewClient(url)
results, err := c.Send(testHeartbeats())
require.NoError(t, err)

Expand Down Expand Up @@ -115,7 +115,7 @@ func TestClient_Send_Err(t *testing.T) {
w.WriteHeader(http.StatusInternalServerError)
})

c := api.NewClient(url, http.DefaultClient)
c := api.NewClient(url)
_, err := c.Send(testHeartbeats())

var errapi api.Err
Expand All @@ -136,7 +136,7 @@ func TestClient_Send_ErrAuth(t *testing.T) {
w.WriteHeader(http.StatusUnauthorized)
})

c := api.NewClient(url, http.DefaultClient)
c := api.NewClient(url)
_, err := c.Send(testHeartbeats())

var errauth api.ErrAuth
Expand All @@ -147,7 +147,7 @@ func TestClient_Send_ErrAuth(t *testing.T) {
}

func TestClient_Send_ErrRequest(t *testing.T) {
c := api.NewClient("invalid-url", http.DefaultClient)
c := api.NewClient("invalid-url")
_, err := c.Send(testHeartbeats())

var errreq api.ErrRequest
Expand Down
23 changes: 7 additions & 16 deletions pkg/api/option_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestOption_WithAuth(t *testing.T) {
req, err := http.NewRequest(http.MethodGet, url, nil)
require.NoError(t, err)

c := api.NewClient("", httpClient(), []api.Option{withAuth}...)
c := api.NewClient("", []api.Option{withAuth}...)
resp, err := c.Do(req)
require.NoError(t, err)

Expand Down Expand Up @@ -81,7 +81,7 @@ func TestOption_WithHostname(t *testing.T) {
req, err := http.NewRequest(http.MethodGet, url, nil)
require.NoError(t, err)

c := api.NewClient("", httpClient(), opts...)
c := api.NewClient("", opts...)
resp, err := c.Do(req)
require.NoError(t, err)

Expand Down Expand Up @@ -129,7 +129,7 @@ func TestOption_WithNTLM(t *testing.T) {
req, err := http.NewRequest(http.MethodGet, url, nil)
require.NoError(t, err)

c := api.NewClient("", &http.Client{}, []api.Option{withNTLM}...)
c := api.NewClient("", []api.Option{withNTLM}...)
resp, err := c.Do(req)
require.NoError(t, err)

Expand Down Expand Up @@ -186,7 +186,7 @@ func TestOption_WithNTLMRequestRetry(t *testing.T) {
req, err := http.NewRequest(http.MethodGet, url, nil)
require.NoError(t, err)

c := api.NewClient("", &http.Client{}, []api.Option{withNTLMRetry}...)
c := api.NewClient("", []api.Option{withNTLMRetry}...)
resp, err := c.Do(req)
require.NoError(t, err)

Expand All @@ -213,7 +213,7 @@ func TestOption_WithProxy(t *testing.T) {
req, err := http.NewRequest(http.MethodGet, "http://example.org", nil)
require.NoError(t, err)

c := api.NewClient("", httpClient(), opts...)
c := api.NewClient("", opts...)
resp, err := c.Do(req)
require.NoError(t, err)

Expand Down Expand Up @@ -248,7 +248,7 @@ func TestOption_WithUserAgent(t *testing.T) {
req, err := http.NewRequest(http.MethodGet, url, nil)
require.NoError(t, err)

c := api.NewClient("", httpClient(), opts...)
c := api.NewClient("", opts...)
resp, err := c.Do(req)
require.NoError(t, err)

Expand Down Expand Up @@ -283,20 +283,11 @@ func TestOption_WithUserAgentUnknownPlugin(t *testing.T) {
req, err := http.NewRequest(http.MethodGet, url, nil)
require.NoError(t, err)

c := api.NewClient("", httpClient(), opts...)
c := api.NewClient("", opts...)
resp, err := c.Do(req)
require.NoError(t, err)

defer resp.Body.Close()

assert.Eventually(t, func() bool { return numCalls == 1 }, time.Second, 50*time.Millisecond)
}

func httpClient() *http.Client {
transport := http.DefaultTransport.(*http.Transport).Clone()
transport.DisableKeepAlives = true

return &http.Client{
Transport: transport,
}
}
12 changes: 6 additions & 6 deletions pkg/api/summary_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestClient_Summaries(t *testing.T) {
require.NoError(t, err)
})

c := api.NewClient(u, http.DefaultClient)
c := api.NewClient(u)
summaries, err := c.Summaries(
time.Date(2020, time.April, 1, 0, 0, 0, 0, time.UTC),
time.Date(2020, time.April, 2, 0, 0, 0, 0, time.UTC),
Expand Down Expand Up @@ -87,7 +87,7 @@ func TestClient_SummariesByCategory(t *testing.T) {
require.NoError(t, err)
})

c := api.NewClient(u, http.DefaultClient)
c := api.NewClient(u)
summaries, err := c.Summaries(
time.Date(2020, time.April, 1, 0, 0, 0, 0, time.UTC),
time.Date(2020, time.April, 2, 0, 0, 0, 0, time.UTC),
Expand Down Expand Up @@ -138,7 +138,7 @@ func TestClient_SummariesWithTimeout(t *testing.T) {
})

opts := []api.Option{api.WithTimeout(20 * time.Millisecond)}
c := api.NewClient(u, http.DefaultClient, opts...)
c := api.NewClient(u, opts...)
_, err := c.Summaries(
time.Date(2020, time.April, 1, 0, 0, 0, 0, time.UTC),
time.Date(2020, time.April, 2, 0, 0, 0, 0, time.UTC),
Expand Down Expand Up @@ -168,7 +168,7 @@ func TestClient_Summaries_Err(t *testing.T) {
w.WriteHeader(http.StatusInternalServerError)
})

c := api.NewClient(u, http.DefaultClient)
c := api.NewClient(u)
_, err := c.Summaries(
time.Date(2020, time.April, 1, 0, 0, 0, 0, time.UTC),
time.Date(2020, time.April, 2, 0, 0, 0, 0, time.UTC),
Expand All @@ -191,7 +191,7 @@ func TestClient_Summaries_ErrAuth(t *testing.T) {
w.WriteHeader(http.StatusUnauthorized)
})

c := api.NewClient(u, http.DefaultClient)
c := api.NewClient(u)
_, err := c.Summaries(
time.Date(2020, time.April, 1, 0, 0, 0, 0, time.UTC),
time.Date(2020, time.April, 2, 0, 0, 0, 0, time.UTC),
Expand All @@ -204,7 +204,7 @@ func TestClient_Summaries_ErrAuth(t *testing.T) {
}

func TestClient_Summaries_ErrRequest(t *testing.T) {
c := api.NewClient("invalid-url", http.DefaultClient)
c := api.NewClient("invalid-url")
_, err := c.Summaries(
time.Date(2020, time.April, 1, 0, 0, 0, 0, time.UTC),
time.Date(2020, time.April, 2, 0, 0, 0, 0, time.UTC),
Expand Down
18 changes: 18 additions & 0 deletions pkg/api/transport.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package api

import (
"net/http"
"time"
)

// NewTransport creates a new http.Transport, with default 30 second TLS timeout.
func NewTransport() *http.Transport {
return &http.Transport{
Proxy: nil,
TLSHandshakeTimeout: 30 * time.Second,
MaxIdleConns: 1,
MaxIdleConnsPerHost: 1,
MaxConnsPerHost: 1,
ForceAttemptHTTP2: true,
}
}

0 comments on commit 2052676

Please sign in to comment.