Skip to content

node: add safe constructors#1328

Open
malsomesh9 wants to merge 1 commit into
hyperledger-labs:mainfrom
malsomesh9:somesh/issue-183-node-panic-cleanup
Open

node: add safe constructors#1328
malsomesh9 wants to merge 1 commit into
hyperledger-labs:mainfrom
malsomesh9:somesh/issue-183-node-panic-cleanup

Conversation

@malsomesh9
Copy link
Copy Markdown
Contributor

Summary

  • add explicit error-returning node constructors (NewFromConfPathE, NewE, NewWithConfPathE)
  • keep the legacy panicing constructors as compatibility wrappers
  • adopt the safe constructor path in benchmark setup so bad config returns an error instead of crashing the process

Why

#183 calls out panic-heavy API paths. The node construction flow was one of the clearest user-facing cases: bad config could trigger a panic during setup. This PR introduces safe alternatives without breaking existing callers and starts moving internal code onto the error-returning path.

Testing

  • go test ./pkg/node
  • go test ./node
  • go test ./integration/benchmark/node

Refs #183

Signed-off-by: Somesh Mal <malsomesh9@gmail.com>
@SaidAltury-ibm SaidAltury-ibm requested review from SaidAltury-ibm and removed request for SaidAltury-ibm April 29, 2026 07:21
@github-actions
Copy link
Copy Markdown

Hey @malsomesh9 👋 thanks for the PR!
I'm your friendly PR Helper Bot 🤖 and I'll be riding shotgun on this one, keeping track of your PR's status to help you get it approved and merged.

This comment updates automatically as you push changes -- think of it as your PR's live scoreboard!
Here's the latest:


PR Checks

DCO Sign-off -- All commits have valid sign-offs. Nice work!


GPG Signature -- Heads up! The following commits don't have a verified GPG signature:

  • 69c5ca1 node: add safe constructors

You'll need to sign your commits with GPG (e.g. git commit -S). See the Signing Guide for a step-by-step walkthrough.


Merge Conflicts -- Oh no, this PR has merge conflicts with the base branch.

Let's get this sorted! Update your branch (e.g. rebase or merge from base) and push. See the Merge Conflicts Guide if you need a hand.


Issue Link -- This PR is not linked to any issue.

Please reference an issue using a closing keyword (e.g. Fixes #123) and ensure the issue is assigned to you. Every PR needs a home!


All checks must pass before this PR can be reviewed. You've got this!

@github-actions
Copy link
Copy Markdown

Hi @malsomesh9 👋 — the recent merge of PR #1416 has introduced a merge conflict in this PR. Please resolve the merge conflict so that this PR can be reviewed again. Thank you!

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.

1 participant