-
Notifications
You must be signed in to change notification settings - Fork 446
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(core): add detection and recovery for missing mutation events #7576
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
No changes to documentation |
Component Testing Report Updated Oct 7, 2024 3:50 PM (UTC) ✅ All Tests Passed -- expand for details
|
⚡️ Editor Performance ReportUpdated Mon, 07 Oct 2024 16:02:37 GMT
Detailed information🏠 Reference resultThe performance result of
🧪 Experiment resultThe performance result of this branch
📚 Glossary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good!
packages/sanity/src/core/store/_legacy/document/utils/eventChainUtils.ts
Show resolved
Hide resolved
packages/sanity/src/core/store/_legacy/document/utils/sequentializeListenerEvents.ts
Outdated
Show resolved
Hide resolved
} | ||
} | ||
|
||
const DEFAULT_MAX_BUFFER_SIZE = 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may be worth mentioning the hard coded value in buffered document in a comment above
packages/sanity/src/core/store/_legacy/document/utils/sequentializeListenerEvents.ts
Outdated
Show resolved
Hide resolved
packages/sanity/src/core/store/_legacy/document/utils/sequentializeListenerEvents.ts
Outdated
Show resolved
Hide resolved
packages/sanity/src/core/store/_legacy/document/getPairListener.ts
Outdated
Show resolved
Hide resolved
packages/sanity/src/core/store/_legacy/document/utils/__test__/sequentializeEvents.test.ts
Outdated
Show resolved
Hide resolved
packages/sanity/src/core/store/_legacy/document/getPairListener.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👩🍳💋
…ror` (#7595) ### Description The following builds off of #7576. This PR: - Propagates the `PairListenerOptions` through the document-store - Adds telemetry for when the `onSyncErrorRecovery` fires. ### What to review - Did I cover all APIs that need to propagate this? Seemed best to add it to the `document-store` options instead of each individual method - Am I logging the telemetry event correctly? ### Testing - There were no existing tests for the document-store so I just manually tested that this fired - I also went through all the types and ensured that the option was propagated correctly ### Notes for release N/A - this one builds off of #7576 but is mostly internal changes --------- Co-authored-by: Bjørge Næss <bjoerge@gmail.com>
Description
In very rare cases, the /listen endpoint may drop mutation events. This PR implements detection of "holes" in the received mutation events, and implements error recovery by restarting the connection.
What to review
Are the threshold values sensible? Error detection and recovery is now triggered by the following thresholds.
DEFAULT_MAX_BUFFER_SIZE=20
: If we se more than 20 mutation events that can't be applied in order we treat it as an errorDEFAULT_DEADLINE_MS=30_000
: If 30 seconds pass since we last received a message that can't be applied in order we treat it as an errorTesting
This PR includes unit tests and has also undergone extensive manual testing.
Notes for release