From f852689104a7a0db73494f7a8b4b5ae363ade7f0 Mon Sep 17 00:00:00 2001
From: Benson Wong <mostlygeek@gmail.com>
Date: Sat, 25 Oct 2025 20:40:05 -0700
Subject: [PATCH] proxy: add panic recovery to Process.ProxyRequest (#363)

Switching to use httputil.ReverseProxy in #342 introduced a possible
panic if a client disconnects while streaming the body. Since llama-swap
does not use http.Server the recover() is not automatically there.

- introduce a recover() in Process.ProxyRequest to recover and log the
  event
- add TestProcess_ReverseProxyPanicIsHandled to reproduce and test the
  fix

fixes: #362
---
 proxy/process.go      | 12 ++++++++
 proxy/process_test.go | 71 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+)

diff --git a/proxy/process.go b/proxy/process.go
index a110748..775f307 100644
--- a/proxy/process.go
+++ b/proxy/process.go
@@ -499,6 +499,18 @@ func (p *Process) ProxyRequest(w http.ResponseWriter, r *http.Request) {
 		startDuration = time.Since(beginStartTime)
 	}
 
+	// recover from http.ErrAbortHandler panics that can occur when the client
+	// disconnects before the response is sent
+	defer func() {
+		if r := recover(); r != nil {
+			if r == http.ErrAbortHandler {
+				p.proxyLogger.Infof("<%s> recovered from client disconnection during streaming", p.ID)
+			} else {
+				p.proxyLogger.Infof("<%s> recovered from panic: %v", p.ID, r)
+			}
+		}
+	}()
+
 	if p.reverseProxy != nil {
 		p.reverseProxy.ServeHTTP(w, r)
 	} else {
diff --git a/proxy/process_test.go b/proxy/process_test.go
index 78922b3..de22661 100644
--- a/proxy/process_test.go
+++ b/proxy/process_test.go
@@ -494,3 +494,74 @@ func TestProcess_EnvironmentSetCorrectly(t *testing.T) {
 	assert.Equal(t, len(process1.cmd.Environ())+2, len(process2.cmd.Environ()), "process2 should have 2 more environment variables than process1")
 
 }
+
+// TestProcess_ReverseProxyPanicIsHandled tests that panics from
+// httputil.ReverseProxy in Process.ProxyRequest(w, r) do not bubble up and are
+// handled appropriately.
+//
+// httputil.ReverseProxy will panic with http.ErrAbortHandler when it has sent headers
+// can't copy the body. This can be caused by a client disconnecting before the full
+// response is sent from some reason.
+//
+// bug: https://github.com/mostlygeek/llama-swap/issues/362
+// see: https://github.com/golang/go/issues/23643 (where panic was added to httputil.ReverseProxy)
+func TestProcess_ReverseProxyPanicIsHandled(t *testing.T) {
+	// Add defer/recover to catch any panics that aren't handled by ProxyRequest
+	// If this recover() is hit, it means ProxyRequest didn't handle the panic properly
+	defer func() {
+		if r := recover(); r != nil {
+			t.Fatalf("ProxyRequest should handle panics from reverseProxy.ServeHTTP, but panic was not caught: %v", r)
+		}
+	}()
+
+	expectedMessage := "panic_test"
+	config := getTestSimpleResponderConfig(expectedMessage)
+
+	process := NewProcess("panic-test", 5, config, debugLogger, debugLogger)
+	defer process.Stop()
+
+	// Start the process
+	err := process.start()
+	assert.Nil(t, err)
+	assert.Equal(t, StateReady, process.CurrentState())
+
+	// Create a custom ResponseWriter that simulates a client disconnect
+	// by panicking when Write is called after headers are sent
+	panicWriter := &panicOnWriteResponseWriter{
+		ResponseRecorder: httptest.NewRecorder(),
+		shouldPanic:      true,
+	}
+
+	// Make a request that will trigger the panic
+	req := httptest.NewRequest("GET", "/slow-respond?echo=test&delay=100ms", nil)
+
+	// This should panic inside reverseProxy.ServeHTTP when the panicWriter.Write() is called.
+	// ProxyRequest should catch and handle this panic gracefully.
+	process.ProxyRequest(panicWriter, req)
+
+	// If we get here, the panic was properly recovered in ProxyRequest
+	// The process should still be in a ready state
+	assert.Equal(t, StateReady, process.CurrentState())
+}
+
+// panicOnWriteResponseWriter is a ResponseWriter that panics on Write
+// to simulate a client disconnect after headers are sent
+// used by: TestProcess_ReverseProxyPanicIsHandled
+type panicOnWriteResponseWriter struct {
+	*httptest.ResponseRecorder
+	shouldPanic   bool
+	headerWritten bool
+}
+
+func (w *panicOnWriteResponseWriter) WriteHeader(statusCode int) {
+	w.headerWritten = true
+	w.ResponseRecorder.WriteHeader(statusCode)
+}
+
+func (w *panicOnWriteResponseWriter) Write(b []byte) (int, error) {
+	if w.shouldPanic && w.headerWritten {
+		// Simulate the panic that httputil.ReverseProxy throws
+		panic(http.ErrAbortHandler)
+	}
+	return w.ResponseRecorder.Write(b)
+}