Potential Issue with Pusher Ping/Pong Messages

[sundayz]

Reverb Version

1.0.0-beta14

Laravel Version

10.10

PHP Version

8.1

Description

If I am understanding the Pusher protocol correctly, then if the WebSocket implementation supports native ping/pong messages, then Pusher will use those, and the pusher:ping and pusher:pong messages are only used if the client doesn't support websocket protocol ping messages.
Link to Pusher docs here

It seems like Laravel Reverb is ONLY sending pusher:ping and not using the websocket native ping messages to determine if a connection is still alive. I found this to be a compatibility issue with the official dotnet Pusher client. If I understand the situation correctly, because the C# implementation of the websocket client handles ping messages at the ws protocol level, they didn't implement any handlers for pusher:ping messages, which makes this client incompatible with Reverb.

If my assumptions are correct then this means Reverb should detect the client version and use a different pinging strategy based on the version? Or in my case could I just disable the reverb ping messages entirely and rely on the websocket pings being handled internally to keep the connection alive?

Edit: This excerpt from the Pusher docs linked above seems to imply that the C# client's behaviour is fine:

If the WebSocket draft supports protocol level ping-pong, then on receipt of a ping message, the client MUST respond with a pong message.

If the client does not support protocol level pings and advertises (on connect) that it implements a protocol version >= 5 then the client MUST respond to a pusher:ping event with a pusher.pong event.

It doesn't explicitly state that clients that already support protocol level pings must ALSO respond to pusher:ping events.

Steps To Reproduce

Set up a minimal Reverb server and minimal Pusher dotnet client (https://github.com/pusher/pusher-websocket-dotnet), wait 1 minute after connection, server drops client connection due to missed ping and client reconnects.

[sundayz]

So websocket ping/pong is handled here:

reverb/src/Servers/Reverb/Connection.php

Line 84 in 8383876

public function control(FrameInterface $message): void

Maybe this should call touch() on the Pusher connection to reset lastSeenAt, and then the pusher:ping fallback won't be necessary?

[drjamesj]

I agree with above approach, probably receiving any control frame should touch() the connection the same way any non-control frame does? But to fully meet the spec, sending a ping() or pong() should also attempt to send the control frame op-code as well?

Lastly, while playing around to check this, I found that PingInactiveConnections occurs on a fixed 60 second interval on the $loop but my hunch is that this should respect the configured ping_interval value? Otherwise it seems possible that active connections may be incorrectly treated at inactive.

reverb/src/Servers/Reverb/Console/Commands/StartServer.php

Lines 91 to 97 in 8383876

	protected function ensureStaleConnectionsAreCleaned(LoopInterface $loop): void
	{
	$loop->addPeriodicTimer(60, function () {
	PruneStaleConnections::dispatch();
	PingInactiveConnections::dispatch();
	});
	}

[joedixon]

I don't think calling touch() on receipt of the control frame fully solves the issue since the dotnet client doesn't respond to pusher:ping so Reverb will never see a response from the client when pinging inactive connections and ultimately terminate the connection.

I'll take a look to see what's involved in adding support for this.

I notice there are some other similar issues on the repo so wonder if this should be addressed by the client. Doesn't look, at first glance, that there are any checks for the advertised supported pusher protocols.

[drjamesj]

Yes but by calling touch() in response to a control frame event would update the connection's lastSeenAt and it wouldn't be included in the next round of PingInactiveConnections anyway right? So this should work as long as the client is indeed sending ping events reliably.

What do you think?

[sundayz]

I notice there are some other similar issues on the repo so wonder if this should be addressed by the client. Doesn't look, at first glance, that there are any checks for the advertised supported pusher protocols.

I believe that because the dotnet WebSocket client supports protocol level pings they didn't bother to include support for pusher:ping messages, since the spec states you can use either method for detecting inactive connections. This is likely an issue with more client implementations out there. You don't need to check the advertised pusher version to know if ping is supported, you check the Sec-Websocket-Version header on conection (I think)

[davidrushton]

We're seeing the same issue using the https://github.com/pusher/pusher-websocket-swift package where (unlike the Echo / Pusher JS client), it is not sending pusher:ping messages and therefore the connections are pruned unexpectedly

[incon]

Appears most non web based clients don't use pusher:ping so most (all?) mobile clients don't work with Reverb.

[joedixon]

Is anyone on this thread affected by the issue able to give #253 a try to see if it resolves the issue?

[davidrushton]

Is anyone on this thread affected by the issue able to give #253 a try to see if it resolves the issue?

Thanks @joedixon. I can give this a go in our Flutter mobile app in the next couple of weeks. Our workaround was just to send a client message on the connection as that keeps it alive but using internal events would be great.

[davidrushton]

@joedixon Tested the dev-fix/ping-pong branch just now and can confirm this resolves the issue for us.

Before this change (and without sending a client message):

1. Our mobile app (Pusher Channels Flutter SDK) connects okay.
2. Reverb debug shows the channels subscriptions.
3. After around 3 minutes and pinging, Reverb debug shows 'Connection Closed' and Flutter connection is lost

Debug logs:

Pruning Stale Connections .....
Pinging Inactive Connections .....
Pruning Stale Connections .....
Pinging Inactive Connections .....
Connection Pinged .....
Pruning Stale Connections .....
Connection Pruned .....
Pinging Inactive Connections .....
Connection Closed .....

With this change:

1. Our mobile app (Pusher Channels Flutter SDK) connects okay.
2. Reverb debug shows the channels subscriptions.
3. After around 3 minutes and 'Control Frame Received', the Flutter connection remains open and is not dropped

Debug logs:

Pruning Stale Connections .....
Pinging Inactive Connections .....
Control Frame Received .....

 1▕ null

Control Frame Received .....

 1▕ null

Pruning Stale Connections .....
Pinging Inactive Connections .....
Control Frame Received .....

 1▕ null

Control Frame Received .....

 1▕ null

Pruning Stale Connections .....
Pinging Inactive Connections .....
Control Frame Received .....

 1▕ null

Control Frame Received .....

 1▕ null

Appreciate the fix 🙏

[joedixon]

Thanks @davidrushton. I'll address those null values in the output.

Did you manage to get the server sending out a ping event or does the client send them too regularly and prevent the server from tiggering?

[davidrushton]

@joedixon Sorry for the delay. We just use the Flutter package defaults but let me know if you want us to test anything specific here.

[manhthang2504]

@joedixon Tested the dev-fix/ping-pong branch just now and can confirm this resolves the issue for us.

Before this change (and without sending a client message):

1. Our mobile app (Pusher Channels Flutter SDK) connects okay.
2. Reverb debug shows the channels subscriptions.
3. After around 3 minutes and pinging, Reverb debug shows 'Connection Closed' and Flutter connection is lost

Debug logs:

Pruning Stale Connections .....
Pinging Inactive Connections .....
Pruning Stale Connections .....
Pinging Inactive Connections .....
Connection Pinged .....
Pruning Stale Connections .....
Connection Pruned .....
Pinging Inactive Connections .....
Connection Closed .....

With this change:

1. Our mobile app (Pusher Channels Flutter SDK) connects okay.
2. Reverb debug shows the channels subscriptions.
3. After around 3 minutes and 'Control Frame Received', the Flutter connection remains open and is not dropped

Debug logs:

Pruning Stale Connections .....
Pinging Inactive Connections .....
Control Frame Received .....

 1▕ null

Control Frame Received .....

 1▕ null

Pruning Stale Connections .....
Pinging Inactive Connections .....
Control Frame Received .....

 1▕ null

Control Frame Received .....

 1▕ null

Pruning Stale Connections .....
Pinging Inactive Connections .....
Control Frame Received .....

 1▕ null

Control Frame Received .....

 1▕ null

Appreciate the fix 🙏

I can confirm the same with .NET Framework websocket client.