Skip to content

[client] Don't abort debug for command when up/down fails#5657

Merged
lixmal merged 1 commit intomainfrom
fix/debug-for-bundle-on-failure
Mar 23, 2026
Merged

[client] Don't abort debug for command when up/down fails#5657
lixmal merged 1 commit intomainfrom
fix/debug-for-bundle-on-failure

Conversation

@lixmal
Copy link
Copy Markdown
Collaborator

@lixmal lixmal commented Mar 23, 2026

Describe your changes

  • netbird debug for previously aborted early if Up, Down, or SetSyncResponsePersistence failed, never reaching bundle generation
  • These failures are now non-fatal: errors are printed to stderr and the command continues to create the debug bundle
  • State restoration at the end (bringing service back down, restoring log level) is also best-effort instead of fatal

The whole point of debug for is to capture diagnostic data, often when things are broken. Aborting before creating the bundle defeats the purpose.

Issue ticket number and link

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

  • Bug Fixes

    • Improved resilience of debug operations; the debug process now continues execution when individual steps fail, providing detailed success and error feedback for better diagnostics.
  • Chores

    • Enhanced logging messages for service state transitions during debug operations.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 68f47739-14a8-4f0d-b263-cfdd90d97e21

📥 Commits

Reviewing files that changed from the base of the PR and between fd9d430 and faf7eef.

📒 Files selected for processing (1)
  • client/cmd/debug.go

📝 Walkthrough

Walkthrough

The change modifies error handling in a debug command function, shifting from returning errors on service lifecycle failures (client.Up, client.Down) to printing errors to stderr and continuing execution. Success branches now log status messages like "netbird up" and "netbird down" at various transition points.

Changes

Cohort / File(s) Summary
Service Lifecycle Error Handling and Status Logging
client/cmd/debug.go
Modified error handling for client.Up, client.Down, sync response persistence, and log-level restoration. Changed from returning errors to printing to stderr via cmd.PrintErrf() while continuing execution. Added success logging that prints status messages ("netbird up", "netbird down", log-level restoration) for conditional branches.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 When up and down dance across the debug trail,
Errors once fatal now whisper, not wail,
Success messages bloom where failures once grew,
The service keeps hopping—resilient and true! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: preventing the debug command from aborting when up/down operations fail.
Description check ✅ Passed The description addresses all critical sections: changes are well-explained, rationale is clear, and documentation needs are justified. All required checklist items are completed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/debug-for-bundle-on-failure

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

@lixmal lixmal merged commit 2313494 into main Mar 23, 2026
40 of 42 checks passed
@lixmal lixmal deleted the fix/debug-for-bundle-on-failure branch March 23, 2026 13:04
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