Skip to content

Commit

Permalink
Fix nil pointer dereference when initializing fetcher with timeout
Browse files Browse the repository at this point in the history
  • Loading branch information
hwen-cb committed Mar 10, 2021
1 parent 7c2169d commit 918f5c9
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 2 deletions.
2 changes: 1 addition & 1 deletion fetcher/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func WithInsecureTLS() Option {
// WithTimeout overrides the default HTTP timeout.
func WithTimeout(timeout time.Duration) Option {
return func(f *Fetcher) {
f.rosettaClient.GetConfig().HTTPClient.Timeout = timeout
f.httpTimeout = timeout
}
}

Expand Down
4 changes: 3 additions & 1 deletion fetcher/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ type Fetcher struct {
retryElapsedTime time.Duration
insecureTLS bool
forceRetry bool
httpTimeout time.Duration

// connectionSemaphore is used to limit the
// number of concurrent requests we make.
Expand All @@ -89,6 +90,7 @@ func New(
maxConnections: DefaultMaxConnections,
maxRetries: DefaultRetries,
retryElapsedTime: DefaultElapsedTime,
httpTimeout: DefaultHTTPTimeout,
}

// Override defaults with any provided options
Expand All @@ -106,7 +108,7 @@ func New(
defaultTransport.MaxIdleConns = f.maxConnections
defaultTransport.MaxIdleConnsPerHost = DefaultMaxConnections
defaultHTTPClient := &http.Client{
Timeout: DefaultHTTPTimeout,
Timeout: f.httpTimeout,
Transport: defaultTransport,
}

Expand Down
27 changes: 27 additions & 0 deletions fetcher/fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,3 +214,30 @@ func TestNewWithHTTPCLient(t *testing.T) {
var assert = assert.New(t)
assert.Same(httpClient, fetcher.rosettaClient.GetConfig().HTTPClient)
}

func TestNewWithTimeout(t *testing.T) {
var assert = assert.New(t)

fetcher := New("https://serveraddress")
assert.Equal(DefaultHTTPTimeout, fetcher.rosettaClient.GetConfig().HTTPClient.Timeout)

var customTimeout = 6*time.Minute
fetcher2 := New("https://serveraddress", WithTimeout(customTimeout))
assert.Equal(customTimeout, fetcher2.rosettaClient.GetConfig().HTTPClient.Timeout)

// If we pass in a http timeout value when initializing the fetcher, and also pass in an existing client,
// we will simply respect the timeout on the existing client and not override it.
var existingClientTimeout = 3*time.Minute
httpClient := &http.Client{
Timeout : existingClientTimeout,
}
apiClient := client.NewAPIClient(
client.NewConfiguration(
"https://serveraddress",
DefaultUserAgent,
httpClient,
),
)
fetcher3 := New("https://serveraddress", WithClient(apiClient), WithTimeout(6*time.Minute))
assert.Equal(existingClientTimeout, fetcher3.rosettaClient.GetConfig().HTTPClient.Timeout)
}

0 comments on commit 918f5c9

Please sign in to comment.