Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle duplicated subscription created event #194

Merged
merged 1 commit into from
Aug 24, 2023
Merged

Handle duplicated subscription created event #194

merged 1 commit into from
Aug 24, 2023

Conversation

AcTiv3MineD
Copy link
Contributor

If for any reason subscription_created webhook is called more than once and the subscription was created on the first one, the subsequent calls will return a 500 status error as a result of the unique constraint on paddle_id.

This PR solves this issue by checking if the subscription exists before proceeding.

@driesvints driesvints changed the title fix: handle duplicated subscription created event Handle duplicated subscription created event Aug 24, 2023
Copy link
Member

@GrahamCampbell GrahamCampbell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kinda feels like the wrong place for this. Presumably this can happen when paddle sends two requests at the same time. It'd be better to do INSERT IGNORE on the subscription, or just catch the exception from the eloquent call.

@driesvints
Copy link
Member

@GrahamCampbell Not sure what you mean. If a subscription_created for the same subscription ID comes in twice it's most likely a network fluke or retry gone wrong. The events should be exactly the same and the second one can safely be ignored.

@GrahamCampbell
Copy link
Member

That's not what the issue is. The issue is two requests come in, both check the DB and see it doesn't exist, then both try to do the insert.

@driesvints
Copy link
Member

@GrahamCampbell I still don't follow... surely the second requests sees that the subscription has been added by the first request?

@GrahamCampbell
Copy link
Member

Not if the first request has not gotten as far as doing the insert when the second does the select.

@driesvints
Copy link
Member

@GrahamCampbell but that would be the same situation we're in right now. We're currently talking if the second request comes in at a later point.

@AcTiv3MineD
Copy link
Contributor Author

@GrahamCampbell Sorry if the description is not very clear.

The issue is as @driesvints described, the first call can "fail" because of network, maybe a listener execution went wrong or even if you hit retry on Paddle's developer tool.

The proposed approach mimics what already exists on:
handleSubscriptionPaymentSucceeded and handlePaymentSucceeded

I thought of checking if there was a subscription first and then proceeding with the execution, but this could lead to Replay attacks, thus, just stopping the execution like the previous endpoints do is the safest option.

@taylorotwell taylorotwell merged commit 1185919 into laravel:1.x Aug 24, 2023
9 checks passed
@AcTiv3MineD AcTiv3MineD deleted the fix-handle-duplicated-subscription-created-event branch June 3, 2024 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants