Skip to content

cmd/wol-proxy: tweak logs to show what is causing wake ups #356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 25, 2025
Merged

cmd/wol-proxy: tweak logs to show what is causing wake ups #356

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8a7b3ab
cmd/wol-proxy: tweak logs to show what is causing wake ups
mostlygeek Oct 21, 2025
6d71b61
cmd/wol-proxy: add skip wakeup
mostlygeek Oct 22, 2025
6cfbb28
cmd/wol-proxy: replace ticker with SSE connection
mostlygeek Oct 24, 2025
2b04ddd
cmd/wol-proxy: increase scanner buffer size
mostlygeek Oct 24, 2025
7164ea8
cmd/wol-proxy: improve failure tracking
mostlygeek Oct 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ GOOS ?= $(shell go env GOOS 2>/dev/null || echo linux)
GOARCH ?= $(shell go env GOARCH 2>/dev/null || echo amd64)
wol-proxy: $(BUILD_DIR)
@echo "Building wol-proxy"
go build -o $(BUILD_DIR)/wol-proxy-$(GOOS)-$(GOARCH) cmd/wol-proxy/wol-proxy.go
go build -o $(BUILD_DIR)/wol-proxy-$(GOOS)-$(GOARCH)-$(shell date +%Y-%m-%d) cmd/wol-proxy/wol-proxy.go

# Phony targets
.PHONY: all clean ui mac linux windows simple-responder simple-responder-windows test test-all test-dev wol-proxy
111 changes: 83 additions & 28 deletions cmd/wol-proxy/wol-proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"bufio"
"context"
"errors"
"flag"
Expand All @@ -13,6 +14,7 @@ import (
"net/url"
"os"
"os/signal"
"strings"
"sync"
"time"
)
Expand Down Expand Up @@ -92,14 +94,9 @@ func main() {
}()

// graceful shutdown
ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt)
defer stop()
ctx, _ := signal.NotifyContext(context.Background(), os.Interrupt)
Copy link

@coderabbitai coderabbitai bot Oct 24, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Resource leak: cancel function must be called.

The cancel function returned by signal.NotifyContext should be called to release resources. Discarding it with _ causes a resource leak.

Apply this diff:

-	ctx, _ := signal.NotifyContext(context.Background(), os.Interrupt)
+	ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt)
+	defer stop()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ctx, _ := signal.NotifyContext(context.Background(), os.Interrupt)
ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt)
defer stop()
🤖 Prompt for AI Agents 
In cmd/wol-proxy/wol-proxy.go around line 97, the cancel function returned by
signal.NotifyContext is discarded which causes a resource leak; change the call
to capture the cancel function (ctx, cancel := signal.NotifyContext(...)) and
ensure you call cancel() when done (e.g., immediately after creating ctx add
defer cancel() in main or the surrounding function) so resources are released.

<-ctx.Done()
shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
if err := server.Shutdown(shutdownCtx); err != nil {
slog.Error("server shutdown error", "error", err)
}
server.Close()
Copy link

@coderabbitai coderabbitai bot Oct 24, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Immediate close terminates active connections abruptly.

server.Close() immediately closes all active connections without waiting for in-flight requests to complete. For graceful shutdown, use server.Shutdown(ctx) with a timeout context.

Apply this diff:

 	ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt)
+	defer stop()
 	<-ctx.Done()
-	server.Close()
+	shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	if err := server.Shutdown(shutdownCtx); err != nil {
+		slog.Error("error during shutdown", "error", err)
+	}
🤖 Prompt for AI Agents 
In cmd/wol-proxy/wol-proxy.go around line 99, replace the immediate
server.Close() call with a graceful shutdown: create a context with timeout
(e.g., context.WithTimeout(context.Background(), shutdownTimeout)), call
server.Shutdown(ctx) and handle its error, and defer cancel the context; if
Shutdown returns an error (or times out) fall back to server.Close() to force
close and log the outcome. Ensure shutdownTimeout is a sensible duration and
propagate/log any errors from Shutdown/Close.

}

type upstreamStatus string
Expand All @@ -124,37 +121,88 @@ func newProxy(url *url.URL) *proxyServer {
failCount: 0,
}

// start a goroutien to check upstream status
// start a goroutine to monitor upstream status via SSE
go func() {
checkUrl := url.Scheme + "://" + url.Host + "/wol-health"
client := &http.Client{Timeout: time.Second}
ticker := time.NewTicker(2 * time.Second)
defer ticker.Stop()
for range ticker.C {
eventsUrl := url.Scheme + "://" + url.Host + "/api/events"
client := &http.Client{
Timeout: 0, // No timeout for SSE connection
}

slog.Debug("checking upstream status at", "url", checkUrl)
resp, err := client.Get(checkUrl)
waitDuration := 10 * time.Second
Comment on lines +127 to +131
Copy link

@coderabbitai coderabbitai bot Oct 24, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

SSE client can hang indefinitely on connect; set dial/TLS timeouts

Timeout=0 is correct for the stream, but the default Transport has no dial or TLS handshake timeouts, so a dead upstream can block forever.

-        client := &http.Client{
-            Timeout: 0, // No timeout for SSE connection
-        }
+        tr := &http.Transport{
+          DialContext:         (&net.Dialer{Timeout: 10 * time.Second, KeepAlive: 30 * time.Second}).DialContext,
+          TLSHandshakeTimeout: 10 * time.Second,
+          IdleConnTimeout:     90 * time.Second,
+          MaxIdleConns:        100,
+        }
+        client := &http.Client{Transport: tr, Timeout: 0} // no overall timeout; stream is long‑lived
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
client := &http.Client{
Timeout: 0, // No timeout for SSE connection
}
slog.Debug("checking upstream status at", "url", checkUrl)
resp, err := client.Get(checkUrl)
waitDuration := 10 * time.Second
tr := &http.Transport{
DialContext: (&net.Dialer{Timeout: 10 * time.Second, KeepAlive: 30 * time.Second}).DialContext,
TLSHandshakeTimeout: 10 * time.Second,
IdleConnTimeout: 90 * time.Second,
MaxIdleConns: 100,
}
client := &http.Client{Transport: tr, Timeout: 0} // no overall timeout; stream is long-lived
waitDuration := 10 * time.Second
🤖 Prompt for AI Agents 
In cmd/wol-proxy/wol-proxy.go around lines 127 to 131, the HTTP client is
created with Timeout: 0 which is fine for an SSE stream but leaves the default
Transport with no dial or TLS handshake timeouts so a dead upstream can block
forever; replace the default transport with a custom http.Transport that sets a
net.Dialer with a reasonable Dial timeout (e.g., 10s) and TLSHandshakeTimeout
(and optionally ResponseHeaderTimeout/ExpectContinueTimeout) and assign that
Transport to the http.Client while keeping Timeout: 0 so the stream itself is
not terminated.


// drain the body
if err == nil && resp != nil {
_, _ = io.Copy(io.Discard, resp.Body)
_ = resp.Body.Close()
for {
slog.Debug("connecting to SSE endpoint", "url", eventsUrl)

req, err := http.NewRequest("GET", eventsUrl, nil)
Comment on lines 126 to 136
Copy link

@coderabbitai coderabbitai bot Oct 24, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Build SSE URL robustly; avoid shadowing net/url; fix initialism

String concat drops any upstream base path and the param name url shadows the net/url package; eventsUrl casing violates ST1003.

-        eventsUrl := url.Scheme + "://" + url.Host + "/api/events"
+        // Build from upstream, preserving base path.
+        eventsURL, err := neturl.JoinPath(upstream.String(), "api", "events")
+        if err != nil {
+          slog.Warn("failed to build SSE URL", "error", err)
+          proxy.setStatus(notready)
+          proxy.incFail()
+          time.Sleep(waitDuration)
+          continue
+        }

Supporting changes outside this hunk:

  • Import alias: neturl "net/url".
  • Rename newProxy param to avoid shadowing and use initialism:
    func newProxy(upstream *url.URL) *proxyServer {
  • Update callsite: proxy := newProxy(upstreamURL)
  • Rename variable to eventsURL.

Based on coding guidelines (staticcheck ST1003).

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In cmd/wol-proxy/wol-proxy.go around lines 126-136, the code builds the SSE
endpoint by string concatenation, shadows the net/url package with a variable
named url, and uses non-API-compliant casing for eventsUrl; fix by: rename the
function parameter and local variable to avoid shadowing (e.g.,
newProxy(upstream *url.URL) or similar) and update its callsite, add or use the
neturl import alias if needed (neturl "net/url"), construct the SSE URL robustly
using URL methods (e.g., ResolveReference or URL.JoinPath with the upstream base
so the upstream path and query are preserved) and rename eventsUrl to eventsURL
to follow ST1003; ensure the http.NewRequest uses the computed eventsURL string.

if err != nil {
slog.Warn("failed to create SSE request", "error", err)
proxy.setStatus(notready)
proxy.failCount++
time.Sleep(waitDuration)
continue
}

if err == nil && resp != nil && resp.StatusCode == http.StatusOK {
slog.Debug("upstream status: ready")
proxy.setStatus(ready)
proxy.statusMutex.Lock()
proxy.failCount = 0
proxy.statusMutex.Unlock()
} else {
slog.Debug("upstream status: notready", "error", err)
req.Header.Set("Accept", "text/event-stream")
req.Header.Set("Cache-Control", "no-cache")
req.Header.Set("Connection", "keep-alive")

resp, err := client.Do(req)
if err != nil {
slog.Error("failed to connect to SSE endpoint", "error", err)
proxy.setStatus(notready)
proxy.statusMutex.Lock()
proxy.failCount++
proxy.statusMutex.Unlock()
time.Sleep(10 * time.Second)
Copy link

@coderabbitai coderabbitai bot Oct 24, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Use waitDuration consistently for retry delays.

These lines use hardcoded 10 * time.Second but line 131 defines waitDuration for this purpose. Use the variable consistently.

Apply this diff:

-				time.Sleep(10 * time.Second)
+				time.Sleep(waitDuration)

Apply to both lines 156 and 166.

Also applies to: 166-166

🤖 Prompt for AI Agents 
In cmd/wol-proxy/wol-proxy.go around lines 156 and 166, the retry sleeps use a
hardcoded 10 * time.Second instead of the previously defined waitDuration;
replace those hardcoded sleep calls with time.Sleep(waitDuration) (or simply use
waitDuration where appropriate) so both retry delays consistently use the
waitDuration variable.

continue
}

if resp.StatusCode != http.StatusOK {
slog.Warn("SSE endpoint returned non-OK status", "status", resp.StatusCode)
_, _ = io.Copy(io.Discard, resp.Body)
_ = resp.Body.Close()
proxy.setStatus(notready)
proxy.failCount++
time.Sleep(10 * time.Second)
continue
}

// Successfully connected to SSE endpoint
slog.Info("connected to SSE endpoint, upstream ready")
proxy.setStatus(ready)
proxy.failCount = 0

// Read from the SSE stream to detect disconnection
scanner := bufio.NewScanner(resp.Body)

// use a fairly large buffer to avoid scanner errors when reading large SSE events
buf := make([]byte, 0, 1024*1024*2)
scanner.Buffer(buf, 1024*1024*2)
events := 0
if slog.Default().Enabled(context.Background(), slog.LevelDebug) {
fmt.Print("Events: ")
}
for scanner.Scan() {
if slog.Default().Enabled(context.Background(), slog.LevelDebug) {
// Just read the events to keep connection alive
// We don't need to process the event data
events++
fmt.Printf("%d, ", events)
}
}
fmt.Println()
Comment on lines 180 to 191
Copy link

@coderabbitai coderabbitai bot Oct 24, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Replace fmt debug output with structured slog logging.

Using fmt.Print and fmt.Printf for debug output breaks structured logging and always prints to stdout. Line 193 also executes unconditionally, printing an empty line even when debug is disabled.

Apply this diff:

 			events := 0
-			if slog.Default().Enabled(context.Background(), slog.LevelDebug) {
-				fmt.Print("Events: ")
-			}
 			for scanner.Scan() {
-				if slog.Default().Enabled(context.Background(), slog.LevelDebug) {
-					// Just read the events to keep connection alive
-					// We don't need to process the event data
-					events++
-					fmt.Printf("%d, ", events)
-				}
+				events++
 			}
-			fmt.Println()
+			slog.Debug("SSE stream ended", "events_received", events)
🤖 Prompt for AI Agents 
In cmd/wol-proxy/wol-proxy.go around lines 182 to 193, replace the fmt debug
output with structured slog calls and avoid unconditional printing: remove
fmt.Print/Printf and the unconditional fmt.Println; inside the existing
slog.Default().Enabled(context.Background(), slog.LevelDebug) checks, increment
events and emit structured debug logs such as slog.Debug("events read",
slog.Int("count", events)) (or slog.DebugContext with ctx if available) so
output only appears when debug is enabled and uses structured attributes instead
of stdout prints.

if err := scanner.Err(); err != nil {
slog.Error("error reading from SSE stream", "error", err)
}

// Connection closed or error occurred
_ = resp.Body.Close()
slog.Info("SSE connection closed, upstream not ready")
proxy.setStatus(notready)
proxy.failCount++

// Wait before reconnecting
time.Sleep(waitDuration)
}
}()

Expand All @@ -175,7 +223,14 @@ func (p *proxyServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}

if p.getStatus() == notready {
slog.Info("upstream not ready, sending magic packet", "mac", *flagMac)
path := r.URL.Path
if strings.HasPrefix(path, "/api/events") {
slog.Debug("Skipping wake up", "req", path)
w.WriteHeader(http.StatusNoContent)
return
}
Comment on lines +223 to +227
Copy link

@coderabbitai coderabbitai bot Oct 24, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Path prefix check may match unintended paths.

strings.HasPrefix(path, "/api/events") will match paths like "/api/eventstream", "/api/events2", or "/api/events_other" in addition to "/api/events" and "/api/events/...". If the intent is to only skip wake-up for the exact "/api/events" endpoint or paths under that namespace, consider a more precise check.

Apply this diff for an exact match or a path under /api/events/:

-		if strings.HasPrefix(path, "/api/events") {
+		if path == "/api/events" || strings.HasPrefix(path, "/api/events/") {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if strings.HasPrefix(path, "/api/events") {
slog.Debug("Skipping wake up", "req", path)
w.WriteHeader(http.StatusNoContent)
return
}
if path == "/api/events" || strings.HasPrefix(path, "/api/events/") {
slog.Debug("Skipping wake up", "req", path)
w.WriteHeader(http.StatusNoContent)
return
}
🤖 Prompt for AI Agents 
In cmd/wol-proxy/wol-proxy.go around lines 223 to 227, the current
strings.HasPrefix(path, "/api/events") will also match unintended routes like
"/api/eventstream" or "/api/events2"; change the guard to only match the exact
"/api/events" or any path under that namespace by checking path == "/api/events"
|| strings.HasPrefix(path, "/api/events/") so only the endpoint and its subpaths
are skipped.


slog.Info("upstream not ready, sending magic packet", "req", path, "from", r.RemoteAddr)
if err := sendMagicPacket(*flagMac); err != nil {
slog.Warn("failed to send magic WoL packet", "error", err)
}
Expand Down