Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: [web3] only ever send RPC socket messages when the socket is open #29195

Merged
merged 1 commit into from
Jan 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions web3.js/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ function generateConfig(configType, format) {
'jayson/lib/client/browser',
'node-fetch',
'rpc-websockets',
'rpc-websockets/dist/lib/client',
'rpc-websockets/dist/lib/client/client.types',
'rpc-websockets/dist/lib/client/websocket',
'rpc-websockets/dist/lib/client/websocket.browser',
'superstruct',
];
}
Expand Down Expand Up @@ -179,6 +183,10 @@ function generateConfig(configType, format) {
'node-fetch',
'react-native-url-polyfill',
'rpc-websockets',
'rpc-websockets/dist/lib/client',
'rpc-websockets/dist/lib/client/client.types',
'rpc-websockets/dist/lib/client/websocket',
'rpc-websockets/dist/lib/client/websocket.browser',
'superstruct',
];

Expand Down
1 change: 1 addition & 0 deletions web3.js/src/__forks__/browser/rpc-websocket-factory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export {default} from 'rpc-websockets/dist/lib/client/websocket.browser';
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export {default} from 'rpc-websockets/dist/lib/client/websocket.browser';
9 changes: 7 additions & 2 deletions web3.js/src/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import {
any,
} from 'superstruct';
import type {Struct} from 'superstruct';
import {Client as RpcWebSocketClient} from 'rpc-websockets';
import RpcClient from 'jayson/lib/client/browser';
import {JSONRPCError} from 'jayson';

Expand All @@ -36,6 +35,7 @@ import fetchImpl, {Response} from './fetch-impl';
import {DurableNonce, NonceAccount} from './nonce-account';
import {PublicKey} from './publickey';
import {Signer} from './keypair';
import RpcWebSocketClient from './rpc-websocket';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We create RpcWebSocketClient like this later:

this._rpcWebSocket = new RpcWebSocketClient(this._rpcWsEndpoint, {
      autoconnect: false,
      max_reconnects: Infinity,
    });

it seems that max_reconnects: Infinity is a bit misleading here, as it is going to be overwritten by explicitly hardcoded max_reconnects: 5 in the rpc-websocket.ts file?

import {MS_PER_SLOT} from './timing';
import {
Transaction,
Expand Down Expand Up @@ -5803,7 +5803,12 @@ export class Connection {
this._rpcWebSocketConnected = true;
this._rpcWebSocketHeartbeat = setInterval(() => {
// Ping server every 5s to prevent idle timeouts
this._rpcWebSocket.notify('ping').catch(() => {});
(async () => {
try {
await this._rpcWebSocket.notify('ping');
// eslint-disable-next-line no-empty
} catch {}
})();
}, 5000);
this._updateSubscriptions();
}
Expand Down
4 changes: 4 additions & 0 deletions web3.js/src/rpc-websocket-factory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import {ICommonWebSocketFactory} from 'rpc-websockets/dist/lib/client/client.types';
import WebsocketFactory from 'rpc-websockets/dist/lib/client/websocket';

export default WebsocketFactory as ICommonWebSocketFactory;
79 changes: 79 additions & 0 deletions web3.js/src/rpc-websocket.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import RpcWebSocketCommonClient from 'rpc-websockets/dist/lib/client';
import RpcWebSocketBrowserFactory from 'rpc-websockets/dist/lib/client/websocket.browser';
import {
ICommonWebSocket,
IWSClientAdditionalOptions,
NodeWebSocketType,
NodeWebSocketTypeOptions,
} from 'rpc-websockets/dist/lib/client/client.types';

import createRpc from './rpc-websocket-factory';

interface IHasReadyState {
readyState: WebSocket['readyState'];
}

export default class RpcWebSocketClient extends RpcWebSocketCommonClient {
private underlyingSocket: IHasReadyState | undefined;
constructor(
address?: string,
options?: IWSClientAdditionalOptions & NodeWebSocketTypeOptions,
generate_request_id?: (
method: string,
params: object | Array<any>,
) => number,
) {
const webSocketFactory = (url: string) => {
const rpc = createRpc(url, {
autoconnect: true,
max_reconnects: 5,
reconnect: true,
reconnect_interval: 1000,
...options,
});
if ('socket' in rpc) {
this.underlyingSocket = (
rpc as ReturnType<typeof RpcWebSocketBrowserFactory>
).socket;
} else {
this.underlyingSocket = rpc as NodeWebSocketType;
}
return rpc as ICommonWebSocket;
};
super(webSocketFactory, address, options, generate_request_id);
}
call(
...args: Parameters<RpcWebSocketCommonClient['call']>
): ReturnType<RpcWebSocketCommonClient['call']> {
const readyState = this.underlyingSocket?.readyState;
if (readyState === 1 /* WebSocket.OPEN */) {
return super.call(...args);
}
return Promise.reject(
new Error(
'Tried to call a JSON-RPC method `' +
args[0] +
'` but the socket was not `CONNECTING` or `OPEN` (`readyState` was ' +
readyState +
')',
),
);
}
notify(
...args: Parameters<RpcWebSocketCommonClient['notify']>
): ReturnType<RpcWebSocketCommonClient['notify']> {
const readyState = this.underlyingSocket?.readyState;
if (readyState === 1 /* WebSocket.OPEN */) {
return super.notify(...args);
}
return Promise.reject(
new Error(
'Tried to send a JSON-RPC notification `' +
args[0] +
'` but the socket was not `CONNECTING` or `OPEN` (`readyState` was ' +
readyState +
')',
),
);
}
}
2 changes: 1 addition & 1 deletion web3.js/test/connection-subscriptions.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import chai from 'chai';
import {Client} from 'rpc-websockets';
import {stub, SinonStubbedInstance, SinonSpy, spy} from 'sinon';
import sinonChai from 'sinon-chai';

Expand All @@ -15,6 +14,7 @@ import {
SlotChangeCallback,
SlotUpdateCallback,
} from '../src';
import type Client from '../src/rpc-websocket';
import {url} from './url';

chai.use(sinonChai);
Expand Down
2 changes: 1 addition & 1 deletion web3.js/test/connection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ import {
stubRpcWebSocket,
restoreRpcWebSocket,
mockRpcMessage,
} from './mocks/rpc-websockets';
} from './mocks/rpc-websocket';
import {
NonceInformation,
TransactionInstruction,
Expand Down
2 changes: 1 addition & 1 deletion web3.js/test/mocks/rpc-http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import bs58 from 'bs58';
import BN from 'bn.js';
import * as mockttp from 'mockttp';

import {mockRpcMessage} from './rpc-websockets';
import {mockRpcMessage} from './rpc-websocket';
import {Connection, PublicKey, Transaction, Signer} from '../../src';
import invariant from '../../src/utils/assert';
import type {Commitment, HttpHeaders, RpcParams} from '../../src/connection';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import {Client as LiveClient} from 'rpc-websockets';
import {expect} from 'chai';
import {createSandbox} from 'sinon';

import {Connection} from '../../src';
import LiveClient from '../../src/rpc-websocket';

type RpcRequest = {
method: string;
Expand Down Expand Up @@ -55,6 +55,7 @@ export const stubRpcWebSocket = (connection: Connection) => {
.callsFake((method: string, params: any) => {
return mockClient.call(method, params);
});
sandbox.stub(rpcWebSocket, 'notify');
};

export const restoreRpcWebSocket = (connection: Connection) => {
Expand Down
2 changes: 1 addition & 1 deletion web3.js/test/nonce.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
import {NONCE_ACCOUNT_LENGTH} from '../src/nonce-account';
import {MOCK_PORT, url} from './url';
import {helpers, mockRpcResponse, mockServer} from './mocks/rpc-http';
import {stubRpcWebSocket, restoreRpcWebSocket} from './mocks/rpc-websockets';
import {stubRpcWebSocket, restoreRpcWebSocket} from './mocks/rpc-websocket';

const expectedData = (authorizedPubkey: PublicKey): [string, string] => {
const expectedData = Buffer.alloc(NONCE_ACCOUNT_LENGTH);
Expand Down
2 changes: 1 addition & 1 deletion web3.js/test/transaction-payer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
import invariant from '../src/utils/assert';
import {MOCK_PORT, url} from './url';
import {helpers, mockRpcResponse, mockServer} from './mocks/rpc-http';
import {stubRpcWebSocket, restoreRpcWebSocket} from './mocks/rpc-websockets';
import {stubRpcWebSocket, restoreRpcWebSocket} from './mocks/rpc-websocket';

describe('Transaction Payer', () => {
let connection: Connection;
Expand Down
12 changes: 10 additions & 2 deletions web3.js/test/websocket.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,15 @@ if (process.env.TEST_LIVE) {
expect(connection._rpcWebSocketHeartbeat).not.to.eq(null);

// test if socket is open
await connection._rpcWebSocket.notify('ping');
let open = false;
while (!open) {
try {
await connection._rpcWebSocket.notify('ping');
open = true;
} catch {
continue;
}
}

await connection.removeSignatureListener(id);
expect(connection._rpcWebSocketConnected).to.eq(false);
Expand All @@ -38,7 +46,7 @@ if (process.env.TEST_LIVE) {

// test if socket is closed
await expect(connection._rpcWebSocket.notify('ping')).to.be.rejectedWith(
'socket not ready',
'Tried to send a JSON-RPC notification `ping` but the socket was not `CONNECTING` or `OPEN` (`readyState` was 3)',
);
});

Expand Down