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

Add basic failover functionality #9

Closed
wants to merge 4 commits into from
Closed

Conversation

gtsonevv
Copy link
Collaborator

@gtsonevv gtsonevv commented Dec 7, 2023

Pre-flight checklist

  • I have read the Contributing Guidelines on pull requests.
  • Commit messages follow the conventional commits spec
  • If this is a code change: I have written unit tests.
  • If this changes code in a published package: I have run pnpm changeset to create a changeset JSON document appropriate for this change.
  • If this is a new API or substantial change: the PR has an accompanying issue (closes #733) and the maintainers have approved on my working plan.

Motivation

Added support for multiple API Keys (since we can have multiple RPC Servers)
Added basic failover functionality
With this design of failover, we will change (rotate) our server when needed and stick to this new server for our next calls. It will help to avoid long responses when first (or more) servers are down.

For now, server rotation is happening only on Timeout error.

Also, check this comment: near#733 (comment)

Test Plan

Added unit tests.

Related issues/PRs

near#733
near#760

packages/cookbook/api-keys/near-connection.js Show resolved Hide resolved
packages/providers/src/fetch_json.ts Show resolved Hide resolved
@@ -333,6 +333,19 @@ export class JsonRpcProvider extends Provider {
return await this.sendJsonRpc('gas_price', [blockId]);
}

/**
* Part of the RPC failover design.
* Changing current (first) rpc server and moves it to the and of the queue.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* Changing current (first) rpc server and moves it to the and of the queue.
* Changing current (first) rpc server and moves it to the end of the queue.

packages/providers/src/json-rpc-provider.ts Show resolved Hide resolved
Comment on lines +191 to +205
test('JsonRpc rotateRpcServers', async () => {
const SERVER_1 = 'server1';
const SERVER_2 = 'server2';
const SERVER_3 = 'server3';
const provider = new JsonRpcProvider({ url: [SERVER_1, SERVER_2, SERVER_3] });
expect(provider.connection.url.length).toEqual(3);
expect(provider.connection.url[0]).toMatch(SERVER_1);
expect(provider.connection.url[1]).toMatch(SERVER_2);
expect(provider.connection.url[2]).toMatch(SERVER_3);
provider.rotateRpcServers();
expect(provider.connection.url.length).toEqual(3);
expect(provider.connection.url[0]).toMatch(SERVER_2);
expect(provider.connection.url[1]).toMatch(SERVER_3);
expect(provider.connection.url[2]).toMatch(SERVER_1);
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add a bad case test which contains bad configuration. The error should be caught at construction time I believe.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will add

/**
* RPC API Keys. Used to authenticate users on RPC Server.
*/
apiKeys: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be { [url: string]: string };

@vikinatora vikinatora closed this Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants