Skip to content

fix: propagate ping spawn errors in verify_port_forwarding#556

Closed
claude-claude[bot] wants to merge 1 commit intofix-pasta-restore-probefrom
claude/fix-22653454450
Closed

fix: propagate ping spawn errors in verify_port_forwarding#556
claude-claude[bot] wants to merge 1 commit intofix-pasta-restore-probefrom
claude/fix-22653454450

Conversation

@claude-claude
Copy link
Contributor

@claude-claude claude-claude bot commented Mar 4, 2026

Auto-Fix for PR #555

Issues Fixed

  • [MEDIUM] Swallowed spawn errors: verify_port_forwarding() used if let Ok(...) on the ping command result, silently discarding spawn failures (e.g., nsenter not found, permission denied). This caused a 5-second busy loop with no backoff, ending with a misleading "ARP not resolved" error instead of reporting the actual spawn failure.
  • [LOW] Stale comment in snapshot.rs: Comment described the old ip neigh show approach; updated to reflect the new ping-based ARP resolution.

Changes

  • src/network/pasta.rs: Replace if let Ok(ref out) = ping_result with .context("running ping via nsenter in namespace")? to propagate spawn errors immediately (matching the old code's behavior with ip neigh show)
  • src/commands/snapshot.rs: Update comment at the verify_port_forwarding() call site to accurately describe the ping → port probe two-step verification

Generated by Claude | Review Run

The new ping-based ARP check used `if let Ok(...)` which silently
swallowed command spawn failures (nsenter/ping not found, permission
denied). This could cause a 5-second busy loop with a misleading
"ARP not resolved" error. Propagate spawn errors immediately with
`.context()?` to match the old code's behavior.

Also update stale comment in snapshot.rs to reflect the ping-based
ARP resolution approach.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@ejc3
Copy link
Owner

ejc3 commented Mar 4, 2026

Cherry-picked into fix-pasta-restore-probe

@ejc3 ejc3 closed this Mar 4, 2026
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