Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Commit

Permalink
Errors in the autopinger when sending pings no longer bubble up unhan…
Browse files Browse the repository at this point in the history
…dled (#3351)

# Summary

A developer noticed that if the autopinger is the next thing in line to send a message, _right_ as the connection has died, the ‘connection closed’ error will bubble up through the `sendPing()` message, unhandled, and will crash the app.

In this PR we make changes to this:

1. Any error is handled and discarded
2. Furthermore, if that error is a ‘connection closed’ error, we stop the pinger
  • Loading branch information
steveluscher authored Oct 8, 2024
1 parent 958385b commit 40dcf3b
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { SOLANA_ERROR__RPC_SUBSCRIPTIONS__CHANNEL_CONNECTION_CLOSED, SolanaError } from '@solana/errors';
import { RpcSubscriptionsChannel } from '@solana/rpc-subscriptions-spec';

import { getRpcSubscriptionsChannelWithAutoping } from '../rpc-subscriptions-autopinger';
Expand Down Expand Up @@ -68,6 +69,29 @@ describe('getRpcSubscriptionsChannelWithAutoping', () => {
}),
);
});
it('continues to ping even though send fataled with a non-connection-closed exception', async () => {
expect.assertions(1);
mockSend.mockRejectedValue(
// Anything other than `SOLANA_ERROR__RPC_SUBSCRIPTIONS__CHANNEL_CONNECTION_CLOSED`.
'o no',
);
getRpcSubscriptionsChannelWithAutoping({
abortSignal: new AbortController().signal,
channel: mockChannel,
intervalMs: MOCK_INTERVAL_MS,
});
// First ping.
await jest.advanceTimersByTimeAsync(MOCK_INTERVAL_MS);
// Second ping.
mockSend.mockClear();
await jest.advanceTimersByTimeAsync(MOCK_INTERVAL_MS);
expect(mockSend).toHaveBeenCalledWith(
expect.objectContaining({
jsonrpc: '2.0',
method: 'ping',
}),
);
});
it('does not send a ping until interval milliseconds after the last sent message', async () => {
expect.assertions(3);
const autopingChannel = getRpcSubscriptionsChannelWithAutoping({
Expand Down Expand Up @@ -132,6 +156,21 @@ describe('getRpcSubscriptionsChannelWithAutoping', () => {
await jest.advanceTimersByTimeAsync(MOCK_INTERVAL_MS);
expect(mockSend).not.toHaveBeenCalled();
});
it('does not send a ping after send fatals with a connection closed error', async () => {
expect.assertions(1);
mockSend.mockRejectedValue(new SolanaError(SOLANA_ERROR__RPC_SUBSCRIPTIONS__CHANNEL_CONNECTION_CLOSED));
getRpcSubscriptionsChannelWithAutoping({
abortSignal: new AbortController().signal,
channel: mockChannel,
intervalMs: MOCK_INTERVAL_MS,
});
// First ping.
await jest.advanceTimersByTimeAsync(MOCK_INTERVAL_MS);
// No more pings.
mockSend.mockClear();
await jest.advanceTimersByTimeAsync(MOCK_INTERVAL_MS);
expect(mockSend).not.toHaveBeenCalled();
});
it('does not send a ping after the abort signal fires', async () => {
expect.assertions(2);
const abortController = new AbortController();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { isSolanaError, SOLANA_ERROR__RPC_SUBSCRIPTIONS__CHANNEL_CONNECTION_CLOSED } from '@solana/errors';
import type { RpcSubscriptionsChannel } from '@solana/rpc-subscriptions-spec';

type Config<TChannel extends RpcSubscriptionsChannel<unknown, unknown>> = Readonly<{
Expand All @@ -18,7 +19,11 @@ export function getRpcSubscriptionsChannelWithAutoping<TChannel extends RpcSubsc
}: Config<TChannel>): TChannel {
let intervalId: ReturnType<typeof setInterval> | undefined;
function sendPing() {
channel.send(PING_PAYLOAD);
channel.send(PING_PAYLOAD).catch((e: unknown) => {
if (isSolanaError(e, SOLANA_ERROR__RPC_SUBSCRIPTIONS__CHANNEL_CONNECTION_CLOSED)) {
pingerAbortController.abort();
}
});
}
function restartPingTimer() {
clearInterval(intervalId);
Expand Down

0 comments on commit 40dcf3b

Please sign in to comment.