Page MenuHomePhabricator

Adding a comment to a conpherence sometimes duplicates the message in the UI
Closed, ResolvedPublic

Description

This has happened to me a couple times. For example, earlier today I tried to add a comment with a meme image and got this:

2014-12-08_17-46-40.png (860×718 px, 542 KB)

If I immediately refresh the page the duplicate doesn't exist.

A good way to reproduce seems to be to add a new user to an existing conpherence with content. The message that appears almost always causes quite a bit of duplication. For example:

screenshot.png (487×589 px, 30 KB)

I am using Firefox 33.1 on Windows 7, and a colleague was using the lastest Firefox on OSX (not sure the exact version) and see's the same issue.

Event Timeline

marpidone raised the priority of this task from to Needs Triage.
marpidone updated the task description. (Show Details)
marpidone added a project: Conpherence.
marpidone added a subscriber: marpidone.
chad triaged this task as Normal priority.Dec 8 2014, 11:59 PM
chad added a project: Transactions.
chad added a subscriber: btrahan.
chad added a subscriber: chad.

I see this often too

So for folks experiencing this, the "add participants" codepath is different than the "adding another message" codepath. Its highly likely that fixing one does not fix the other.

As such, specific reproduction steps on the "adding another message" way this reproduces are very useful...! Please include them if you have them. Thanks!

This is probably my fault, but I haven't actually seen it occur.

I have, I presume it to be some race condition with aphlict server / crappy internet connection.

Here's some exact steps that I just tested:

  1. Create a new message with myself as the only participant. Send the following messages: "Test", "Test 2", "Test 3", "Test 4".
  2. Refresh the page by hitting F5
  3. Add another participant (cspeck in this case)
  4. Refresh the page by hitting F5

This screenshot shows the results of these steps:

example.png (620×1 px, 69 KB)

Also, we are using phabricator on a LAN server, so internet connection may not be a factor. I could probably grab a copy of any relevant logs if they would help.

Do you run the aphlict server for live updates?

We do run the aphlict service for live updates.
edit: just to clarify, @marpidone and myself are working together and testing out phabricator for our purposes.

This looks to no longer happen in the use-case of adding users to conpherences. I just added several users to a conpherence and the content wasn't duplicated. The server gets updated regularly so it would've been running from a revision from last night. We haven't been regularly using conpherences so it could have been fixed within the past few weeks.

I have a reliable repro case for this after D11143. Locally, the WebSocket notification channel is dramatically faster than the flash stuff was, and consistently causes a race condition:

  • Submit chat.
  • Get a notification about it before I get the response back from the server.
  • Since the notification happens first, we trigger an update.
  • Then the response comes back from the server and we add a copy of my message.
  • Then the update comes back and we add a second copy.

I imagine this could race in similar situations with multiple users, but this one is pretty easy for me to repro (all chat repros).

Fixes might involve:

  • Keep track of which transactionPHIDs we know about?
  • Do another "what ID do we know about?" check after responses come back but before we update the thread.

I haven't seen this issue in a while, do we feel it's resolved?

There's still a race condition that we should fix, the window is just a lot narrower.

This was fixed but D12104 breaks things again as the fix here was making basic updates a little wonky. I'll take another crack at fixing this. Code gets tighter each pass at least. :D

I think I fixed this too with the recent changes. If not, how can I reproduce?

btrahan raised the priority of this task from Normal to High.
btrahan added a subscriber: hach-que.

Current repro for durable column

  • open durable column
  • spam it
  • see dupes

I think I just need to make this transaction aware and render messages per transaction. Keeping track of updates that are in flight and adjusting as necessary hasn't proven to be a winning strategy for me so far.

I still occasionally see messages being duplicated. I updated this past weekend.

Reopening in case that sets off Herald for some people.

Can you please re-open with reproduction instructions? Algorithmically speaking, this is impossible AFAIK because we now use the unique transaction ids to guarantee we only render each transaction once.

@sshannin is sending me screenshots where this is occurring in the durable column view with asynchronous updates via other users sending messages so I'll take a poke around today in case there's something one off in that code path.

To document

  • every message @btrahan sent me came in duplicated at first (first screenshot)
  • None of the message I sent showed as duplicated for me
  • Suspecting maybe some weird firefox behavior, I opened a chrome side-by-side
  • My cross-browser messages also showed duplicated (second screenshot)

This all was on secure.phab within the past few minutes.

dup.png (1×1 px, 450 KB)

dup2 (1×1 px, 461 KB)

Thanks for the persistence @sshannin. :D

If after updating to HEAD People are still experiencing issues please do re-open with reproduction instructions. Thanks!