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.

Sign up for GitHub

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

Jump to bottom

Skip error logs for FGW responses with NOT_RECEIVED status #2303

Merged
merged 3 commits into from
Dec 10, 2024
Merged

Skip error logs for FGW responses with NOT_RECEIVED status #2303

merged 3 commits into from
Dec 10, 2024

Conversation

wojciechos
Copy link
Contributor

@wojciechos wojciechos commented Dec 4, 2024 •
edited
Loading

Do not log an error when FGW returns NOT_RECEIVED as this is a common scenario and should not generate misleading or confusing log entries for users.

@codecov Codecov
Copy link

codecov bot commented Dec 4, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 75.36%. Comparing base (926b485) to head (cd9a8b1).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
rpc/transaction.go 40.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2303      +/-   ##
==========================================
+ Coverage   75.25%   75.36%   +0.11%     
==========================================
  Files         108      108              
  Lines       11541    11544       +3     
==========================================
+ Hits         8685     8700      +15     
+ Misses       2188     2181       -7     
+ Partials      668      663       -5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rianhughes rianhughes self-requested a review December 5, 2024 09:02
@wojciechos wojciechos changed the title Add NotReceived case handling in adaptTransactionStatus Avoid logging errors for NOT_RECEIVED responses from FGW to reduce noise and confusion in logs Dec 5, 2024
@wojciechos wojciechos changed the title Avoid logging errors for NOT_RECEIVED responses from FGW to reduce noise and confusion in logs Skip error logs for FGW responses with NOT_RECEIVED status Dec 5, 2024
kirugan
kirugan reviewed Dec 5, 2024
View reviewed changes
rpc/transaction.go Outdated Show resolved Hide resolved
@wojciechos wojciechos force-pushed the wojciechos/handle_not_received branch from fdcbe48 to b5a688c Compare December 5, 2024 13:45
@wojciechos wojciechos requested a review from kirugan December 5, 2024 16:52
wojciechos and others added 2 commits December 9, 2024 17:23
@wojciechos
Add NotReceived case handling in adaptTransactionStatus
1c4a3e6 
The NotReceived case in adaptTransactionStatus should return nil, nil instead
of falling through to default case. This allows the error to naturally propagate
from the original ErrTxnHashNotFound in TransactionStatus without logging
unnecessary errors for the common case of transactions not being found.
@rianhughes @wojciechos
dont log error for not_received responses from the fgw (#2304)
30add4e 
* dont log error for not_received responses from the fgw

* lint
@wojciechos wojciechos force-pushed the wojciechos/handle_not_received branch 3 times, most recently from 90aa3c6 to c878b1e Compare December 10, 2024 10:00
@wojciechos
Improve transaction status error handling
cd9a8b1 
Introduce a dedicated error variable for transaction not found cases in the
adapter layer. This separates internal error handling from API errors and
follows Go best practices by using errors.Is() for comparisons.

The change:
- Adds errTransactionNotFound for internal error handling
- Replaces string-based error comparison with errors.Is()
- Maintains existing API behavior while improving code maintainability
@wojciechos wojciechos force-pushed the wojciechos/handle_not_received branch from c878b1e to cd9a8b1 Compare December 10, 2024 19:03
@wojciechos wojciechos enabled auto-merge (squash) December 10, 2024 19:05
@wojciechos wojciechos disabled auto-merge December 10, 2024 20:51
@wojciechos wojciechos merged commit 3a7abeb into main Dec 10, 2024
12 of 13 checks passed
@wojciechos wojciechos deleted the wojciechos/handle_not_received branch December 10, 2024 20:52
weiihann added a commit that referenced this pull request Dec 12, 2024
@weiihann
Squashed commit of the following:
f1d1ff2 
commit 7687360
Author: Ng Wei Han <[email protected]>
Date:   Thu Dec 12 18:54:32 2024 +0800

    Remove size in OrderedSet (#2319)

commit 65b7507
Author: Ng Wei Han <[email protected]>
Date:   Thu Dec 12 18:20:55 2024 +0800

    Fix and refactor trie proof logics (#2252)

commit 2b1b219
Author: aleven1999 <[email protected]>
Date:   Thu Dec 12 12:11:28 2024 +0400

    Remove unused code (#2318)

commit 0a21162
Author: Daniil Ankushin <[email protected]>
Date:   Thu Dec 12 00:04:08 2024 +0700

    Remove unused code (#2317)

commit 8bf9be9
Author: Rian Hughes <[email protected]>
Date:   Wed Dec 11 14:11:22 2024 +0200

    update invoke v3 txn validation to require sender_address (#2308)

commit 91d0f8e
Author: Kirill <[email protected]>
Date:   Wed Dec 11 16:01:10 2024 +0400

    Add schema_version to output of db info command (#2309)

commit 60e8cc9
Author: AnavarKh <[email protected]>
Date:   Wed Dec 11 16:04:31 2024 +0530

    Update download link for Juno snapshots from dev to io in Readme file (#2314)

commit 8862de1
Author: wojciechos <[email protected]>
Date:   Wed Dec 11 11:20:02 2024 +0100

    Improve binary build workflow for cross-platform releases (#2315)

    - Add proper architecture handling in matrix configuration
    - Implement caching for Go modules and Rust dependencies
    - Streamline dependency installation for both Linux and macOS
    - Improve binary artifact handling and checksums
    - Add retention policy for build artifacts
    - Split build steps for better clarity and maintainability

    This update ensures more reliable and efficient binary builds
    across all supported platforms.

commit e75e504
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Wed Dec 11 07:35:16 2024 +0000

    Bump nanoid from 3.3.7 to 3.3.8 in /docs in the npm_and_yarn group across 1 directory (#2316)

    Bump nanoid in /docs in the npm_and_yarn group across 1 directory

    Bumps the npm_and_yarn group with 1 update in the /docs directory: [nanoid](https://github.com/ai/nanoid).

    Updates `nanoid` from 3.3.7 to 3.3.8
    - [Release notes](https://github.com/ai/nanoid/releases)
    - [Changelog](https://github.com/ai/nanoid/blob/main/CHANGELOG.md)
    - [Commits](ai/nanoid@3.3.7...3.3.8)

    ---
    updated-dependencies:
    - dependency-name: nanoid
      dependency-type: indirect
      dependency-group: npm_and_yarn
    ...

    Signed-off-by: dependabot[bot] <[email protected]>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 3a7abeb
Author: wojciechos <[email protected]>
Date:   Tue Dec 10 21:52:49 2024 +0100

    Skip error logs for FGW responses with NOT_RECEIVED status (#2303)

    * Add NotReceived case handling in adaptTransactionStatus

    ---------

    Co-authored-by: Rian Hughes <[email protected]>
weiihann added a commit that referenced this pull request Dec 13, 2024
@weiihann
Squashed commit of the following:
105255a 
commit 7687360
Author: Ng Wei Han <[email protected]>
Date:   Thu Dec 12 18:54:32 2024 +0800

    Remove size in OrderedSet (#2319)

commit 65b7507
Author: Ng Wei Han <[email protected]>
Date:   Thu Dec 12 18:20:55 2024 +0800

    Fix and refactor trie proof logics (#2252)

commit 2b1b219
Author: aleven1999 <[email protected]>
Date:   Thu Dec 12 12:11:28 2024 +0400

    Remove unused code (#2318)

commit 0a21162
Author: Daniil Ankushin <[email protected]>
Date:   Thu Dec 12 00:04:08 2024 +0700

    Remove unused code (#2317)

commit 8bf9be9
Author: Rian Hughes <[email protected]>
Date:   Wed Dec 11 14:11:22 2024 +0200

    update invoke v3 txn validation to require sender_address (#2308)

commit 91d0f8e
Author: Kirill <[email protected]>
Date:   Wed Dec 11 16:01:10 2024 +0400

    Add schema_version to output of db info command (#2309)

commit 60e8cc9
Author: AnavarKh <[email protected]>
Date:   Wed Dec 11 16:04:31 2024 +0530

    Update download link for Juno snapshots from dev to io in Readme file (#2314)

commit 8862de1
Author: wojciechos <[email protected]>
Date:   Wed Dec 11 11:20:02 2024 +0100

    Improve binary build workflow for cross-platform releases (#2315)

    - Add proper architecture handling in matrix configuration
    - Implement caching for Go modules and Rust dependencies
    - Streamline dependency installation for both Linux and macOS
    - Improve binary artifact handling and checksums
    - Add retention policy for build artifacts
    - Split build steps for better clarity and maintainability

    This update ensures more reliable and efficient binary builds
    across all supported platforms.

commit e75e504
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Wed Dec 11 07:35:16 2024 +0000

    Bump nanoid from 3.3.7 to 3.3.8 in /docs in the npm_and_yarn group across 1 directory (#2316)

    Bump nanoid in /docs in the npm_and_yarn group across 1 directory

    Bumps the npm_and_yarn group with 1 update in the /docs directory: [nanoid](https://github.com/ai/nanoid).

    Updates `nanoid` from 3.3.7 to 3.3.8
    - [Release notes](https://github.com/ai/nanoid/releases)
    - [Changelog](https://github.com/ai/nanoid/blob/main/CHANGELOG.md)
    - [Commits](ai/nanoid@3.3.7...3.3.8)

    ---
    updated-dependencies:
    - dependency-name: nanoid
      dependency-type: indirect
      dependency-group: npm_and_yarn
    ...

    Signed-off-by: dependabot[bot] <[email protected]>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit 3a7abeb
Author: wojciechos <[email protected]>
Date:   Tue Dec 10 21:52:49 2024 +0100

    Skip error logs for FGW responses with NOT_RECEIVED status (#2303)

    * Add NotReceived case handling in adaptTransactionStatus

    ---------

    Co-authored-by: Rian Hughes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rianhughes rianhughes rianhughes approved these changes

@derrix060 derrix060 derrix060 approved these changes

@kirugan kirugan Awaiting requested review from kirugan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wojciechos @kirugan @derrix060 @rianhughes