arlyon/async-stripe

Override NotificationEventData with the EventData object, and remove WebhookEvent

jblachly opened this issue · 5 comments

Describe the bug

Thanks for this fantastic library!

Stripe webhooks send events (https://stripe.com/docs/api/events).

The Event struct is defined as (partially)

...
pub data: NotificationEventData,
...

(source: https://docs.rs/async-stripe/0.18.2/stripe/struct.Event.html)

Examination of NotificationEventData shows that it's a unit struct with no impl.

However, there is another type, enum EventData (https://docs.rs/async-stripe/0.18.2/stripe/struct.EventData.html) that looks like it should occupy that field instead.

As a consequence of all this, deserializing stripe webhook Event JSON into Event always produces an empty data field:

{"api_version":"2022-11-15","created":1676683497,"data":{},"id":"evt_1McfAbIHpLLUnAWDGwPuZqiP","livemode":false,"pending_webhooks":2,"request":{"id":"req_IyoaUtGIeIh3hf","idempotency_key":"7526092e-b6f5-4ecc-ab96-2448c354855f"},"type":"customer.created"}

When I instead deserialize into raw serde_json::Value, we can see the webhook POSTed a more complete customer object:

{"api_version":"2022-11-15","created":1676683841,"data":{"object":{"address":null,"balance":0,"created":1676683841,"currency":null,"default_source":null,"delinquent":false,"description":"(created by Stripe CLI)","discount":null,"email":null,"id":"cus_NNQ6MntvErqcPD","invoice_prefix":"250FDE08","invoice_settings":{"custom_fields":null,"default_payment_method":null,"footer":null,"rendering_options":null},"livemode":false,"metadata":{},"name":null,"next_invoice_sequence":1,"object":"customer","phone":null,"preferred_locales":[],"shipping":null,"tax_exempt":"none","test_clock":null}},"id":"evt_1McfG9IHpLLUnAWD51Hr6Yzi","livemode":false,"object":"event","pending_webhooks":2,"request":{"id":"req_zeSLVaMU1NXBZW","idempotency_key":"a42e4efb-ac71-4675-83ec-50aeb75eae29"},"type":"customer.created"}

Unfortunately I can't propose a fix because I don't (yet) have a handle on how the automatic generation works from OpenAPI schema.

To Reproduce

  1. Set up webhook
  2. Deserialize POSTed data into stripe::Event
  3. Alternatively, deserialize POSTed data into serde_json::Value and observe difference

Expected behavior

The Event object's data field should be of type EventData, not NotificationEventData

Code snippets

No response

OS

mac os x 12

Rust version

rustc 1.67.0 (fc594f156 2023-01-24)

Library version

0.18.2

API version

2022-11-15

Additional context

No response

I noticed the same issue, and can confirm that making my own version of the Event struct with the type-swapped field pub data: EventData seems to work.

I notice that Webhook::construct_event delivers a type WebhookEvent rather than Event. WebhookEvent has field data: EventData, so it solves the problem I was having with Event.

Note also that construct_event validates the webhook payload signature, so that's probably the right way to handle incoming webhook events.

I'm not sure why there are two types (Event and WebhookEvent) that seem to represent the same stripe object, but if they're both needed then I think Event should still be fixed because e.g. Event::retrieve returns the Event missing the data contents.

Hi! The reason we have two types is because NotificationEventData doesn't output in the correct format. To be honest, I think we should have just overridden this case originally but I don't use webhooks personally and this is a very big library :)

I agree that WebhookEvent is probably not really needed, but we currently don't have any mechanism for overriding certain types so that is why it is there.

(also, sorry for the late reply, I accidentally dismissed this notification originally it seems)

@arlyon can I ask what you mean by "NotificationEventData doesn't output in the correct format"?

I tried replacing NotificationEventData with EventData, and it did work for me, so I guess I'm not understanding what the issue is.

In my experience WebhookEvent works well (i.e. I am able to deserialize webhook events) but Event is a bit broken (most of the data I would want is trapped behind the data field that doesn't get deserialized). So I hope WebhookEvent doesn't go away in favor of today's Event implementation.

The stripe api doesn't specify what NotificationEventData should actually be and so when the codegen generates that type, it generates an empty struct. This is not a deficiency of the code generation, the stripe openapi definitions literally just don't give us any more info than the fact it is an 'object'.

The goal is to remove as much hand-written code as possible, so if we build a mechanism to override NotificationEventData with the correct type, then the auto-generated Event type is correct, and we can remove our hand-maintained copy.

Upon further investigation, we can actually just rename this and export it with the correct type instead.