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

fix(client): strip unknown data received from webhook #332

Merged
merged 1 commit into from
Feb 1, 2022

Conversation

samuelmasse
Copy link
Contributor

@samuelmasse samuelmasse commented Jan 31, 2022

Since the messaging api might add stuff to its webhook payload, we shouldn't enforce that unknown keys are not present in the payload since this would prevent us in the future from upgrading the messaging api without having to upgrade everything that listens to its events at the same time.

The contract of compatibility with the API user is : when making a request, the data that you were previously sent back will always be sent back, but don't assume that any additional data will not be added to the response in the future. This applies to webhooks as well. We don't guarantee the API user that more data will not be added to the webooks payload, but we assure that the data that was there in the past will still be there.

When making API request, we don't use stripUnknown because it is not part of the contract. We are free to add additional fields later that we accept in request, but older versions of the API shouldn't accept them.

This contract assures compatibility of the API when moving from a older version to a newer version of the messaging API.

Closes DEV-2274

@linear
Copy link

linear bot commented Jan 31, 2022

DEV-2274 Add stripUnknown to the messaging client webhook receiver

This is required to allow us to retroactively add more stuff to the payloads without breaking previous versions

@samuelmasse samuelmasse merged commit 84a4baa into master Feb 1, 2022
@samuelmasse samuelmasse deleted the sm-client-strip-unknown branch February 1, 2022 00:10
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.

2 participants