tansengming / stripe-rails

A Rails Engine for integrating with Stripe
MIT License
753 stars 123 forks source link

Add a hook to run some code before any Stripe event callbacks #188

Open ndbroadbent opened 4 years ago

ndbroadbent commented 4 years ago

Hello,

I saw that the stripe.event callback is run at the very end:

      def run_callbacks(evt, target)
        _run_callbacks evt.type, evt, target
        _run_callbacks 'stripe.event', evt, target
      end

It would be very useful if I could also run a callback at the beginning. In my case, I want to add the event ID as extra context for my Sentry error reports, in case one of my event handlers crashes with an error. e.g.

before_stripe_event_callback do |event|
   Raven.extra_context(event_id: event.id)
end

This would make it much easier for me to look up the Stripe event in the Stripe web UI, so I can see what event data caused the error to happen.

Do you have any feedback or suggestions for this idea, or any thoughts on how this could be implemented?

ndbroadbent commented 4 years ago

Here's a monkey-patch I'm using for now if anyone is interested!

# frozen_string_literal: true

require 'stripe/callbacks'
require 'stripe/rails/version'

if Stripe::Rails::VERSION != '2.0.0'
  raise "stripe-rails has been updated to version #{Stripe::Rails::VERSION}! " \
    "Please make sure that Stripe::Callbacks#run_callbacks hasn't been updated:\n" \
    "https://github.com/tansengming/stripe-rails/blob/master/lib/stripe/callbacks.rb\n" \
    '(Compare this to the "Original method:" below.)' \
    'Please also check this GitHub issue to see if this patch is still required: ' \
    'https://github.com/tansengming/stripe-rails/issues/188'
end

module Stripe
  module Callbacks
    # Original method:
    # def run_callbacks(evt, target)
    #   _run_callbacks evt.type, evt, target
    #   _run_callbacks 'stripe.event', evt, target
    # end

    def self.run_callbacks(evt, target)
      Raven.extra_context(
        stripe_event_id: evt[:id],
        stripe_event_type: evt[:type],
        stripe_event_created: evt[:created]
      )
      _run_callbacks evt.type, evt, target
      _run_callbacks 'stripe.event', evt, target
    end
  end
end

UPDATE: Still works in 2.0.0

ndbroadbent commented 1 year ago

UPDATE: I'm still using this monkeypatch, and it's still working fine on the latest version (2.4.0). I don't mind it too much but would be cool to have an officially supported way to do this

tansengming commented 1 year ago

Hi @ndbroadbent thanks for the suggestion. I like the idea!

However I don't have the bandwidth to work on this from scratch. If you (or anyone else!) would like to create a PR for this it stands a fair chance of getting merged.

ndbroadbent commented 1 year ago

Sure, I would be happy to look into this! I will open a PR to get some feedback

ndbroadbent commented 11 months ago

Hi @tansengming, my company is taking part in Hacktoberfest this year and we're looking for some repos to contribute to. I remembered this issue and would like to create a PR! I was just wondering if you could please add the hacktoberfest topic to the repo so that my PR will count towards this? Thanks!

tansengming commented 11 months ago

@ndbroadbent that's a great idea! Thanks for helping out!

I just added the topic.

image