Store events to avoid duplication. Move event handling to async tasks.

[begedin]

What's in this PR?

This PR adds a StripeEvent model and associated table.

Then it hooks into the current webhook handling process.

As a webhook request is received, a StripeEvent record is stored, with fields

* `id_from_stripe` - the id of the Stripe event
* `status` - the state of the event. Automatically set to `"unprocessed"`

Once the request is processed, the created event is set to either "processed" or "errored" depending on the processing result.

Details

• We first attempt to find the event in the database, using :id_from_stripe. The event record is only created when there is no such event stored already.
• If the event record is found, then it only get's processed if the status is "unprocessed" or "errored". An already processed event is ignored.
• The endpoint returns a 204 if the event was ignored due to being processed already, or due to the system not supporting that specific event type.
• In all other cases, the endpoint returns a 200

References

Fixes #512
Fixes #547

[begedin]

get_by might return a nil. We weren't restricting the type.

[joshsmith]

We should still consider doing the following here:

• Add processing status, respond with 200 immediately, and kick off asynchronous task that updates later
• Add another field for the event type (for easily looking at events in the database, in logs, etc)
• Add unhandled status for unhandled events instead of a 204, since Stripe will keep trying to hit us for all these "unhandled" events

[joshsmith]

@begedin did not get a chance to implement my comment, but think all of that should still be implemented.

[begedin]

Add processing status, respond with 200 immediately, and kick off asynchronous task that updates later

Working on this one

Add another field for the event type (for easily looking at events in the database, in logs, etc)

Done

Add unhandled status for unhandled events instead of a 204, since Stripe will keep trying to hit us for all these "unhandled" events

We now store the event as "unhandled". Returning a 204, however, is not a problem. It's just for us, so we can easily test the response and see that the server differentiates. According to stripe documentation, any 2xx response will be considered as "event is done, do not send it again".

That being said, we only return a 204 in the case of "ignored by environment", because we, for example, got a livemode event on staging or vice-versa. We do not return it in case of "unhandled", since in that case, we should just consume the event and (for now) not do anything about it. In this case, we return a 200, same as "errored" or "processed"

From https://stripe.com/docs/webhooks#responding-to-a-webhook

To acknowledge receipt of a webhook, your endpoint should return a 2xx HTTP status code. Any other information returned in the request headers or request body is ignored. All response codes outside this range, including 3xx codes, will indicate to Stripe that you did not receive the webhook.

[begedin]

I have now added async processing of webhooks.

In order to achieve this, we use a Task.Supervisor, referenced :webhook_processor

Testing was giving me trouble with this one, but I managed to get around it by explicitly waiting for the child processes to exit, using a &Process.monitor/1 followed by an assert_received. It's not perfect, but I don't have a better idea. I could use an experts thoughts on this one. Should I maybe "inject" our supervisor and stub it out with a test module, similar to how we do it with our services?

[begedin]

The process is now as follows:

• Controller will call WebhookProcessor.process_async(json, handlermodule)
• process_async will use a supervisor to handle the event. It will immediately return either {:ok, pid} or {:ok, :ignored_by_environment}. For the first result, the reply will be 200, for the second, 204. Both are considered "handled, do not send again" by stripe.
• The supervisor will run the task. Event will be handled according to our handler setup
  • Event record is initially created with status processing
  • If event already exists and its status is already processing, the task just returns with nil
  • If event already exists and its status is something else, we try to process it again
  • If processing is completed successfully, event will be set to processed
  • If there is some sort of error, event will be set to errored
  • If we do not handle the type of event, it will be set to unhandled

[begedin]

Something we should consider adding but is probably out of scope for this PR:

• Right now, we respond with 2xx for all events, so they do not get resent. Hower, some events are currently unhandled because we do not support the type yet. Some simply error out. Both of these should probably be either periodically restarted or handled in some other way. We Should figure out what to do with them and make an issue prior to merging this one.

[joshsmith]

@begedin if they error out or are unhandled I think that means human intervention time, not machine. We haven't written code yet to effectively handle the edge case, but we can go in and process those events since we're keeping track.

I think were in a good state to merge now.