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

feat: expose node account id in relay logging of failed transactions #3292

Merged

Conversation

natanasow
Copy link
Collaborator

Description:

The Relay server currently lacks visibility into which node account ID a transaction was submitted to when it fails due to an SDK error. With the recent update to the hedera-sdk-js, the error object now includes the node account ID for failed transactions.

This new capability in the Relay's logging system is expected to improve troubleshooting and provide better insights into node-specific transaction failures.

Solution:

Update the Relay server's logging system to extract and log the node account ID from the error object provided by the SDK

Related issue(s):

Fixes #3278

Notes for reviewer:

Checklist

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

Signed-off-by: nikolay <[email protected]>
@natanasow natanasow self-assigned this Nov 22, 2024
@natanasow natanasow added the enhancement New feature or request label Nov 22, 2024
@natanasow natanasow added this to the 0.61.0 milestone Nov 22, 2024
Copy link

github-actions bot commented Nov 22, 2024

Test Results

 18 files   -   4  235 suites   - 60   29m 54s ⏱️ - 6m 22s
608 tests  -   1  604 ✅ + 21  4 💤 ±0  0 ❌  - 22 
624 runs   - 226  620 ✅  - 203  4 💤 ±0  0 ❌  - 23 

Results for commit d669b9b. ± Comparison against base commit b55ca6b.

This pull request removes 1 test.
"before all" hook for "@release should deploy a contract" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-1 RPC Server Acceptance Tests RPC Server Acceptance Tests eth_getLogs "before all" hook for "@release should deploy a contract"

♻️ This comment has been updated with latest results.

@natanasow natanasow marked this pull request as ready for review November 22, 2024 14:18
Copy link

sonarcloud bot commented Nov 22, 2024

Copy link
Member

@quiet-node quiet-node left a comment

Choose a reason for hiding this comment

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

LGTM

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.
Please open a ticket to move logicinto SDKError & unpdate existing error logs later

packages/relay/src/lib/clients/sdkClient.ts Show resolved Hide resolved
@Nana-EC Nana-EC merged commit dc860b3 into main Nov 23, 2024
44 of 46 checks passed
@Nana-EC Nana-EC deleted the 3278-expose-node-account-id-in-relay-logging-of-failed-txs branch November 23, 2024 03:35
Copy link

codecov bot commented Nov 23, 2024

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 77.82%. Comparing base (b55ca6b) to head (d669b9b).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
packages/relay/src/lib/clients/sdkClient.ts 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3292      +/-   ##
==========================================
- Coverage   78.88%   77.82%   -1.06%     
==========================================
  Files          48       66      +18     
  Lines        3608     4474     +866     
  Branches      841     1005     +164     
==========================================
+ Hits         2846     3482     +636     
- Misses        423      617     +194     
- Partials      339      375      +36     
Flag Coverage Δ
config-service 98.14% <ø> (ø)
relay 78.55% <0.00%> (-0.04%) ⬇️
server 83.28% <ø> (?)
ws-server 36.66% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/relay/src/lib/clients/sdkClient.ts 55.08% <0.00%> (-0.95%) ⬇️

... and 19 files with indirect coverage changes

---- 🚨 Try these New Features:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose Node Account ID in Relay Logging for Failed Transactions
3 participants