integrallis / stripe_event

Stripe webhook integration for Rails applications.
https://rubygems.org/gems/stripe_event
MIT License
844 stars 104 forks source link

docs: Update README.md #157

Closed soudai-s closed 1 year ago

soudai-s commented 2 years ago

Setting up according to the README will cause problems because the same instance will continue to be used. To solve this problem, the README must be changed and the configuration must be changed as follows.

# Before
StripeEvent.configure do |events|
  events.all BillingEventLogger.new(Rails.logger)
  events.subscribe 'customer.created', CustomerCreated.new
end

# After
StripeEvent.configure do |events|
  events.all ->(event) { BillingEventLogger.new(Rails.logger).call(event) }
  events.subscribe 'customer.created', ->(event) { CustomerCreated.new.call(event) }
end

It should pass the procedures including initialization wrapped by Lambda instead of instances which already initialized in the configuration.

soudai-s commented 2 years ago

The current README causes problems in such cases.

class CustomerCreated
  attr_reader :subscription

  def call(event)
    # Passing instances directly in the configuration causes problems due to race conditions!
    @subscription = event.data.object
  end
end
rmm5t commented 1 year ago

@soudai-s The particular example in the README is fine, because the implied usage of Rails.logger is thread safe.

The call method definitely shouldn't assign instance variables. That would be a recipe for disaster.

However, if you absolutely must assign instance variables in your call method, your proc wrapper is a fine workaround.