From 978b665aa66d21d70e2cd1ef074da3729973f149 Mon Sep 17 00:00:00 2001
From: mrhid6 <mrhid6@gmail.com>
Date: Fri, 31 Jul 2026 09:25:27 +0100
Subject: [PATCH] fix: stop local relay teardown from logging a spurious
 proxy_failed reason

Session.Close now closing its own accepted conn (from the prior fix wave)
made net.ErrClosed on the guacd-side reader indistinguishable from a real
remote failure, so a normal browser-tab close could race the handler's
defer and intermittently log console.proxy_failed on a healthy session.
Add a closing flag, set before Close's sync.Once body actually tears
anything down, that setReason respects -- a deliberate local teardown can
no longer produce or race in a failure reason, while Close's own explicit
reason argument still wins normally.
---
 server/internal/proxy/session.go | 36 ++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/server/internal/proxy/session.go b/server/internal/proxy/session.go
index d76b932..cdd07a1 100644
--- a/server/internal/proxy/session.go
+++ b/server/internal/proxy/session.go
@@ -32,10 +32,11 @@ type Session struct {
 	allowed  []string
 	timeout  time.Duration
 
-	once   sync.Once
-	mu     sync.Mutex
-	reason string
-	conn   net.Conn
+	once    sync.Once
+	mu      sync.Mutex
+	reason  string
+	conn    net.Conn
+	closing bool
 
 	rendezvous *time.Timer
 }
@@ -71,30 +72,43 @@ func (s *Session) Reason() string {
 	return s.reason
 }
 
+// setReason records r as the session's failure reason, first write wins. It is
+// a no-op once a deliberate teardown (Close) has begun: a local Close closing
+// the conn out from under the relay goroutines produces exactly the kind of
+// error (net.ErrClosed, a broken pipe on write, ...) that looks like a remote
+// failure but is not one, and must not overwrite — or race to set — the real
+// reason, or invent one where a clean local close has none.
 func (s *Session) setReason(r string) {
 	s.mu.Lock()
-	if s.reason == "" {
+	if s.reason == "" && !s.closing {
 		s.reason = r
 	}
 	s.mu.Unlock()
 }
 
 // Close tears the session down once. A non-empty reason is recorded only if no
-// reason has been recorded already. It closes both the listener and, if a
-// connection has already been accepted, that connection too — an unconditional
-// kill for the whole relay chain regardless of which stage it is in.
+// reason has been recorded already, and only before teardown begins. It closes
+// both the listener and, if a connection has already been accepted, that
+// connection too — an unconditional kill for the whole relay chain regardless
+// of which stage it is in.
 func (s *Session) Close(reason string) {
 	if reason != "" {
 		s.setReason(reason)
 	}
 	s.once.Do(func() {
+		// Recorded before anything is actually closed: the reader/writer
+		// goroutines in relay() call setReason from the errors this Close
+		// itself is about to cause (a closed conn, a closed stream), and
+		// those must be recognised as teardown noise, not a genuine failure.
+		s.mu.Lock()
+		s.closing = true
+		conn := s.conn
+		s.mu.Unlock()
+
 		if s.rendezvous != nil {
 			s.rendezvous.Stop()
 		}
 		_ = s.listener.Close()
-		s.mu.Lock()
-		conn := s.conn
-		s.mu.Unlock()
 		if conn != nil {
 			_ = conn.Close()
 		}