Distant chat: closing a conversation left its contact behind, and always reported success - #378
Open
jolavillette wants to merge 1 commit into
Open
Conversation
…ays said it worked
closeDistantChatConnexion() closes the GXS tunnel and stops there, on a comment
wondering whether the contact should go too ("also remove contact. Or do we wait
for the notification?"). It should: handleRecvDataItem() drops incoming items
only for tunnels it finds no contact for, so the entry left in
mDistantChatContacts keeps accepting data for a conversation the user has just
left. A peer that keeps writing re-digs the tunnel and the chat comes back.
markDistantChatAsClosed(), which runs when the *remote* end closes, has always
removed it; closing it ourselves now means the same thing, and that function
becomes the same call.
The return value was the constant true, so every caller -- the chat window, the
web UI, any JSON API client -- was told the conversation had been closed even
when there was nothing to close: closeExistingTunnel() answers false and logs
"Cannot close distant tunnel connection. No connection openned for tunnel id"
when the tunnel had already died on its own, which is the normal state of a
conversation left open for a while. It now answers whether anything was actually
released, so a client can tell the user which of the two happened.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
DistantChatService::closeDistantChatConnexion()closes the GXS tunnel andstops there, on a comment wondering whether the contact should go with it:
It should.
handleRecvDataItem()drops an incoming item only for tunnels itfinds no contact for, so the entry left in
mDistantChatContactskeepsaccepting data for a conversation the user has just left: a peer who carries on
writing re-digs the tunnel and the chat comes back.
markDistantChatAsClosed(), which runs when the remote end closes, has alwaysremoved it — closing it ourselves now means the same thing, and that function
becomes the same call.
The return value was the constant
true, so every caller — the chat window, theweb UI, any JSON API client — was told the conversation had been closed even
when there was nothing to close. That is not a rare case:
closeExistingTunnel()answers false and logswhenever the tunnel died on its own first, which is the normal state of a
conversation left open for a while. It now answers whether anything was actually
released, so a client can tell the user which of the two happened instead of
claiming a close that never took place.
Found from the web UI, where "Leave Chat" reported a closed tunnel and the
conversation kept coming back. Built and run on Linux/Qt5.
🤖 Generated with Claude Code