Add Stripe.WebhookPlug for easier handling of webhook events

[alexgleason]

The Stripe.Webhook module does some of the heavy lifting when it comes to handling Stripe events, but it's still requires the user to understand how body parsers work to use it:

payload is the raw, unparsed content body sent by Stripe, which can be
retrieved with Plug.Conn.read_body/2. Note that Plug.Parsers will read
and discard the body, so you must implement a [custom body reader][1] if the
plug is located earlier in the pipeline.

The goal of this PR is to close the gap by adding a plug that makes it extremely easy to implement a webhook handler.

Adding the plug to your endpoint.ex looks like this:

plug Stripe.WebhookPlug,
  at: "/webhook/stripe",
  handler: MyAppWeb.StripeHandler, # your custom handler module
  secret: "whsec_******"

The user needs to implement a handler module with a handle_event/1 function that takes a Stripe.Event arg and returns :ok or {:ok, _}

# lib/myapp_web/stripe_handler.ex

defmodule MyAppWeb.StripeHandler do
  @behaviour Stripe.WebhookHandler

  @impl true
  def handle_event(%Stripe.Event{type: "charge.succeeded"} = event) do
    # TODO: handle the charge.succeeded event
  end

  @impl true
  def handle_event(%Stripe.Event{type: "invoice.payment_failed"} = event) do
    # TODO: handle the invoice.payment_failed event
  end

  # Return HTTP 200 for unhandled events
  @impl true
  def handle_event(_event), do: :ok
end

This pattern is inspired by the StripeEvent Ruby gem I used while working with Stripe in Rails. With functional programming it's a little better, and can even support multiple webhooks.

plug is added as an optional dependency, meaning end users will have to include it on their own unless they're working with this repo directly.

Note: the docs wouldn't build for me, so I upgraded ex_doc and that fixed it.

[coveralls]

Coverage decreased (-0.2%) to 90.675% when pulling bd1c23c on alexgleason:webhook-plug into e82ab41 on code-corps:master.

[coveralls]

Coverage decreased (-0.2%) to 90.559% when pulling a100e3f on alexgleason:webhook-plug into dbff7f3 on code-corps:master.

[aseigo]

I don't see the advantage of this in its current form. If I were to adopt this into my project, it would replace 4 lines in my router:

  scope "/h", MyApp.Web do
    pipe_through(:api)
    post("/subscriber", StripeEventsController, :subscriber_event)
  end

for 4 lines in my router setting up a plug, and 1 module (the Controller) with 1 module (the handler). The only difference would be these two lines would go away in my code base:

    [signature] = Plug.Conn.get_req_header(conn, "stripe-signature")
    Stripe.Webhook.construct_event(payload, signature, secret)

That seems like an awfully small savings for adding a plug and what not to the core library imho. What would make it more interesting (again, imho) is if instead of a bunch of handle_event/3 callbacks the different events were actually purpose-specific functions. e.g. if I could implement a SubscriptionWebhook handler that would have callbacks for created/updated/delete, preventing my code from even having to know the event's type string. That would actually make the webhook code more readable and less error prone.

This would mean more work on the library side, but that's the sort of thing that gives libraries value rather than being a bunch of code that saves me 2 lines in my own application (and at the cost of not being able to control the responses fully, esp in the case of error).

This is also the sort of thing that would make an excellent add-on library rather than be in the core library imho: removes the need for an added optional dependency (though I suspect everyone already has that), makes it entirely opt-in for those who need/want this, and would allow for greater exploration / experimentation with a good API without having to rely on it making into the main upstream stripity_stripe library.

[alexgleason]

You must have a custom body parser in there, right? That's the point of this, to remove the need for it.

[alexgleason]

By the time the request hits your controller, it will have already passed through Plug.Parsers, which converts the raw body into a native Elixir map and discards the raw body. It's not possible to verify the event's signature in the controller, because the raw body is already gone by that point.

Here's an example: https://stackoverflow.com/questions/41510957/read-the-raw-body-from-a-plug-connection-after-parsers-in-elixir

This Stripe.WebhookPlug solution might not benefit someone who has already overcome this obstacle, but it will definitely help future devs who are scratching their heads wondering "why hasn't this been solved this yet?" It's a painful barrier to cross when you're just trying to implement webhook events, but you have to dig into the internals of Phoenix first. This MR solves it.

[aseigo]

Yes, I have a custom body parser, as per the examples. I'm not sure which is easier: reading the docs and seeing you have to put this Plug in the right place in endpoint.ex or reading he docs and seeing you need a body parser ... I do agree that the Plug probably feels more "natural" as it is more common to use Plugs in general, so perhaps that is an improvement. It does solve the lesser issue though: knowing what the hooks are an ensuring there are implementations of them was the actually tricky bit IME.

[Rio517]

I'm just running into this. At least by virtue of this PR, I was able to figure it out.

@aseigo, it would be great if you would be willing to accept this, but I understand if not. Perhaps close the PR if you think it should be rejected?

[aseigo]

@Rio517 it's not my repo :) I was just looking through some of the PRs at the time for a related (but different) issue ... personally, if this was my call, I'd still be wary of adding a dependency on plug in the core library here, and probably rather instead note this caveat more clearly in the docs and create a separate lib that depends on both stripity_stripe and plug that contains this functionality. But, again, it's not my repo. :)

[snewcomer]

@alexgleason Happy to merge this and publish once updated!

[alexgleason]

Done!

[snewcomer]

Wonderful PR!

[snewcomer]

Perfect

[snewcomer]

I think one thing that makes this a great candidate for an abstraction is the need for a blog post to help people reach an easy integration with this library (e.g. the need for the unparsed raw body). Thanks a lot!