Fix server keep alive timeout not properly destroying connections

[mattias-wiberg]

Issue

GitHub Issue: #2953
Reporter: @zabranskiy
Package: @grpc/grpc-js
Severity: Bug - Server connections hang indefinitely when clients don't respond to keepalive pings

Root Cause

The server-side keepalive ping timeout handler uses session.close() to terminate connections when clients fail to respond to keepalive pings. However, session.close() performs a graceful shutdown that does not forcibly terminate the underlying connection, causing it to hang indefinitely.

Fix Summary

Changed keepalive error handlers to use session.destroy() instead of session.close() to forcibly close connections when:

• Keepalive ping timeout expires without response
• Ping callback receives an error
• Sending the ping fails

Files Modified

• packages/grpc-js/src/server.ts (6 occurrences changed)
  • 3 changes in _sessionHandler method (lines ~1609, ~1635, ~1643)
  • 3 changes in _channelzSessionHandler method (lines ~1807, ~1830, ~1843)

Testing Recommendations

To verify the fix:

1. Start a gRPC server with these options:

   {
     'grpc.keepalive_time_ms': 10000,
     'grpc.keepalive_timeout_ms': 5000,
     'grpc.keepalive_permit_without_calls': 1
   }

2. Connect a client and send a request
3. Stop the client process (SIGSTOP) after receiving the first ping response
4. Observe server logs - after the timeout, the connection should be destroyed (not hanging)

Consistency

This change aligns server-side keepalive handling with the client-side transport implementation (packages/grpc-js/src/transport.ts), which already uses session.destroy() via handleDisconnect() for keepalive timeouts.

Backward Compatibility

This is a bug fix that should not affect normal operation:

• Proper keepalive implementations will continue to work normally
• Only affects scenarios where clients fail to respond to keepalive pings
• Changes do not affect the public API or configuration options

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: mattias-wiberg / name: Mattias Wiberg (2901922)

[mattias-wiberg]

Bump

[OlegStrokan]

@mattias-wiberg I think we’re the only two people on Earth who have discovered this issue. I was trying to implement internet disconnection detection for streaming, using only bare keepalive, without any additional healthcheck/ heartbeat. I even created PR to nest library because it doesn’t map keepalive settings to grpc-js. But later I discovered that the errors still didn’t propagate to the application layer in gprc-js library (I checked with gRPC tracing).
So I ended up generating more CO2 using Claude than all soccer moms combined to understand was ist das
It wasn’t a priority task, so I never proposed a change to the library. Now I see your solution and you’re a lifesaver.
I’m too scared to use a custom grpc-js fork in prod, but it seems like no code review happened. (You’ll probably get a GitHub notification and think “Oh a maintainer finally commented on my pr, but then you will see it’s just some guy with the same issue, sorry 🙃)

[murgatroid99]

Thank you for the fix.

[murgatroid99]

This is now out in version 1.14.2.