[client] Don't abort UI debug bundle when up/down fails by lixmal · Pull Request #5780 · netbirdio/netbird

lixmal · 2026-04-02T12:15:51Z

Describe your changes

Mirror the CLI fix from #5657 for the UI debug bundle: when Up/Down/SetLogLevel/SetSyncResponsePersistence calls fail during debug collection, log a warning and continue instead of aborting the entire bundle.

Log warnings instead of returning errors in configureServiceForDebug, so the bundle is still generated even if service state changes fail
Downgrade restoreServiceState error logging from Errorf to Warnf for consistency

Issue ticket number and link

Stack

Checklist

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
- Debug data collection now continues even if initial setup calls fail; failures are logged without blocking the process.
- Timing/waiting delays during debug setup now occur only after successful operations to avoid unnecessary waits.
Chores
- Reduced log severity for restore-related failures from error to warning.
- Added conditional tracking to ensure the service is restored to its prior "up" state when appropriate.

coderabbitai · 2026-04-02T12:16:09Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 85258419-f3b7-4e1b-b46f-bc6e8156adda

📥 Commits

Reviewing files that changed from the base of the PR and between 0293cc2 and ad2c07d.

📒 Files selected for processing (2)

client/cmd/debug.go
client/ui/debug.go

📝 Walkthrough

Walkthrough

configureServiceForDebug no longer returns an error and now logs RPC failures instead of failing. Sleep delays were moved into success branches. Debug collection proceeds even if service setup RPCs fail. A new debugInitialState.needsRestoreUp field tracks whether to bring the service back up; restore logic uses this flag and downgrades some restore failures to warnings.

Changes

Cohort / File(s)	Summary
Debug service configuration `client/ui/debug.go`	Removed error return from `configureServiceForDebug`; RPC failures (Up, SetLogLevel, Down, SetSyncResponsePersistence) are logged, not returned. `time.Sleep` calls moved to success branches. Added `debugInitialState.needsRestoreUp` and changed restore log severity from `Errorf` to `Warnf`.
Debug command lifecycle / restore flow `client/cmd/debug.go`	Introduced `needsRestoreUp` tracking in `runForDuration` to mark when a final `Up` (restoration) is required; set/cleared based on successful Down/Up calls and conditionally perform final `Up` if still needed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

[Management/Client] Trigger debug bundle runs from API/Dashboard (#4592) #4832: Modifies client debug flow in client/ui/debug.go and client/cmd/debug.go—overlaps with these changes.
[client] Don't abort debug for command when up/down fails #5657: Makes debug lifecycle RPC failures non-fatal and introduces/adjusts state tracking for restore (needsRestoreUp).

Suggested reviewers

pappz
mlsmaycon

Poem

🐰 I hopped in logs and RPC streams bright,

Warnings now whisper where errors took flight,
I nudged the up-flag back into place,
So debugging blooms at gentler pace,
Hooray — the bundle hops on through the night! 🎋

🚥 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 accurately summarizes the main change: preventing debug bundle generation from aborting when service up/down operations fail.
Description check	✅ Passed	The description includes the key motivation (mirroring CLI fix `#5657`), explains the specific changes made, and properly documents why the changes are needed.

_{✏️ 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/ui-debug-bundle-no-abort

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.}

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dfa4b328-932c-4005-a8dd-badac43dbcea

📥 Commits

Reviewing files that changed from the base of the PR and between c2c6396 and 0293cc2.

📒 Files selected for processing (1)

client/ui/debug.go

…o-abort

sonarqubecloud · 2026-04-08T05:50:50Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

pappz · 2026-04-08T08:26:48Z

+			log.Warnf("failed to bring service up: %v", err)
+		} else {
+			log.Info("Service brought up for debug")
+			time.Sleep(time.Second * 10)


Is not it block the UI?

It's running in a goroutine, and it's just moved. The sleep was already there

pappz · 2026-04-08T08:27:05Z

+		log.Warnf("failed to bring service down: %v", err)
+	} else {
+		state.needsRestoreUp = !state.wasDown
+		time.Sleep(time.Second)


Is not it block the UI?

It's running in a goroutine, and it's just moved. The sleep was already there

pappz · 2026-04-08T08:27:19Z

+		log.Warnf("failed to bring service back up: %v", err)
+	} else {
+		state.needsRestoreUp = false
+		time.Sleep(time.Second * 3)


Is not it block the UI?

It's running in a goroutine, and it's just moved. The sleep was already there

Don't abort UI debug bundle when up/down fails

0293cc2

coderabbitai Bot reviewed Apr 2, 2026

View reviewed changes

Comment thread client/ui/debug.go

lixmal added 2 commits April 7, 2026 18:06

Restore up state when re-Up fails during debug bundle

ad2c07d

Merge remote-tracking branch 'origin/main' into fix/ui-debug-bundle-n…

4f6b155

…o-abort

pappz reviewed Apr 8, 2026

View reviewed changes

pappz approved these changes Apr 8, 2026

View reviewed changes

lixmal merged commit 332c624 into main Apr 8, 2026
44 of 45 checks passed

lixmal deleted the fix/ui-debug-bundle-no-abort branch April 8, 2026 08:33

Uh oh!

Conversation

lixmal commented Apr 2, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes

Issue ticket number and link

Stack

Checklist

Documentation

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

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud Bot commented Apr 8, 2026

Quality Gate passed

Uh oh!

pappz Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

lixmal Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

pappz Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

lixmal Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

pappz Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

lixmal Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lixmal commented Apr 2, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 2, 2026 •

edited

Loading