Skip to content

Commit

Permalink
Introduce GetBackendWithConfig and make logging configurable per ba…
Browse files Browse the repository at this point in the history 
…ckend

Introduces a new system for getting backends with a set of custom
configuration that's a little cleaner than adding a new setter for every
option that we add in the future.

`BackendConfig` is introduced a user configurable struct which is then
passed to `GetBackendWithConfig`, which returns a backend configured to
spec. `GetBackend` remains, is backward compatible, and continues to set
a default backend for either type.

A new option on `BackendConfig` is being able to set a per-backend
`Logger` and `LogLevel`. This is the impetus that sparked this change as
documented in #608.

So the unfortunate part is that `BackendConfig` is confusingly named
similarly to the very poorly named `BackendConfiguration`, which is the
final implementation of a backend. I wanted to keep this patch backward
compatible, but I want to make a breaking change in the future to rename
this to `backendImplementation` and make it unexported at the same time
because it shouldn't be needed outside of the core package.

We should also unexport `NewBackends` at the same time. It was
introduced to allow some configuration for backends, but it was a pretty
bad idea.

Fixes #608.
  • Loading branch information
@brandur
brandur committed Jul 3, 2018
1 parent 157f27f commit 647da42
Show file tree
Hide file tree
Showing 5 changed files with 189 additions and 84 deletions.
8 changes: 3 additions & 5 deletions error_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,11 @@ func TestErrorResponse(t *testing.T) {
}))
defer ts.Close()

SetBackend("api", &BackendConfiguration{
Type: APIBackend,
URL: ts.URL,
HTTPClient: &http.Client{},
backend := GetBackendWithConfig(APIBackend, &BackendConfig{
URL: ts.URL,
})

err := GetBackend(APIBackend).Call(http.MethodGet, "/v1/account", "sk_test_badKey", nil, nil)
err := backend.Call(http.MethodGet, "/v1/account", "sk_test_badKey", nil, nil)
assert.Error(t, err)

stripeErr := err.(*Error)
Expand Down
15 changes: 4 additions & 11 deletions order/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,6 @@ func TestOrderReturn(t *testing.T) {
}

func TestOrderReturn_RequestParams(t *testing.T) {
// Restore the backend when this test cases completes.
backend := stripe.GetBackend("api")
defer func() {
stripe.SetBackend("api", backend)
}()

// Create an ephemeral test server so that we can inspect request attributes.
var lastRequest *http.Request
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand All @@ -65,16 +59,15 @@ func TestOrderReturn_RequestParams(t *testing.T) {
defer ts.Close()

// Configure the stripe client to use the ephemeral backend.
stripe.SetBackend("api", &stripe.BackendConfiguration{
Type: stripe.APIBackend,
URL: ts.URL + "/v1",
HTTPClient: &http.Client{},
backend := stripe.GetBackendWithConfig(stripe.APIBackend, &stripe.BackendConfig{
URL: ts.URL,
})
orderClient := Client{B: backend, Key: stripe.Key}

p := &stripe.OrderReturnParams{}
p.SetStripeAccount("acct_123")

order, err := Return("or_123", p)
order, err := orderClient.Return("or_123", p)
assert.Nil(t, err)
assert.NotNil(t, order)

Expand Down
212 changes: 163 additions & 49 deletions stripe.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ var LogLevel = 2

// Logger controls how stripe performs logging at a package level. It is useful
// to customise if you need it prefixed for your application to meet other
// requirements
// requirements.
//
// This Logger will be inherited by any backends created by default, but will
// be overridden if a backend is created with GetBackendWithConfig.
var Logger Printfer

//
Expand Down Expand Up @@ -101,12 +104,54 @@ type Backend interface {
SetMaxNetworkRetries(maxNetworkRetries int)
}

// BackendConfiguration is the internal implementation for making HTTP calls to Stripe.
// BackendConfig is used to configure a new Stripe backend.
type BackendConfig struct {
// HTTPClient is an HTTP client instance to use when making API requests.
//
// If left unset, it'll be set to a default HTTP client for the package.
HTTPClient *http.Client

// LogLevel is the logging level of the library and defined by:
//
// 0: no logging
// 1: errors only
// 2: errors + informational (default)
// 3: errors + informational + debug
//
// Defaults to 0 (no logging), so please make sure to set this if you want
// to see logging output in your custom configuration.
LogLevel int

// Logger is where this backend will write its logs.
//
// If left unset, it'll be set to Logger.
Logger Printfer

// MaxNetworkRetries sets maximum number of times that the library will
// retry requests that appear to have failed due to an intermittent
// problem.
//
// Defaults to 0.
MaxNetworkRetries int

// URL is the base URL to use for API paths.
//
// If left empty, it'll be set to the default for the SupportedBackend.
URL string
}

// BackendConfiguration is the internal implementation for making HTTP calls to
// Stripe.
//
// The public use of this struct is deprecated. It will be renamed and changed
// to unexported in a future version.
type BackendConfiguration struct {
Type SupportedBackend
URL string
HTTPClient *http.Client
MaxNetworkRetries int
LogLevel int
Logger Printfer
}

// Call is the Backend.Call implementation for invoking Stripe APIs.
Expand Down Expand Up @@ -185,8 +230,8 @@ func (s *BackendConfiguration) NewRequest(method, path, key, contentType string,

req, err := http.NewRequest(method, path, body)
if err != nil {
if LogLevel > 0 {
Logger.Printf("Cannot create Stripe request: %v\n", err)
if s.LogLevel > 0 {
s.Logger.Printf("Cannot create Stripe request: %v\n", err)
}
return nil, err
}
Expand Down Expand Up @@ -233,8 +278,8 @@ func (s *BackendConfiguration) NewRequest(method, path, key, contentType string,
// the backend's HTTP client to execute the request and unmarshals the response
// into v. It also handles unmarshaling errors returned by the API.
func (s *BackendConfiguration) Do(req *http.Request, v interface{}) error {
if LogLevel > 1 {
Logger.Printf("Requesting %v %v%v\n", req.Method, req.URL.Host, req.URL.Path)
if s.LogLevel > 1 {
s.Logger.Printf("Requesting %v %v%v\n", req.Method, req.URL.Host, req.URL.Path)
}

start := time.Now()
Expand All @@ -244,36 +289,36 @@ func (s *BackendConfiguration) Do(req *http.Request, v interface{}) error {
for retry := 0; ; {
res, err = s.HTTPClient.Do(req)
if s.shouldRetry(err, res, retry) {
if LogLevel > 0 {
Logger.Printf("Request failed with error: %v. Response: %v\n", err, res)
if s.LogLevel > 0 {
s.Logger.Printf("Request failed with error: %v. Response: %v\n", err, res)
}
sleep := sleepTime(retry)
time.Sleep(sleep)
retry++
if LogLevel > 1 {
Logger.Printf("Retry request %v %v time.\n", req.URL, retry)
if s.LogLevel > 1 {
s.Logger.Printf("Retry request %v %v time.\n", req.URL, retry)
}
} else {
break
}
}

if LogLevel > 2 {
Logger.Printf("Completed in %v\n", time.Since(start))
if s.LogLevel > 2 {
s.Logger.Printf("Completed in %v\n", time.Since(start))
}

if err != nil {
if LogLevel > 0 {
Logger.Printf("Request to Stripe failed: %v\n", err)
if s.LogLevel > 0 {
s.Logger.Printf("Request to Stripe failed: %v\n", err)
}
return err
}
defer res.Body.Close()

resBody, err := ioutil.ReadAll(res.Body)
if err != nil {
if LogLevel > 0 {
Logger.Printf("Cannot parse Stripe response: %v\n", err)
if s.LogLevel > 0 {
s.Logger.Printf("Cannot parse Stripe response: %v\n", err)
}
return err
}
Expand All @@ -282,8 +327,8 @@ func (s *BackendConfiguration) Do(req *http.Request, v interface{}) error {
return s.ResponseToError(res, resBody)
}

if LogLevel > 2 {
Logger.Printf("Stripe Response: %q\n", resBody)
if s.LogLevel > 2 {
s.Logger.Printf("Stripe Response: %q\n", resBody)
}

if v != nil {
Expand All @@ -309,8 +354,8 @@ func (s *BackendConfiguration) ResponseToError(res *http.Response, resBody []byt
e, ok := errMap["error"]
if !ok {
err := errors.New(string(resBody))
if LogLevel > 0 {
Logger.Printf("Unparsable error returned from Stripe: %v\n", err)
if s.LogLevel > 0 {
s.Logger.Printf("Unparsable error returned from Stripe: %v\n", err)
}
return err
}
Expand Down Expand Up @@ -364,14 +409,16 @@ func (s *BackendConfiguration) ResponseToError(res *http.Response, resBody []byt
stripeErr.Err = &RateLimitError{stripeErr: stripeErr}
}

if LogLevel > 0 {
Logger.Printf("Error encountered from Stripe: %v\n", stripeErr)
if s.LogLevel > 0 {
s.Logger.Printf("Error encountered from Stripe: %v\n", stripeErr)
}

return stripeErr
}

// SetMaxNetworkRetries sets max number of retries on failed requests
//
// This function is deprecated. Please use GetBackendWithConfig instead.
func (s *BackendConfiguration) SetMaxNetworkRetries(maxNetworkRetries int) {
s.MaxNetworkRetries = maxNetworkRetries
}
Expand Down Expand Up @@ -462,32 +509,85 @@ func FormatURLPath(format string, params ...string) string {
return fmt.Sprintf(format, untypedParams...)
}

// GetBackend returns the currently used backend in the binding.
func GetBackend(backend SupportedBackend) Backend {
switch backend {
// GetBackend returns one of the library's supported backends based off of the
// given argument.
//
// It returns an existing default backend if one's already been created.
func GetBackend(backendType SupportedBackend) Backend {
var backend Backend

backends.mu.RLock()
switch backendType {
case APIBackend:
backends.mu.RLock()
ret := backends.API
backends.mu.RUnlock()
if ret != nil {
return ret
backend = backends.API
case UploadsBackend:
backend = backends.Uploads
}
backends.mu.RUnlock()
if backend != nil {
return backend
}

backend = GetBackendWithConfig(
backendType,
&BackendConfig{
HTTPClient: httpClient,
LogLevel: LogLevel,
Logger: Logger,
MaxNetworkRetries: 0,
URL: "", // Set by GetBackendWithConfiguation when empty
},
)

backends.mu.Lock()
defer backends.mu.Unlock()

switch backendType {
case APIBackend:
backends.API = backend
case UploadsBackend:
backends.Uploads = backend
}

return backend
}

// GetBackendWithConfig is the same as GetBackend except that it can be given a
// configuration struct that will configure certain aspects of the backend
// that's return.
func GetBackendWithConfig(backendType SupportedBackend, config *BackendConfig) Backend {
if config.HTTPClient == nil {
config.HTTPClient = httpClient
}

if config.Logger == nil {
config.Logger = Logger
}

switch backendType {
case APIBackend:
if config.URL == "" {
config.URL = apiURL
}
backends.mu.Lock()
defer backends.mu.Unlock()
backends.API = &BackendConfiguration{Type: backend, URL: apiURL, HTTPClient: httpClient}
return backends.API

// Add the /v1/ prefix because all client packages expect it. We should
// probably change this to just make it explicit wherever it's needed
// to fulfill the principle of least astonishment.
config.URL += "/v1"

return newBackendConfiguration(backendType, config)

case UploadsBackend:
backends.mu.RLock()
ret := backends.Uploads
backends.mu.RUnlock()
if ret != nil {
return ret
if config.URL == "" {
config.URL = uploadsURL
}
backends.mu.Lock()
defer backends.mu.Unlock()
backends.Uploads = &BackendConfiguration{Type: backend, URL: uploadsURL, HTTPClient: httpClient}
return backends.Uploads

// Add the /v1/ prefix because all client packages expect it. We should
// probably change this to just make it explicit wherever it's needed
// to fulfill the principle of least astonishment.
config.URL += "/v1"

return newBackendConfiguration(backendType, config)
}

return nil
Expand All @@ -511,10 +611,8 @@ func Int64Value(v *int64) int64 {
// should only need to use this for testing purposes or on App Engine.
func NewBackends(httpClient *http.Client) *Backends {
return &Backends{
API: &BackendConfiguration{
Type: APIBackend, URL: APIURL, HTTPClient: httpClient},
Uploads: &BackendConfiguration{
Type: UploadsBackend, URL: UploadsURL, HTTPClient: httpClient},
API: GetBackend(APIBackend),
Uploads: GetBackend(UploadsBackend),
}
}

Expand Down Expand Up @@ -586,7 +684,7 @@ func StringValue(v *string) string {
// Private constants
//

const apiURL = "https://api.stripe.com/v1"
const apiURL = "https://api.stripe.com"

// apiversion is the currently supported API version
const apiversion = "2018-02-06"
Expand All @@ -604,7 +702,7 @@ const defaultHTTPTimeout = 80 * time.Second
const maxNetworkRetriesDelay = 5000 * time.Millisecond
const minNetworkRetriesDelay = 500 * time.Millisecond

const uploadsURL = "https://uploads.stripe.com/v1"
const uploadsURL = "https://uploads.stripe.com"

//
// Private types
Expand Down Expand Up @@ -689,6 +787,22 @@ func isHTTPWriteMethod(method string) bool {
return method == http.MethodPost || method == http.MethodPut || method == http.MethodPatch || method == http.MethodDelete
}

// newBackendConfiguration returns a new Backend based off a given type and
// fully initialized BackendConfig struct.
//
// The vast majority of the time you should be calling GetBackendWithConfig
// instead of this function.
func newBackendConfiguration(backendType SupportedBackend, config *BackendConfig) Backend {
return &BackendConfiguration{
HTTPClient: config.HTTPClient,
LogLevel: config.LogLevel,
Logger: config.Logger,
MaxNetworkRetries: config.MaxNetworkRetries,
Type: backendType,
URL: config.URL,
}
}

// sleepTime calculates sleeping/delay time in milliseconds between failure and a new one request.
func sleepTime(numRetries int) time.Duration {
// Apply exponential backoff with minNetworkRetriesDelay on the
Expand Down
Loading

0 comments on commit 647da42

Please sign in to comment.