Skip to content

fix: return error instead of log.Fatal in HolePunching - #11365

Merged
lidel merged 4 commits into
ipfs:masterfrom
reflecttypefor:master
Jun 21, 2026
Merged

fix: return error instead of log.Fatal in HolePunching#11365
lidel merged 4 commits into
ipfs:masterfrom
reflecttypefor:master

Conversation

@reflecttypefor

@reflecttypefor reflecttypefor commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

The HolePunching function in core/node/libp2p/relay.go calls log.Fatal when Swarm.EnableHolePunching is explicitly enabled but Swarm.RelayClient.Enabled is not true. log.Fatal immediately terminates the process with os.Exit(1), which bypasses normal shutdown hooks and prevents the fx framework from handling the failure gracefully. The function already has a named err return value, so returning an error is the correct Go idiom.

Replace log.Fatal(...) with err = fmt.Errorf(...) so the error propagates through fx's provider chain. This gives users a descriptive startup error message instead of an abrupt process termination. No behavioral change for the default case (where Swarm.EnableHolePunching is not explicitly set) — hole punching is still silently disabled when relay client is off.

Signed-off-by: reflecttypefor <reflecttypefor@outlook.com>
@reflecttypefor
reflecttypefor requested a review from a team as a code owner June 18, 2026 06:43
lidel added 2 commits June 21, 2026 23:01
The Changelog section is generated at release time, so hand-written
notes belong under Highlights instead.

@lidel lidel 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.

Thanks! Added test to lock this in.

@lidel
lidel merged commit ceefb9d into ipfs:master Jun 21, 2026
24 checks passed
@lidel lidel mentioned this pull request Jul 16, 2026
62 tasks
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.

2 participants