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

Handle GRPC timeout error #883

Merged
merged 12 commits into from
Feb 14, 2023
Merged

Handle GRPC timeout error #883

merged 12 commits into from
Feb 14, 2023

Conversation

Ivo-Yankov
Copy link
Collaborator

@Ivo-Yankov Ivo-Yankov commented Feb 7, 2023

Description:
This PR aims to resolve several inconsistent errors that occur at times of increased load.

  • Adds a new env var for configuring sdkClient._maxExecutionTime.
  • Fixes a rare bug in eth_getTransansactionByHash and eth_getTransactionReceipt.
  • Improves error handling.

Related issue(s):

Fixes #874

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
@codecov-commenter
Copy link

codecov-commenter commented Feb 7, 2023

Codecov Report

Base: 74.16% // Head: 75.09% // Increases project coverage by +0.92% 🎉

Coverage data is based on head (1c3aa1a) compared to base (1f9b860).
Patch coverage: 68.62% of modified lines in pull request are covered.

❗ Current head 1c3aa1a differs from pull request most recent head 8d4d502. Consider uploading reports for the commit 8d4d502 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #883      +/-   ##
==========================================
+ Coverage   74.16%   75.09%   +0.92%     
==========================================
  Files          29       16      -13     
  Lines        1827     1349     -478     
  Branches      339      254      -85     
==========================================
- Hits         1355     1013     -342     
+ Misses        374      269     -105     
+ Partials       98       67      -31     
Impacted Files Coverage Δ
packages/relay/src/lib/errors/SDKClientError.ts 62.50% <0.00%> (-4.17%) ⬇️
packages/relay/src/lib/clients/sdkClient.ts 20.70% <22.22%> (+0.08%) ⬆️
packages/relay/src/lib/eth.ts 84.74% <71.42%> (+1.90%) ⬆️
packages/relay/src/lib/clients/mirrorNodeClient.ts 94.42% <100.00%> (+0.09%) ⬆️
packages/relay/src/lib/errors/JsonRpcError.ts 72.72% <100.00%> (+2.72%) ⬆️
packages/relay/src/lib/hbarlimiter/index.ts 100.00% <100.00%> (ø)
packages/server/src/rateLimit/index.ts
packages/server/src/server.ts
packages/server/src/validator/constants.ts
...kages/server/src/koaJsonRpc/lib/RpcInvalidError.ts
... and 9 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
@Ivo-Yankov Ivo-Yankov marked this pull request as ready for review February 8, 2023 12:33
@Ivo-Yankov Ivo-Yankov self-assigned this Feb 8, 2023
@Ivo-Yankov Ivo-Yankov linked an issue Feb 8, 2023 that may be closed by this pull request
@Ivo-Yankov Ivo-Yankov added enhancement New feature or request P1 dev tools Features enabling dev tool integration limechain labels Feb 8, 2023
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

LG, just some clean up and comment suggestions

helm-chart/templates/configmap.yaml Outdated Show resolved Hide resolved
packages/relay/src/lib/clients/sdkClient.ts Show resolved Hide resolved
packages/server/tests/localAcceptance.env Outdated Show resolved Hide resolved
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Nana-EC
Nana-EC previously approved these changes Feb 14, 2023
Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

LG

TIER_1_RATE_LIMIT = 100
TIER_2_RATE_LIMIT = 800
TIER_3_RATE_LIMIT = 1600
DEFAULT_RATE_LIMIT = 400
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did we double everything here?
Maybe update the PR comments

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
@sonarcloud
Copy link

sonarcloud bot commented Feb 14, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

LG

@Nana-EC Nana-EC added this to the 0.18.0 milestone Feb 14, 2023
@Ivo-Yankov Ivo-Yankov merged commit 41debf8 into main Feb 14, 2023
@Ivo-Yankov Ivo-Yankov deleted the 874-handle-grpc-timeout-error branch February 14, 2023 14:18
Nana-EC pushed a commit that referenced this pull request Feb 15, 2023
* feat: add grpc timeout config and error handling

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* chore: add new predefined errors

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* chore: add env var to helmchart configs

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* chore: increase rate limits

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* chore: increase rate limits

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* fix: add workaround for rare bug

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* chore: revert rate limit increase

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* nit: address comments

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* nit: address comment from a previous PR

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* nit: typo

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* chore: revert rate limits for acceptance tests

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

---------

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Nana-EC pushed a commit that referenced this pull request Feb 15, 2023
* feat: add grpc timeout config and error handling

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* chore: add new predefined errors

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* chore: add env var to helmchart configs

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* chore: increase rate limits

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* chore: increase rate limits

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* fix: add workaround for rare bug

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* chore: revert rate limit increase

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* nit: address comments

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* nit: address comment from a previous PR

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* nit: typo

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* chore: revert rate limits for acceptance tests

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

---------

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: Nana Essilfie-Conduah <nana@swirldslabs.com>
Nana-EC added a commit that referenced this pull request Feb 15, 2023
Cherry pick PRs that went into main branch since cutting a 0.18.0-alpha1. PRs map to individual commits
- [PR 882 - eth_feeHistory invalid cached response](#882)
- [PR 887 - Needed changes for metrics improvements](#887)
- [PR 883 - Handle GRPC timeout error](#883)
- PR 898 - update-logs-and-metrics Added the requestId to the server context](#898)

---------

Signed-off-by: ebadiere <ebadiere@gmail.com>
Signed-off-by: Nana Essilfie-Conduah <nana@swirldslabs.com>

---------

Signed-off-by: nikolay <n.atanasow94@gmail.com>
Signed-off-by: Nana Essilfie-Conduah <nana@swirldslabs.com>
Signed-off-by: Alfredo Gutierrez <alfredo@swirldslabs.com>
Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Signed-off-by: ebadiere <ebadiere@gmail.com>
Co-authored-by: Nikolay Atanasow <n.atanasow94@gmail.com>
Co-authored-by: Alfredo Gutierrez <alfredo@swirldslabs.com>
Co-authored-by: Ivo Yankov <ivo@devlabs.bg>
Co-authored-by: Eric Badiere <ebadiere@gmail.com>
Nana-EC pushed a commit that referenced this pull request Feb 15, 2023
* feat: add grpc timeout config and error handling

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* chore: add new predefined errors

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* chore: add env var to helmchart configs

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* chore: increase rate limits

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* chore: increase rate limits

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* fix: add workaround for rare bug

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* chore: revert rate limit increase

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* nit: address comments

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* nit: address comment from a previous PR

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* nit: typo

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

* chore: revert rate limits for acceptance tests

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>

---------

Signed-off-by: Ivo Yankov <ivo@devlabs.bg>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev tools Features enabling dev tool integration enhancement New feature or request limechain P1
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

intermittent unreliable contract deploys
3 participants