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.

Sign up for GitHub

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

Jump to bottom

Emit disconnect_document with errorType to FRS on FF client disconnects #23958

Draft
wants to merge 68 commits into
base: main
Choose a base branch
from
Draft

Emit disconnect_document with errorType to FRS on FF client disconnects #23958

wants to merge 68 commits into from

Conversation

RishhiB
Copy link
Contributor

@RishhiB RishhiB commented Feb 28, 2025

Currently, when clients disconnect, no specific reason is sent to FRS, making it difficult to distinguish between normal disconnects and data corruption cases. This PR adds an event disconnect_document which will be emitted from R11sDocumentDeltaConnection to FRS if the error that is causing the disconnect is a FluidErrorTypes.dataCorruptionError

Changes made

  1. Add a disconnect_document event emission in R11sDocumentDeltaConnection when disconnecting

  2. Capturing error details including:

    • Client ID
    • Document ID
    • Error type
    • Corruption status (based on error type)

  3. Adding a server-side handler in the Nexus lambda to log these disconnection events via Lumberjack, anything beyond this is out of scope of this PR

AB#13598

This is a copy of #23550 with Jason's feedback

RishhiB and others added 30 commits January 14, 2025 21:06
@RishhiB
Cherry-pick loader and container-definitions from microsoft#23268
c8fb4b5
@RishhiB
Merge branch 'main' into cherry-pick-disconnectReason-loader
4e867dc
@RishhiB
api extractor, format and dev tools
36988b3
@RishhiB
document delta connector cherry-pick
b3664f9
@RishhiB
cherry-pick- server fluidContainer
f1bd575
@RishhiB
add jsdoc and format
b5bf87d
@RishhiB
Merge branch 'main' into cherry-pick-disconnectReason-loader
089e75f
@RishhiB
extract string from error in disconnectCore
9214ea2
@RishhiB
remove extra jsdoc
590d091
@RishhiB
move DisconnectReason to core-interfaces
c19ab83
@RishhiB
remove type
1a482d5
@RishhiB @alexvy86
Update packages/common/core-interfaces/src/disconnectReason.ts
3a78424 
Co-authored-by: Alex Villarreal <[email protected]>
@RishhiB
feedback
49b3125
@RishhiB
enforce error object when Corruption reason
863e7d5
@RishhiB
wip
8421721
@RishhiB
format
0f7d687
@RishhiB
Merge branch 'main' into cherry-pick-disconnectReason-loader
861e707
@RishhiB
fix back compat
deff9ff
@RishhiB
Merge branch 'main' into cherry-pick-disconnectReason-loader
4ab55a7
@RishhiB
undo random delete
eb1a10f
@RishhiB
format
95f63ed
@RishhiB
prettier for server
19ebadb
@RishhiB
undo loader changes
1d5e99c
@RishhiB
fix typo
8730cb1
@RishhiB
api-doc
65960f4
@RishhiB
remove IContainer changes
a5d3b35
@RishhiB
Merge branch 'main' into cherry-pick-disconnectReason-loader
5c14139
@RishhiB
change to internal
fe68cc7
@RishhiB
wip
03db41d
@RishhiB
isCorruption
bf9844a
RishhiB added 18 commits February 13, 2025 21:49
@RishhiB
format
e756050
@RishhiB
Merge branch 'main' into cherry-pick-disconnectReason-loader
726eca4
@RishhiB
Revert "use helpers to generate tests"
c49ad13 
This reverts commit 3e8df2e.
@RishhiB
Merge branch 'main' into cherry-pick-disconnectReason-loader
0a1701a
@RishhiB
add error back in
69e6b18
@RishhiB
Merge branch 'main' into cherry-pick-disconnectReason-loader
386d400
@RishhiB
remove data processing error
87e872e
@RishhiB
design change
ff6b536
@RishhiB
fix tests
61b36ed
@RishhiB
dont log if errorType is undefined
3b83f4a
@RishhiB
update to use dispose instead of disconnect
a61344f
@RishhiB
Merge branch 'main' into cherry-pick-disconnectReason-loader
95940af
@RishhiB
fix tests
7701d53
@RishhiB
Revert "update to use dispose instead of disconnect"
f6550cc 
This reverts commit a61344f.
@RishhiB
generate tests
3e3b052
@RishhiB
zach's feedback
159ad19
@RishhiB
Merge branch 'main' into cherry-pick-disconnectReason-loader
cf3ffa4
@RishhiB
jason's feedback
054c761
@RishhiB RishhiB requested review from Copilot and jason-ha and removed request for Copilot February 28, 2025 21:50
@github-actions github-actions bot added base: main PRs targeted against main branch area: driver Driver related issues area: server Server related issues (routerlicious) labels Feb 28, 2025
@github-actions github-actions bot removed the area: server Server related issues (routerlicious) label Feb 28, 2025
@RishhiB RishhiB marked this pull request as draft March 3, 2025 17:54
jason-ha
jason-ha reviewed Mar 3, 2025
View reviewed changes
Copy link
Contributor

@jason-ha jason-ha left a comment

Choose a reason for hiding this comment

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

LMK when actually ready for review (build failed and not showing all of the changes)
Should probably clone the outstanding issues from the prior PR and add those folks to PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jason-ha jason-ha jason-ha left review comments

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
area: driver Driver related issues base: main PRs targeted against main branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@RishhiB @jason-ha