Skip to content

2046_fabricx_public_params_setup_returns_errors - #2246

Draft
HayimShaul wants to merge 1 commit into
mainfrom
2046_fabricx_public_params_setup_returns_errors
Draft

2046_fabricx_public_params_setup_returns_errors#2246
HayimShaul wants to merge 1 commit into
mainfrom
2046_fabricx_public_params_setup_returns_errors

Conversation

@HayimShaul

Copy link
Copy Markdown
Contributor

Fixes #2046

fabricx.Backend.InstallPublicParams and UpdatePublicParams panicked instead of
reporting failures: UpdatePublicParams dereferenced a nil issuer client, both panicked on
a SetupPublicParams view error, and the install variant did so from a background
goroutine, which took the whole test process down with panic: failed updating pps.

Both now return an error, and so does the fabric.Backend interface they implement:

  • the background installation is crash-safe (recover()) and records its outcome per TMS;
    WaitForPublicParams blocks on it, PendingInstallError polls it;
  • UpdatePublicParams waits for the issuer client the way the install path does, so a
    not-yet-started issuer node is a wait rather than a nil-interface panic. The retry budget
    is configurable (ClientRetries, ClientRetryDelay, InstallDelay), defaulting to
    today's 60 attempts / 1s / 10s;
  • fabric.NetworkHandler converts the errors into gomega failures (PostRun,
    UpdatePublicParams) and logs a still-pending install failure in Cleanup, so the NWO
    platform API is unchanged;
  • cc.GenericBackend follows the new signatures and returns its chaincode lookup,
    mkdir and packaging failures as errors;
  • the view payload is marshalled with encoding/json instead of common.JSONMarshall,
    whose failure mode is a gomega assertion — another hidden panic on this very path.

Behaviour change worth calling out: an installation that failed in the background used to
crash the run, or pass unnoticed. It now fails the spec that updates the public parameters,
carrying the original error, or is logged at teardown.

Tests: integration/nwo/token/fabricx/factory_test.go covers both paths, a recovered
panic, the wait timeout and the exact payload sent to the view.
TestInstallPublicParamsBackgroundFailureDoesNotCrashProcess re-executes the failing
installation in a child process — the only way to assert that the goroutine no longer kills
the runner — and it fails if the panic is reintroduced.

make checks is green. make lint reports only findings already present on main
(verified by linting a clean origin/main worktree); this change adds none.

Docs: docs/development/debug-integration-tests.md gains a section on where a public
parameters setup failure now surfaces.

@HayimShaul HayimShaul added this to the Q3/26 milestone Aug 17, 2026
@HayimShaul HayimShaul self-assigned this Aug 17, 2026
The fabricx token backend panicked when the SetupPublicParams view failed,
and it did so on a background goroutine, so the whole test process died
with `panic: failed updating pps` instead of the suite reporting a failure.
It also panicked when the issuer client never became ready, and inside
common.JSONMarshall, whose failure mode is a gomega assertion.

InstallPublicParams and UpdatePublicParams now return an error, and so does
the fabric.Backend interface they implement. The background installation
records its outcome, including a recovered panic, and NetworkHandler
surfaces it: UpdatePublicParams fails fast with the original error, Cleanup
logs it at teardown, and WaitForPublicParams lets a test block on it.
Errors become gomega assertions at the NetworkHandler boundary, so the NWO
platform API is unchanged.

Adds unit tests for both paths, including a regression test that runs the
failing installation in a child process to prove it no longer crashes, and
documents where the failure surfaces in the integration-test debugging doc.

Signed-off-by: Hayim.Shaul@ibm.com <hayimsha@fhe03.vpc.cloud9.ibm.com>
@HayimShaul
HayimShaul force-pushed the 2046_fabricx_public_params_setup_returns_errors branch from ec518cc to ee6fa0c Compare August 18, 2026 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants