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: sends events in order #736

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

fix: sends events in order #736

wants to merge 20 commits into from

Conversation

yuhao900914
Copy link
Contributor

@yuhao900914 yuhao900914 commented May 4, 2024

Summary

Jira AMP-98809

This change is for making sure the events sent into the backend are in order.
Previously in the Typescript SDK, we

  • Sent different promises in parallel using Promise.all, which led to race conditions.
  • Scheduled individual events based on their own timeout, which also resulted in events being sent out of order.

Current behavior:

  • In the event of an error, we filter out problematic events while keeping all other events in the queue. These events will be retried with the next trigger.

Checklist

  • Does your PR title have the correct title format?
  • Does your PR have a breaking change?:

@yuhao900914 yuhao900914 self-assigned this May 4, 2024
@yuhao900914 yuhao900914 marked this pull request as draft May 4, 2024 00:36
@yuhao900914 yuhao900914 changed the title Event in order fix: sends events in order May 4, 2024
@yuhao900914 yuhao900914 requested review from izaaz, a team and Mercy811 May 6, 2024 18:11
@yuhao900914 yuhao900914 marked this pull request as ready for review May 7, 2024 02:05
@yuhao900914 yuhao900914 requested a review from izaaz May 8, 2024 21:52
@yuhao900914 yuhao900914 requested a review from Mercy811 May 9, 2024 22:40
Copy link
Contributor

@Mercy811 Mercy811 left a comment

Choose a reason for hiding this comment

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

Thank you Alyssa. Overall looks good to me. See nits.

packages/analytics-core/src/plugins/destination.ts Outdated Show resolved Hide resolved
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.

None yet

5 participants