improvement(driver): Make error emission on disconnect optional
This PR makes the emission of errors on disconnects optional rather than required.
Currently, errors are always emitted on disconnect. However, scenarios like Container.dispose can represent a clean disconnect and shouldn't necessarily trigger an error.
This change introduces an optional error parameter to the dispose and disconnect methods in DocumentDeltaConnection. If an error is provided, it will be used; otherwise, the disconnect is treated as a clean one.
}
Looks like a breaking change to IDocumentDeltaConnectionEvents disconnect event
Refers to: packages/drivers/driver-base/src/documentDeltaConnection.ts:451 in dbdc5b1. [](commit_id = dbdc5b1aa8411d31137c4c1582fc73c563cb53cf, deletion_comment = False)
}
Looks like a breaking change to IDocumentDeltaConnectionEvents disconnect event
Refers to: packages/drivers/driver-base/src/documentDeltaConnection.ts:451 in dbdc5b1. [](commit_id = dbdc5b1, deletion_comment = False)
Ah thank you! I hope its correct now.
This code looks bogus. This appears to be listening for the local "disconnect" event and expecting two parameters. But AFAICT there is only one (reason) which may now be undefined. Am I reading this wrong?
Refers to: packages/drivers/driver-base/src/documentDeltaConnection.ts:641 in b2c5630. [](commit_id = b2c5630103f1b4c688339d0596dbd0172438fae5, deletion_comment = False)
@RishhiB Did someone ask for this change? One is that it is a breaking change and I don't think we can merge it like this as it might break our partners who will be listening to this. Also, previously we were clearly saying in the message that is "client who is closing the connection", so when they would receive that, they would not be doing anything.
@RishhiB Did someone ask for this change? One is that it is a breaking change and I don't think we can merge it like this as it might break our partners who will be listening to this. Also, previously we were clearly saying in the message that is "client who is closing the connection", so when they would receive that, they would not be doing anything.
Yes FRS asked for this. This is the prerequisite for this PR https://github.com/microsoft/FluidFramework/pull/24926, which allows sending disconnect status to FRS. The feature in ADO; https://dev.azure.com/fluidframework/internal/_workitems/edit/8411
🔗 No broken links found! ✅
Your attention to detail is admirable.
linkcheck output
> [email protected] ci:check-links /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test "npm run serve -- --no-open" 3000 check-links
1: starting server using command "npm run serve -- --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"
> [email protected] serve
> docusaurus serve --no-open
[SUCCESS] Serving "build" directory at: http://localhost:3000/
> [email protected] check-links
> linkcheck http://localhost:3000 --skip-file skipped-urls.txt
Crawling...
Stats:
231978 links
1733 destination URLs
1964 URLs ignored
0 warnings
0 errors
@RishhiB Did someone ask for this change? One is that it is a breaking change and I don't think we can merge it like this as it might break our partners who will be listening to this. Also, previously we were clearly saying in the message that is "client who is closing the connection", so when they would receive that, they would not be doing anything.
Yes FRS asked for this. This is the prerequisite for this PR #24926, which allows sending disconnect status to FRS. The feature in ADO; https://dev.azure.com/fluidframework/internal/_workitems/edit/8411
So, if the disconnection is from the socket side, we provide the error. For all other cases, where dispose is coming from the loader, as far as connection is concerned we are just saying that it is the client who is closing this conneciton. I think, we can just create an error type corresponding to the Client Closing delta connection and emit that on connection object and listener would then just check that error type and deduce why the connection get closed instead of undefined.