Skip to content

Fix potential panic from using host before error check in GetP2PNode#24

Merged
xzyaoi merged 3 commits intofix/#21from
copilot/sub-pr-22-again
Feb 12, 2026
Merged

Fix potential panic from using host before error check in GetP2PNode#24
xzyaoi merged 3 commits intofix/#21from
copilot/sub-pr-22-again

Conversation

Copy link
Contributor

Copilot AI commented Feb 12, 2026

Addresses review feedback from PR #22: host.ID().String() was called before checking the error returned by newHost(), which would panic if newHost returns nil host on error.

Changes:

  • Moved error check immediately after newHost() call
  • Only set MyID and P2PNode when err == nil
// Before:
host, err := newHost(ctx, seedInt, ds)
MyID = host.ID().String()  // Panics if host is nil
P2PNode = &host
if err != nil {
    common.Logger.Error("Error while creating P2P node: ", err)
    os.Exit(1)
}

// After:
host, err := newHost(ctx, seedInt, ds)
if err != nil {
    common.Logger.Error("Error while creating P2P node: ", err)
    os.Exit(1)
}
MyID = host.ID().String()
P2PNode = &host

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits February 12, 2026 10:07
Co-authored-by: xzyaoi <10499297+xzyaoi@users.noreply.github.com>
Co-authored-by: xzyaoi <10499297+xzyaoi@users.noreply.github.com>
Copilot AI changed the title [WIP] WIP Address feedback from review on pull request #22 Fix potential panic from using host before error check in GetP2PNode Feb 12, 2026
Copilot AI requested a review from xzyaoi February 12, 2026 10:10
@xzyaoi xzyaoi marked this pull request as ready for review February 12, 2026 12:36
Copilot AI review requested due to automatic review settings February 12, 2026 12:36
@xzyaoi xzyaoi merged commit 78d89aa into fix/#21 Feb 12, 2026
@xzyaoi xzyaoi deleted the copilot/sub-pr-22-again branch February 12, 2026 12:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a potential panic in GetP2PNode by ensuring newHost() errors are handled before dereferencing/using the returned host.

Changes:

  • Move the newHost() error check to occur before host.ID() is accessed.
  • Only set MyID and P2PNode after confirming err == nil.
  • Remove unused indirect Go module dependencies (tidy go.mod/go.sum).

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/internal/protocol/host.go Prevents nil-host dereference by checking err before accessing host.ID().
src/go.mod Drops unused indirect dependencies.
src/go.sum Removes corresponding unused checksums.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 56 to +62
host, err := newHost(ctx, seedInt, ds)
MyID = host.ID().String()
P2PNode = &host
if err != nil {
common.Logger.Error("Error while creating P2P node: ", err)
os.Exit(1)
}
MyID = host.ID().String()
P2PNode = &host
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

Consider adding a regression test for this nil-host panic scenario (i.e., when newHost returns (nil, err)). Because GetP2PNode calls os.Exit(1) on error, it’s hard to unit test as-is; a small refactor (e.g., injecting an exit function or returning the error instead of exiting) would make this behavior testable and prevent future regressions.

Copilot uses AI. Check for mistakes.
xzyaoi added a commit that referenced this pull request Feb 12, 2026
* fix #21

* Update src/entry/cmd/root.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Xiaozhe Yao <xiaozhe.yaoi@gmail.com>

* Update src/internal/protocol/host.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Xiaozhe Yao <xiaozhe.yaoi@gmail.com>

* Fix potential panic from using host before error check in GetP2PNode (#24)

* Initial plan

* Fix error handling in GetP2PNode - check err before using host

Co-authored-by: xzyaoi <10499297+xzyaoi@users.noreply.github.com>

* Complete fix for error handling review feedback

Co-authored-by: xzyaoi <10499297+xzyaoi@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: xzyaoi <10499297+xzyaoi@users.noreply.github.com>

---------

Signed-off-by: Xiaozhe Yao <xiaozhe.yaoi@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
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

Comments