fix: make shutdown safe during active transfers

[mittalsonal]

Summary

• Prevent shutdown panics by no longer closing eventChan while transfer callbacks may still enqueue events
• Add a separate shutdown signal so event processing exits cleanly and late callbacks are dropped safely
• Switch HTTP server shutdown from hard Close() to graceful Shutdown(ctx) with a forced-close fallback
• Add a regression test proving shutdown waits for an active request to finish

Fixes #47

Validation

• git diff --check
• go test -run TestHTTPServerShutdownWaitsForActiveRequest -count=1 -v in beamsync (passes with Go 1.25.5)
• go test ./... in beamsync reaches the existing suite but local Windows profile fails TLS config tests because C:\Users\Garima Varshney\.config exists as a file, preventing os.UserConfigDir() subdirectory creation
• go test ./... in desktop requires frontend assets first (frontend/dist missing locally), matching the CI order where frontend build runs before desktop Go tests

[coderabbitai]

📝 Walkthrough

Walkthrough

HTTPServer.Shutdown is updated to attempt a 30-second graceful shutdown via http.Server.Shutdown(ctx) before falling back to http.Server.Close(). In desktop/app.go, the App struct gains eventsClosed chan struct{} and shutdownOnce sync.Once, which are used to make shutdown idempotent and stop the event-relay goroutine and callbacks from writing to a closed channel.

Changes

Graceful Shutdown Fix

Layer / File(s)	Summary
HTTPServer graceful shutdown with fallback beamsync/server.go, beamsync/server_test.go	HTTPServer.Shutdown now calls s.server.Shutdown(ctx) with a 30-second timeout and only calls s.server.Close() as a fallback on error. A new test coordinates an in-flight request with a concurrent shutdown call to assert the server waits for the active request before returning.
App event-relay shutdown coordination desktop/app.go	Adds eventsClosed chan struct{} and shutdownOnce sync.Once to App. NewApp initializes eventsClosed. processEvents switches from a range loop to a for/select that exits when eventsClosed is closed. shutdown wraps all teardown in shutdownOnce.Do and closes eventsClosed instead of eventChan. makeCallback guards against enqueuing after shutdown.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 Hoppin' through the shutdown lane,
No more panics, no more pain!
eventsClosed waves goodbye,
In-flight transfers still can fly.
Once is all we need, I say—
Graceful exits, hip hooray! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: making shutdown safe during active transfers, which addresses the core issue of the changeset.
Linked Issues check	✅ Passed	All primary objectives from issue #47 are addressed: event channel safety, graceful event processing, graceful HTTP shutdown with timeout, and regression test coverage for active requests.
Out of Scope Changes check	✅ Passed	All changes are directly related to addressing shutdown safety during active transfers as specified in issue #47; no unrelated modifications detected.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (2)

desktop/app.go (2)

199-213: 💤 Low value

Inconsistent indentation inside shutdownOnce.Do closure.

The closure body (lines 200-212) has inconsistent indentation which makes the code structure unclear.

♻️ Suggested formatting fix

 func (a *App) shutdown(ctx context.Context) {
 	a.shutdownOnce.Do(func() {
-	if a.serverApp != nil {
-		fmt.Println("🛑 Shutting down receiver server...")
-		if err := a.serverApp.Shutdown(); err != nil {
-			fmt.Println("⚠️ Server shutdown error:", err)
+		if a.serverApp != nil {
+			fmt.Println("🛑 Shutting down receiver server...")
+			if err := a.serverApp.Shutdown(); err != nil {
+				fmt.Println("⚠️ Server shutdown error:", err)
+			}
 		}
-	}
-	if a.senderApp != nil {
-		fmt.Println("🛑 Shutting down sender server...")
-		if err := a.senderApp.Shutdown(); err != nil {
-			fmt.Println("⚠️ Sender shutdown error:", err)
+		if a.senderApp != nil {
+			fmt.Println("🛑 Shutting down sender server...")
+			if err := a.senderApp.Shutdown(); err != nil {
+				fmt.Println("⚠️ Sender shutdown error:", err)
+			}
 		}
-	}
 		close(a.eventsClosed)
 	})
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@desktop/app.go` around lines 199 - 213, Fix the inconsistent indentation
inside the shutdownOnce.Do closure in the app.go file. The if statements
checking a.serverApp and a.senderApp, along with the close(a.eventsClosed) call,
should all be aligned at the same indentation level within the closure body.
Ensure all code within the anonymous function passed to shutdownOnce.Do follows
consistent indentation rules so the block structure is clear.

---

160-179: 💤 Low value

Inconsistent indentation obscures code structure.

Lines 166-174 and 175 appear to be at inconsistent indentation levels within the case block. While functionally correct (Go uses braces, not indentation), this harms readability and could mislead future maintainers.

♻️ Suggested formatting fix

 func (a *App) processEvents() {
 	for {
 		select {
 		case event, ok := <-a.eventChan:
 			if !ok {
 				return
 			}
-		if event.Name == "device_connected" {
-			currentRealIP := getLocalIP()
-			if a.currentIP != "" && a.currentIP != currentRealIP {
-				fmt.Printf("🔄 IP Change Detected! Old: %s, New: %s\n", a.currentIP, currentRealIP)
-				a.currentIP = currentRealIP
-				newURL := fmt.Sprintf("%s://%s:%s", beamsync.ServerScheme(), a.currentIP, a.currentPort)
-				a.safeEmit("url_changed", newURL)
+			if event.Name == "device_connected" {
+				currentRealIP := getLocalIP()
+				if a.currentIP != "" && a.currentIP != currentRealIP {
+					fmt.Printf("🔄 IP Change Detected! Old: %s, New: %s\n", a.currentIP, currentRealIP)
+					a.currentIP = currentRealIP
+					newURL := fmt.Sprintf("%s://%s:%s", beamsync.ServerScheme(), a.currentIP, a.currentPort)
+					a.safeEmit("url_changed", newURL)
+				}
 			}
-		}
 			a.safeEmit(event.Name, event.Data)
 		case <-a.eventsClosed:
 			return
 		}
 	}
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@desktop/app.go` around lines 160 - 179, The indentation within the case
event, ok := <-a.eventChan block is inconsistent. The if event.Name ==
"device_connected" block (lines 168-174) and the subsequent
a.safeEmit(event.Name, event.Data) call (line 175) are not properly aligned with
the preceding if !ok check. Fix this by ensuring all statements within this case
block are indented at the same level to reflect their common position within the
select statement's case handler.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@desktop/app.go`:
- Around line 199-213: Fix the inconsistent indentation inside the
shutdownOnce.Do closure in the app.go file. The if statements checking
a.serverApp and a.senderApp, along with the close(a.eventsClosed) call, should
all be aligned at the same indentation level within the closure body. Ensure all
code within the anonymous function passed to shutdownOnce.Do follows consistent
indentation rules so the block structure is clear.
- Around line 160-179: The indentation within the case event, ok :=
<-a.eventChan block is inconsistent. The if event.Name == "device_connected"
block (lines 168-174) and the subsequent a.safeEmit(event.Name, event.Data) call
(line 175) are not properly aligned with the preceding if !ok check. Fix this by
ensuring all statements within this case block are indented at the same level to
reflect their common position within the select statement's case handler.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 086cdea7-2cab-479b-94b4-42401c790ce0

📥 Commits

Reviewing files that changed from the base of the PR and between 5e2fe38 and 1446063.

📒 Files selected for processing (3)

• beamsync/server.go
• beamsync/server_test.go
• desktop/app.go