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

06788 Better handling of failed async hashing #6796

Merged
merged 3 commits into from May 26, 2023

Conversation

imalygin
Copy link
Member

Description:

Currently the exception is handled this way in MerkleUtils#rehashTree:

  final Future<Hash> future = MerkleCryptoFactory.getInstance().digestTreeAsync(root);
            try {
                return future.get();
            } catch (InterruptedException | ExecutionException e) {
                Thread.currentThread().interrupt();
            }

So, if any exception happens during the hashing, the stacktrace is lost.

This exception must not be swallowed. It should do two things:

Log the exception it caught
Rethrow the ExecutionException wrapped in some unchecked exception

Related issue(s):

Fixes #6788

Checklist

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

@imalygin imalygin added the Tech Debt Reduced Issues which reduce technical debt. label May 25, 2023
@imalygin imalygin added this to the v0.39 milestone May 25, 2023
@imalygin imalygin requested a review from a team as a code owner May 25, 2023 20:28
@imalygin imalygin self-assigned this May 25, 2023
@imalygin imalygin requested review from a team as code owners May 25, 2023 20:28
artemananiev
artemananiev previously approved these changes May 25, 2023
@imalygin imalygin force-pushed the 06788-better-exception-handling branch 2 times, most recently from 8381557 to 5576de7 Compare May 25, 2023 20:36
Signed-off-by: Ivan Malygin <ivan@swirldslabs.com>
OlegMazurov
OlegMazurov previously approved these changes May 25, 2023
@github-actions
Copy link

github-actions bot commented May 25, 2023

Node: Unit Test Results

    1 411 files      1 411 suites   16m 27s ⏱️
100 315 tests 100 308 ✔️ 7 💤 0
106 653 runs  106 646 ✔️ 7 💤 0

Results for commit 4ba083b.

♻️ This comment has been updated with latest results.

artemananiev
artemananiev previously approved these changes May 25, 2023
@codecov
Copy link

codecov bot commented May 25, 2023

Codecov Report

Patch coverage has no change and project coverage change: +21.83 🎉

Comparison is base (49fb8fc) 67.90% compared to head (4ba083b) 89.74%.

Additional details and impacted files
@@              Coverage Diff               @@
##             develop    #6796       +/-   ##
==============================================
+ Coverage      67.90%   89.74%   +21.83%     
+ Complexity     22849    17788     -5061     
==============================================
  Files           2126     1417      -709     
  Lines         138924    52074    -86850     
  Branches        7970     5283     -2687     
==============================================
- Hits           94337    46734    -47603     
+ Misses         43015     4311    -38704     
+ Partials        1572     1029      -543     

see 944 files with indirect coverage changes

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

@github-actions
Copy link

github-actions bot commented May 25, 2023

Node: E2E Test Results

    1 files      1 suites   19m 19s ⏱️
310 tests 310 ✔️ 0 💤 0
328 runs  328 ✔️ 0 💤 0

Results for commit 4ba083b.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented May 25, 2023

Node: Integration Test Results

    4 files      4 suites   13m 31s ⏱️
170 tests 170 ✔️ 0 💤 0
172 runs  172 ✔️ 0 💤 0

Results for commit 4ba083b.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented May 25, 2023

Platform: JUnit Test Report

     524 files           1 errors  523 suites   23m 9s ⏱️
13 827 tests 13 791 ✔️ 36 💤 0
15 812 runs  15 776 ✔️ 36 💤 0

Results for commit 4ba083b.

♻️ This comment has been updated with latest results.

Signed-off-by: Ivan Malygin <ivan@swirldslabs.com>
@imalygin imalygin dismissed stale reviews from artemananiev and OlegMazurov via 4ba083b May 26, 2023 00:49
@sonarcloud
Copy link

sonarcloud bot commented May 26, 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
No Duplication information No Duplication information

@sonarcloud
Copy link

sonarcloud bot commented May 26, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

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

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@imalygin imalygin merged commit 7c29b0e into develop May 26, 2023
23 of 25 checks passed
@imalygin imalygin deleted the 06788-better-exception-handling branch May 26, 2023 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tech Debt Reduced Issues which reduce technical debt.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better exception handling in MerkleUtils#rehashTree
4 participants