This repository was archived by the owner on Jan 5, 2026. It is now read-only.
Repository navigation
Fix NamedPipe reconnection - #6526
Merged
Merged
Conversation
fangyangci (woasishen)
force-pushed
the
main
branch
2 times, most recently
from
October 23, 2022 18:03
0880fbc to
84dbf43
Compare
fangyangci (woasishen)
force-pushed
the
main
branch
from
October 24, 2022 08:36
3309e9b to
ac84efb
Compare
xieofxie
reviewed
Oct 25, 2022
xieofxie
reviewed
Oct 25, 2022
tracyboehrer (tracyboehrer)
approved these changes
Oct 25, 2022
Member
|
If the others are good with this, go ahead and merge. Scheduled to get a release out this week, and would like to get this in. Trying to cut an RC build today. |
Member
|
Ping on this. Good to go or should it waiting until next release, which will be in December. |
Muthuveer Somanathan (msomanathan)
approved these changes
Oct 25, 2022
tracyboehrer (tracyboehrer)
pushed a commit
that referenced
this pull request
Oct 25, 2022
* Fix NamedPipe reconnection * fix comments * fix coding bug * remove unused property IsConnected * fix typo Co-authored-by: fangyangci <fangyangci@microsoft.com>
This was referenced Aug 31, 2026
This was referenced Sep 11, 2026
This was referenced Sep 24, 2026
This was referenced Oct 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6306
Description
NamedPipe Reconnection did not fix well by the previous PR for this issue(#6306), the disconnect issue(ib/ob false) still occur.
This pr add retry in NamedPipeTransport, and will fix NamedPipe ib/ob false issue, after customer update their SDK and I publish new version of DL-ASE.
This change is backward compatibility, verified in Testing.
Specific Changes
Because NamedPipeTransport.ConnectAsync implemetations handle both connectAsync and processAsync, can't find out the correct time to set the IsConnected status outside.
This does happen sometimes in Azure.
Testing
We have two program, bot and ase, they use namedPipeTransport as a pipe to comunicate.
This pr will update the SDK which is used by both bot and ase.
So I call the program before this change "old bot"/ "old ase".
And call the program after this change "new bot"/ "new ase".
There are more than one bugs in the NamedPipe connect/reconnect.
Some were fixed in BotBuild SDK4.17, and some were fixed in our ase version 1.1.0.0 which was published early this year. But still connection issue hanppends.
Below I verified all kinds of scenarios I can think of. Because some scenarios were hardly reproduce in Azure, so I verified them locally.
Here is my verified scenarios.
My conclusion is: after both bot and ase update their SDK, all issue will be fixed. And if customer just update one of them, won't bring in any harm to their service.