Duplicate desktop notifications for same chat.

[mallenexpensify]

@garrettmknight has only Chat open on Desktop and is getting multiple notifications.
It could be related to this (https://github.com/Expensify/Expensify/issues/137041), but my hunch is no.
Also.. I'm not sure if it's directly related to this (#331), because this issue appears to be more about web and e.com vs chat.e.com.

cc @marcaaron and @chiragsalian since I think you've both worked on this

[ryanldonato]

I'm experiencing the same issue

[twisterdotcom]

+1, this is back today for me.

[mallenexpensify]

@marcaaron or @Jag96, can you look into this? It looks like dupe issues was fixed in the below that it looks like you two worked on. #333 and #357
I rarely get them but got a dupe this morn on Desktop

[marcaaron]

I think @tgolen is working on fixing this already where we are reimplementing the active client manager #469

[mallenexpensify]

@ryanldonato, when Chirag's update goes through it'll fix your issue because you're on web and the notifications that are coming from expensify.com will stop and you'll only get them via chat.expensify.com

[AndrewGable]

FYI - I get 10x dupes per chat now

[tgolen]

Can you add a screenshot of the JS console (in verbose logging mode) and filter it for [NOTIFICATION]? I'd be interested to see what that looks like.

[AndrewGable]

[twisterdotcom]

I got nothing for that:

[tgolen]

I looked into this a little more this morning with the help of Ted, and here is what I've been able to determine so far.

1. From looking at the server logs, the pusher event is only getting sent once
2. The pusher client on the website ends up having multiple handlers subscribed to the same report comment event
3. I think this is happening because of this line of code which will call resolve() every time the socket is connected (this happens every time pusher reconnects to the network).
4. Each time resolve() is called then subscribeToReportCommentEvents() is also called, which is what is adding the multiple handlers

This is my theory anyway. I've been attempting to reproduce it locally without any luck. cc @marc

[tgolen]

I could be wrong about that because this SO suggests that multiple calls to resolve() won't do anything, and it looks like I can confirm that.

So I don't know how we are subscribing to the same event multiple times, but I saw that it only happened after pusher had reconnected...

[marcaaron]

Here's where we do the work of subscribing and will only subscribe if we are not already subscribed so my first guess would be that Pusher has unsubscribed from the channel - but maybe has not cleared out it's callback map so new callbacks can be added...?

https://github.com/Expensify/ReactNativeChat/blob/8fe5d09579a85f5b59af9e609144df452927d523/src/lib/actions/Report.js#L221-L230

It gets called from here:
https://github.com/Expensify/ReactNativeChat/blob/8fe5d09579a85f5b59af9e609144df452927d523/src/page/home/HomePage.js#L38

So maybe the HomePage component is re-mounting causing multiple event callbacks? Possibly during a redirect of some kind?

[marcaaron]

Either way, one idea is to treat these events like any other event we only want to attach one single callback to and make use of unbind just like we do in jQuery with off().on() see Pusher JS lib doc section on this here.

[marcaaron]

Hmm so that didn't work. I think this is a possible bug with the Pusher library itself. Calling socket.disconnect() and socket.connect() results in multiple event callbacks. Tested this out by calling Pusher.reconnect() method multiple times which is just doing this...

https://github.com/Expensify/ReactNativeChat/blob/13f818ba65ddbfb2b5fcd33945c8063877f9fe8f/src/lib/Pusher/pusher.js#L357-L363

And each time a new callback was added. Not really at all what I would suspect so will keep digging here and possibly open an issue with Pusher about it.

[marcaaron]

Ok got it. Not a Pusher bug. Expensify 🐛

[marcaaron]

Tested the theory above again but this time subscribed with a new event directly to a channel and did not observe the same behavior. Then figured it must be something we are doing and it was. We were binding the subscription_succeeded event which would re-fire when calling connect thus binding another event to the channel here:

https://github.com/Expensify/ReactNativeChat/blob/0c949268e8c696e90f350b2e0eb64fca7ff8c20a/src/lib/Pusher/pusher.js#L191-L192

[tgolen]

Aha! Great sleuthing, Marc!