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

Fallback from batch provider to singular provider #144

Merged
merged 3 commits into from
Aug 21, 2023

Conversation

guplersaxanoid
Copy link

Description

With the ETH sdk we use the batch http provider by default. Some RPC endpoints don't support batch requests. We should instead fallback to using the singular provider if this happens

Fixes subquery#1941

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have tested locally
  • I have performed a self review of my changes
  • Updated any relevant documentation
  • Linked to any relevant issues
  • I have added tests relevant to my changes
  • Any dependent changes have been merged and published in downstream modules
  • My code is up to date with the base branch
  • I have updated relevant changelogs. We suggest using chan

@guplersaxanoid guplersaxanoid changed the title handle batch ETH client not supported Fallback from batch provider to singular provider Aug 17, 2023
@github-actions
Copy link

github-actions bot commented Aug 17, 2023

Coverage report

St.
Category Percentage Covered / Total
🔴 Statements 45.2% 2903/6422
🔴 Branches 59.32% 226/381
🔴 Functions 36.56% 83/227
🔴 Lines 45.2% 2903/6422

Test suite run success

31 tests passing in 5 suites.

Report generated by 🧪jest coverage report action from 84ea4dd

this.name = network.name;
} catch (e) {
//batch request error for filecoin
if (e.message.includes('"message":"we can\'t execute this request"')) {

Choose a reason for hiding this comment

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

This would only work for filecoin. Other RPC providers might do other things.

I think we should change this error handling. Maybe we can go off the http code or if its not a json rpc response

@@ -144,8 +144,7 @@ export class EthereumApi implements ApiWrapper<EthereumBlockWrapper> {
this.chainId = network.chainId;
this.name = network.name;
} catch (e) {
//batch request error for filecoin
if (e.message.includes('"message":"we can\'t execute this request"')) {
if ((e as Error).message.startsWith('Invalid response')) {

Choose a reason for hiding this comment

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

Does this check against an Ethers error?

Copy link
Author

Choose a reason for hiding this comment

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

'Invalid response \n' + JSON.stringify(result),

This is how JsonRpcBatchProvider class intercepts invalid responses

@stwiname stwiname merged commit 925650d into main Aug 21, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fallback from batch provider to singular provider
2 participants