Skip to content

fix: keep best-effort candidate-roster export failures from escaping the handle path - #26901

Open
ruslanvelkov-beep wants to merge 4 commits into
hiero-ledger:mainfrom
ruslanvelkov-beep:fix/guard-candidate-roster-export
Open

ruslanvelkov-beep wants to merge 4 commits into
hiero-ledger:mainfrom
ruslanvelkov-beep:fix/guard-candidate-roster-export

Conversation

@ruslanvelkov-beep

@ruslanvelkov-beep ruslanvelkov-beep commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Description

At PREPARE_UPGRADE, PlatformStateUpdates.doExport(...) resolves the export path and calls the roster export helper with no exception handling — unlike the putCandidateRoster call just above it, which is wrapped. The export is a best-effort operator convenience and is not consensus state, but an unchecked exception from it escapes handleTxBody; the enclosing dispatch then catches it and rolls back the stack, discarding the candidate roster already applied on that node while peers whose export succeeded keep it — a node-local, configuration/environment-dependent divergence.

Escaping cases include a malformed configured path (InvalidPathException), a path resolving to a filesystem root (NullPointerException), and an I/O failure during the write surfacing as UncheckedIOException (e.g. a full disk). DiskStartupNetworks.tryToExport only catches IOException, so these unchecked types are not absorbed there.

This wraps doExport in a try/catch that logs and swallows any failure, so a best-effort export can never affect committed state — matching the existing treatment of putCandidateRoster.

Notes for reviewer

  • No behavior change on the success path; only failures that previously propagated are now logged and swallowed.
  • catch(Exception) covers every reachable unchecked type; an IOError (Error) from toAbsolutePath is intentionally not caught.

Tests

  • PlatformStateUpdatesTest (16, all green) adds cases for: export-helper failure, malformed path, root path, disk-full-during-write, and a forced createTempFile IllegalArgumentException — each asserting the failure is swallowed and the candidate roster stays committed.

Checklist

  • Documented (code comments)
  • Tested (unit)

…the handle path

Signed-off-by: Ruslan Velkov <ruslan.velkov@limechain.tech>
@ruslanvelkov-beep
ruslanvelkov-beep requested a review from a team as a code owner August 19, 2026 09:43
@ruslanvelkov-beep ruslanvelkov-beep self-assigned this Aug 19, 2026
@lfdt-bot

lfdt-bot commented Aug 19, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Impacted file tree graph

@@             Coverage Diff              @@
##               main   #26901      +/-   ##
============================================
+ Coverage     71.25%   71.27%   +0.02%     
- Complexity    11906    11907       +1     
============================================
  Files          2596     2596              
  Lines        109552   109555       +3     
  Branches      12364    12365       +1     
============================================
+ Hits          78061    78087      +26     
+ Misses        27447    27433      -14     
+ Partials       4044     4035       -9     
Files with missing lines Coverage Δ Complexity Δ
...p/workflows/handle/steps/PlatformStateUpdates.java 96.92% <100.00%> (+0.14%) 0.00 <0.00> (ø)

... and 8 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@testlens-app

testlens-app Bot commented Sep 18, 2026

Copy link
Copy Markdown

🚨 TestLens detected 1 failed test 🚨

Here is what you can do:

  1. Inspect the test failures carefully.
  2. If you are convinced that some of the tests are flaky, you can mute them below.
  3. Finally, trigger a rerun by checking the rerun checkbox.

Failed Jobs without Test Failures

600: [FLOW] PR Checks / CI Complete
600: [FLOW] PR Checks / CI Complete
600: [FLOW] PR Checks / MATS / Snyk Scan / Snyk Checks
600: [FLOW] PR Checks / MATS / Snyk Scan / Snyk Checks
701: [FLOW] Auto Unapprove PR / Auto Unapprove PR
701: [FLOW] Auto Unapprove PR / Auto Unapprove PR

Test Summary

600: [FLOW] PR Checks / MATS / HAPI Tests / hapi-tests-misc / HAPI Tests (Misc) > :test-clients:testSubprocessConcurrent

Test Runs Flakiness
LedgerIdPublicationTimestampTest > ledgerIdPublicationIsNotBackdatedWithinItsBlock() > as written 1% 🟡

🏷️ Commit: 443a13c
▶️ Tests: 123876 executed
🟡 Checks: 98/99 completed

Test Failures

LedgerIdPublicationTimestampTest > ledgerIdPublicationIsNotBackdatedWithinItsBlock() > as written (:test-clients:testSubprocessConcurrent in 600: [FLOW] PR Checks / MATS / HAPI Tests / hapi-tests-misc / HAPI Tests (Misc))
org.opentest4j.AssertionFailedError: LedgerIdPublicationOrder ended with result: Timed out in PT5M
	at com.hedera.node.test.clients@0.79.0-SNAPSHOT/com.hedera.services.bdd.spec.utilops.streams.assertions.AbstractEventualStreamAssertion.assertHasPassed(AbstractEventualStreamAssertion.java:69)
	at com.hedera.node.test.clients@0.79.0-SNAPSHOT/com.hedera.services.bdd.spec.utilops.streams.assertions.EventualStreamAssertion.assertHasPassed(EventualStreamAssertion.java:80)
	at com.hedera.node.test.clients@0.79.0-SNAPSHOT/com.hedera.services.bdd.spec.HapiSpec.checkStream(HapiSpec.java:1095)
	at com.hedera.node.test.clients@0.79.0-SNAPSHOT/com.hedera.services.bdd.spec.HapiSpec.exec(HapiSpec.java:1037)
	at com.hedera.node.test.clients@0.79.0-SNAPSHOT/com.hedera.services.bdd.spec.HapiSpec.run(HapiSpec.java:804)
	at com.hedera.node.test.clients@0.79.0-SNAPSHOT/com.hedera.services.bdd.spec.HapiSpec.execute(HapiSpec.java:754)
	at java.base/java.util.concurrent.RecursiveAction.exec(RecursiveAction.java:194)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:511)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.tryRemoveAndExec(ForkJoinPool.java:1490)
	at java.base/java.util.concurrent.ForkJoinPool.helpJoin(ForkJoinPool.java:2248)
	at java.base/java.util.concurrent.ForkJoinTask.awaitDone(ForkJoinTask.java:499)
	at java.base/java.util.concurrent.ForkJoinTask.get(ForkJoinTask.java:1015)
	at java.base/java.util.concurrent.RecursiveAction.exec(RecursiveAction.java:194)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:511)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1450)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:2019)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:187)

Rerun Controls

Note

Checks are currently running using the configuration below.

Select tests to mute in this pull request:

🔲 LedgerIdPublicationTimestampTest > ledgerIdPublicationIsNotBackdatedWithinItsBlock()

Reuse successful test results:

🔲 ♻️ Only rerun the tests that failed or were muted before

Click the checkbox to trigger a rerun:

🔲 Rerun jobs


Learn more about TestLens at testlens.app/docs.

@ibankov ibankov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good

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.

3 participants