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
- Set up webhook
- Deserialize POSTed data into
stripe::Event
- 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.