Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow setting write_timeout for connections on Ruby 2.6+ #950

Merged
merged 1 commit into from Oct 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Expand Up @@ -186,11 +186,12 @@ retries are safe.

### Configuring Timeouts

Open and read timeouts are configurable:
Open, read and write timeouts are configurable:

```ruby
Stripe.open_timeout = 30 # in seconds
Stripe.read_timeout = 80
Stripe.write_timeout = 30 # only supported on Ruby 2.6+
```

Please take care to set conservative read timeouts. Some API requests can take
Expand Down
1 change: 1 addition & 0 deletions lib/stripe.rb
Expand Up @@ -71,6 +71,7 @@ class << self
def_delegators :@configuration, :connect_base, :connect_base=
def_delegators :@configuration, :open_timeout, :open_timeout=
def_delegators :@configuration, :read_timeout, :read_timeout=
def_delegators :@configuration, :write_timeout, :write_timeout=
def_delegators :@configuration, :proxy, :proxy=
def_delegators :@configuration, :verify_ssl_certs, :verify_ssl_certs=
def_delegators :@configuration, :ca_bundle_path, :ca_bundle_path=
Expand Down
3 changes: 3 additions & 0 deletions lib/stripe/connection_manager.rb
Expand Up @@ -119,6 +119,9 @@ def execute_request(method, uri, body: nil, headers: nil, query: nil)

connection.open_timeout = Stripe.open_timeout
connection.read_timeout = Stripe.read_timeout
if connection.respond_to?(:write_timeout=)
connection.write_timeout = Stripe.write_timeout
end

connection.use_ssl = uri.scheme == "https"

Expand Down
11 changes: 11 additions & 0 deletions lib/stripe/stripe_configuration.rb
Expand Up @@ -42,6 +42,7 @@ class StripeConfiguration
attr_reader :max_network_retry_delay
attr_reader :open_timeout
attr_reader :read_timeout
attr_reader :write_timeout
attr_reader :proxy
attr_reader :verify_ssl_certs

Expand Down Expand Up @@ -72,6 +73,7 @@ def initialize

@open_timeout = 30
@read_timeout = 80
@write_timeout = 30

@api_base = "https://api.stripe.com"
@connect_base = "https://connect.stripe.com"
Expand Down Expand Up @@ -109,6 +111,15 @@ def read_timeout=(read_timeout)
StripeClient.clear_all_connection_managers
end

def write_timeout=(write_timeout)
unless Net::HTTP.instance_methods.include?(:write_timeout=)
raise NotImplementedError
end

@write_timeout = write_timeout
StripeClient.clear_all_connection_managers
end

def proxy=(proxy)
@proxy = proxy
StripeClient.clear_all_connection_managers
Expand Down
4 changes: 4 additions & 0 deletions test/stripe/connection_manager_test.rb
Expand Up @@ -39,6 +39,7 @@ class ConnectionManagerTest < Test::Unit::TestCase

old_open_timeout = Stripe.open_timeout
old_read_timeout = Stripe.read_timeout
old_write_timeout = Stripe.write_timeout

begin
# Make sure any global initialization here is undone in the `ensure`
Expand All @@ -47,6 +48,7 @@ class ConnectionManagerTest < Test::Unit::TestCase

Stripe.open_timeout = 123
Stripe.read_timeout = 456
Stripe.write_timeout = 789 if WRITE_TIMEOUT_SUPPORTED

conn = @manager.connection_for("https://stripe.com")

Expand All @@ -63,6 +65,7 @@ class ConnectionManagerTest < Test::Unit::TestCase
# Timeouts
assert_equal 123, conn.open_timeout
assert_equal 456, conn.read_timeout
assert_equal 789, conn.write_timeout if WRITE_TIMEOUT_SUPPORTED

assert_equal true, conn.use_ssl?
assert_equal OpenSSL::SSL::VERIFY_PEER, conn.verify_mode
Expand All @@ -72,6 +75,7 @@ class ConnectionManagerTest < Test::Unit::TestCase

Stripe.open_timeout = old_open_timeout
Stripe.read_timeout = old_read_timeout
Stripe.write_timeout = old_write_timeout if WRITE_TIMEOUT_SUPPORTED
end
end

Expand Down
3 changes: 3 additions & 0 deletions test/stripe/stripe_configuration_test.rb
Expand Up @@ -16,6 +16,7 @@ class StripeConfigurationTest < Test::Unit::TestCase
assert_equal 0, config.max_network_retries
assert_equal 30, config.open_timeout
assert_equal 80, config.read_timeout
assert_equal 30, config.write_timeout
assert_equal "https://api.stripe.com", config.api_base
assert_equal "https://connect.stripe.com", config.connect_base
assert_equal "https://files.stripe.com", config.uploads_base
Expand All @@ -25,10 +26,12 @@ class StripeConfigurationTest < Test::Unit::TestCase
config = Stripe::StripeConfiguration.setup do |c|
c.open_timeout = 100
c.read_timeout = 100
c.write_timeout = 100 if WRITE_TIMEOUT_SUPPORTED
end

assert_equal 100, config.open_timeout
assert_equal 100, config.read_timeout
assert_equal 100, config.write_timeout if WRITE_TIMEOUT_SUPPORTED
end
end

Expand Down
13 changes: 13 additions & 0 deletions test/stripe_test.rb
Expand Up @@ -54,6 +54,19 @@ class StripeTest < Test::Unit::TestCase
assert_equal 10, Stripe.read_timeout
end

if WRITE_TIMEOUT_SUPPORTED
should "allow write timeout to be configured" do
Stripe.write_timeout = 10
assert_equal 10, Stripe.write_timeout
end
else
should "raise when write timeout to be configured is not supported" do
assert_raises NotImplementedError do
Stripe.write_timeout = 10
end
end
end

should "allow api_key to be configured" do
Stripe.api_key = "sk_local_test"
assert_equal "sk_local_test", Stripe.api_key
Expand Down
2 changes: 2 additions & 0 deletions test/test_helper.rb
Expand Up @@ -56,6 +56,8 @@ class TestCase
include Stripe::TestData
include Mocha

WRITE_TIMEOUT_SUPPORTED = Net::HTTP.instance_methods.include?(:write_timeout=)

setup do
Stripe.api_key = "sk_test_123"
Stripe.api_base = "http://localhost:#{MOCK_PORT}"
Expand Down