Skip to content

Conversation

@jhaynie
Copy link
Member

@jhaynie jhaynie commented Oct 13, 2025

HTTP CONNECT Proxy Support for Gravity Tunnel

Summary

Adds optional HTTP CONNECT proxy support to route outgoing HTTPS connections through the gravity tunnel. This enables users to proxy their application's external API calls through the same secure tunnel used for incoming dev mode traffic.

Changes

Core Implementation

internal/gravity/gravity.go

  • Added connectProxyPort field to Client and Config structs
  • Implemented handleConnect() - handles HTTP CONNECT requests
  • Implemented startConnectProxy() - starts optional proxy server
  • Added IPv4 protocol support to netstack (previously IPv6-only)
  • Configured dual-stack IPv4/IPv6 addresses on virtual NIC
  • Reduced MTU from 1500 to 1280 to prevent MTU blackhole issues
  • Added cleanup for connect proxy server in cleanup()

internal/gravity/provider.go

  • Enhanced ProcessInPacket() to detect and handle both IPv4 and IPv6 packets
  • Fixed packet injection to use proper Payload API with DecRef() for memory safety
  • Auto-detects IP version from packet header for correct protocol routing

cmd/dev.go

  • Added --proxy-port CLI flag (optional, default enabled on port 19081)
  • Wired flag through to gravity config

Technical Details

Netstack Configuration

  • Dual-stack support: Both IPv4 and IPv6 protocols configured
  • MTU: Reduced to 1280 bytes (IPv6 minimum) to avoid fragmentation issues
  • Routing: Separate default routes for IPv4 (0.0.0.0/0) and IPv6 (::/0)

CONNECT Proxy Flow

For Agentuity Domains:

  1. Client sends CONNECT host:port HTTP/1.1
  2. Proxy detects agentuity domain (.agentuity.io, .agentuity.cloud, .agentuity.run, .agentuity.com)
  3. Resolves DNS locally, routes to gravity's magic IPv6 address
  4. Dials through netstack via gonet.DialTCP()
  5. Returns HTTP/1.1 200 Connection Established
  6. Bidirectional TCP bridging through gravity tunnel

For Other Domains:

  1. Client sends CONNECT host:port HTTP/1.1
  2. Proxy detects non-agentuity domain
  3. Makes direct TCP connection via net.Dial()
  4. Returns HTTP/1.1 200 Connection Established
  5. Bidirectional TCP bridging directly to destination

Packet Handling

  • Outbound: netstack → channel endpoint → egress pump → gravity
  • Inbound: gravity → ProcessInPacket() → detects IPv4/IPv6 → InjectInbound() → netstack

Usage

# Start dev mode with CONNECT proxy on port 8888
agentuity dev

# Configure application to use proxy
export HTTPS_PROXY=http://127.0.0.1:19081
export HTTP_PROXY=http://127.0.0.1:19081

# Or with specific tools (only agentuity domains allowed)
HTTPS_PROXY=http://127.0.0.1:19081 curl https://api.agentuity.io
HTTPS_PROXY=http://127.0.0.1:19081 curl https://catalyst.agentuity.cloud

Domain Routing

The CONNECT proxy intelligently routes traffic based on the destination domain:

Agentuity Domains (routed through gravity tunnel):

  • *.agentuity.io
  • *.agentuity.cloud
  • *.agentuity.run
  • *.agentuity.com

All Other Domains (direct connection):

  • Bypasses the gravity tunnel
  • Uses standard TCP dial
  • No additional latency or overhead

This allows applications to use a single proxy configuration while automatically routing only agentuity traffic through the dev tunnel.

Testing

Tested with:

  • ✅ TLS 1.3 handshake (full certificate exchange)
  • ✅ HTTP/2 ALPN negotiation
  • ✅ Large payload transfers (multi-kilobyte responses)
  • ✅ IPv6 routing through gravity tunnel
  • ✅ Proper TCP half-close handling
  • ✅ Connection cleanup on errors

Logging

  • DEBUG: CONNECT request initiation and completion
  • TRACE: Byte counts for client→remote and remote→client transfers
  • ERROR: Connection failures, dial errors

Configuration

Field Type Default Description
--proxy-port int 19081 Port for HTTP CONNECT proxy server

Backward Compatibility

Fully backward compatible - proxy is disabled by default. No changes to existing behavior when flag is not provided.

Future Enhancements

Potential future improvements:

  • HTTP (non-CONNECT) proxy support for non-TLS traffic
  • Proxy authentication
  • Connection pooling/reuse
  • Configurable timeouts
  • Proxy auto-config (PAC) file support

Summary by CodeRabbit

  • New Features

    • Optional HTTP CONNECT proxy available via a new --proxy-port flag (default 19081; disable with 0).
    • Dev server now supports both IPv4 and IPv6 (dual-stack) for better connectivity.
  • Bug Fixes

    • Improved handling of IPv4/IPv6 packets to reduce dropped traffic.
    • MTU adjusted for greater network stability, especially over IPv6.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 13, 2025

Walkthrough

Adds an optional in-process HTTP CONNECT proxy to the dev server via a new CLI flag (--proxy-port). Wires ConnectProxyPort through gravity.Config, starts/stops the proxy, extends netstack to handle IPv4+IPv6, and updates inbound packet processing to detect and route by IP version.

Changes

Cohort / File(s) Summary
CLI flag & config plumbing
cmd/dev.go
Adds --proxy-port flag (default 19081). Validates value and wires ConnectProxyPort *uint into gravity.Config.
Gravity core: proxy, netstack & lifecycle
internal/gravity/gravity.go
Adds ConnectProxyPort *uint to Config and connectProxyPort *uint, connectProxy *http.Server to Client. Implements startConnectProxy and handleConnect, starts/stops internal CONNECT proxy, extends netstack to IPv4+IPv6, adjusts MTU and routing, and updates cleanup to shut down the proxy.
Provider packet handling
internal/gravity/provider.go
Rewrites ProcessInPacket to validate payload, detect IP version via header.IPVersion, choose protocol (ipv4/ipv6), construct PacketBuffer correctly, inject packet with appropriate network protocol, and ensure buffer cleanup.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor DevUser as Dev CLI User
  participant DevCmd as dev command
  participant Gravity as gravity.Client
  participant Net as Netstack (IPv4/IPv6)
  participant Proxy as CONNECT Proxy
  participant Tunnel as Gravity Tunnel
  participant Remote as Remote Host

  DevUser->>DevCmd: run `dev --proxy-port=19081`
  DevCmd->>Gravity: New(Config{ConnectProxyPort})
  DevCmd->>Gravity: Start()
  Gravity->>Net: Initialize IPv4 + IPv6, routing, MTU 1280
  Gravity->>Proxy: startConnectProxy(port)
  Note right of Proxy: CONNECT proxy listening

  rect rgb(235,245,255)
    participant ClientApp as Local App
    ClientApp->>Proxy: HTTP CONNECT target:port
    alt target routed via tunnel
      Proxy->>Tunnel: Dial via gravity tunnel
      Tunnel-->>Proxy: Connection established
    else direct target
      Proxy->>Remote: TCP dial target:port (direct)
      Remote-->>Proxy: Connection established
    end
    Proxy-->>ClientApp: 200 Connection Established
    Note over Proxy,ClientApp: Bidirectional data relay (proxy)
  end

  Note over Gravity: On shutdown, stop proxy and cleanup
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

Thump, I found a tiny port to mind,
A tunnel stitched for hosts, both v4 and v6 aligned.
I sniff the version, hop the right way through,
CONNECTs established — carrots for the crew.
— a rabbit, proxy-watching, nibbling configs true 🥕

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly describes the primary change by indicating that a development mode proxy is being created using Gravity, which directly reflects the addition of HTTP CONNECT proxy support in dev mode through the Gravity tunnel.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch proxy

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a591b5d and ff00ee7.

📒 Files selected for processing (3)
  • cmd/dev.go (2 hunks)
  • internal/gravity/gravity.go (7 hunks)
  • internal/gravity/provider.go (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • cmd/dev.go
🧰 Additional context used
🧬 Code graph analysis (1)
internal/gravity/gravity.go (2)
internal/project/project.go (1)
  • ProjectContext (379-388)
internal/dev/server.go (2)
  • Server (11-15)
  • New (95-102)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build and Test (macos-latest)
  • GitHub Check: Test Install Script (blacksmith-4vcpu-ubuntu-2204)
  • GitHub Check: Analyze (go)
🔇 Additional comments (9)
internal/gravity/provider.go (2)

10-13: LGTM!

The new imports are appropriate for the IP version detection and dual-stack packet handling implemented below.


95-127: LGTM! Past review concern addressed.

The implementation correctly validates packet length before processing, detects IP version, and uses proper memory management with the Payload API and DecRef. This addresses the previous review comment requesting header length guards.

internal/gravity/gravity.go (7)

25-25: LGTM! Past review concern addressed.

The duplicate import issue has been resolved; only the cnet alias remains. The ipv4 import is needed for dual-stack networking support.

Also applies to: 31-31


40-40: LGTM!

Setting MTU to 1280 (IPv6 minimum per RFC 8200) is a conservative choice that prevents MTU path discovery issues in dual-stack environments.


55-55: LGTM!

The new fields for CONNECT proxy support are appropriately typed and initialized. Using *uint for the port makes it clear when the feature is disabled (nil).

Also applies to: 61-61, 80-80, 101-101


170-329: LGTM! Past review concern addressed.

The CONNECT handler correctly:

  • Guards against nil IP addresses with explicit checks (lines 213-217, 221-228)
  • Supports both IPv4 and IPv6 routing through the gravity tunnel
  • Implements proper TCP half-close with CloseWrite
  • Performs bidirectional copying with appropriate cleanup

This addresses the previous review comment requesting nil guards for address map lookups.


386-393: LGTM!

The CONNECT proxy shutdown is properly implemented with timeout and error preservation.


660-660: LGTM!

The dual-stack networking configuration correctly:

  • Registers both IPv4 and IPv6 protocol handlers
  • Configures appropriate addresses with standard prefix lengths
  • Sets up separate default routes for each address family

Also applies to: 672-724


829-832: LGTM!

The CONNECT proxy is started at the appropriate time (after provider connection) with proper error handling.


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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (4)
internal/gravity/gravity.go (4)

198-206: Normalize domain patterns to avoid incorrect equality checks

Mixing dot-prefixed and bare domains makes host == domain[1:] unsafe when domain doesn’t start with a dot.

Apply this diff:

-	agentuityDomains := []string{".agentuity.io", ".agentuity.cloud", ".agentuity.run", ".agentuity.com", "agentuity.ai"}
+	agentuityDomains := []string{".agentuity.io", ".agentuity.cloud", ".agentuity.run", ".agentuity.com", ".agentuity.ai"}
 	isAgentuityDomain := false
 	for _, domain := range agentuityDomains {
-		if strings.HasSuffix(host, domain) || host == domain[1:] {
+		d := strings.TrimPrefix(domain, ".")
+		if strings.HasSuffix(host, domain) || host == d {
 			isAgentuityDomain = true
 			break
 		}
 	}

258-263: Add timeouts to direct outbound dials

Using net.Dial without deadlines can hang indefinitely. Use a Dialer with reasonable timeouts.

Apply this diff:

-		remoteConn, err = net.Dial("tcp", net.JoinHostPort(host, portStr))
+		dialer := net.Dialer{Timeout: 10 * time.Second, KeepAlive: 30 * time.Second}
+		remoteConn, err = dialer.Dial("tcp", net.JoinHostPort(host, portStr))

338-355: Bind proxy server lifecycle to context for cleaner shutdowns

Optional: set BaseContext and ConnContext so connections inherit c.context and cancel promptly on shutdown.

Example:

 server := &http.Server{
   Addr: fmt.Sprintf("127.0.0.1:%d", port),
   Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
     c.handleConnect(w, r)
   }),
   ReadTimeout:  0,
   WriteTimeout: 0,
+  BaseContext: func(net.Listener) context.Context { return c.context },
 }

746-753: Optional: avoid package shadowing with local variable name network

The local var network networkProvider shadows the imported network package elsewhere in the file. While scoped and not a compile error, renaming improves clarity.

Example:

-	var network networkProvider
+	var netProv networkProvider
-	NetworkInterface:  &network,
+	NetworkInterface:  &netProv,
-	network.client = client
+	netProv.client = client
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f22bf44 and d7db2da.

📒 Files selected for processing (3)
  • cmd/dev.go (2 hunks)
  • internal/gravity/gravity.go (7 hunks)
  • internal/gravity/provider.go (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
cmd/*.go

📄 CodeRabbit inference engine (.cursor/rules/code-generation.mdc)

Ensure CLI commands build and test the pipeline (generation → SDK loading → agent usage) without writing into source SDK

Files:

  • cmd/dev.go
🧬 Code graph analysis (2)
cmd/dev.go (2)
internal/gravity/gravity.go (2)
  • New (88-107)
  • Config (70-86)
internal/dev/server.go (2)
  • New (95-102)
  • ServerArgs (17-22)
internal/gravity/gravity.go (2)
internal/project/project.go (1)
  • ProjectContext (379-388)
internal/dev/server.go (2)
  • Server (11-15)
  • New (95-102)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build and Test (macos-latest)
  • GitHub Check: Analyze (go)
🔇 Additional comments (2)
internal/gravity/provider.go (1)

113-119: PacketBuffer construction and DecRef look correct

Using Payload with a defensive copy and deferring DecRef is the right pattern for channel.Endpoint.InjectInbound.

cmd/dev.go (1)

141-155: Wiring ConnectProxyPort through gravity.Config looks good

Pointer semantics (nil disables, >0 enables) are handled correctly.

Comment on lines 129 to 135
connectProxyPort, _ := cmd.Flags().GetInt("proxy-port")
var connectProxyPortPtr *uint
if connectProxyPort > 0 {
port := uint(connectProxyPort)
connectProxyPortPtr = &port
}

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Default enables proxy despite “disabled if zero” description (and PR states default disabled)

Current default 19081 enables the CONNECT proxy by default. Either set default to 0 (and keep description), or update description/PR to reflect enabled-by-default. Also validate port range.

Apply this diff to make it disabled by default and validate:

-	connectProxyPort, _ := cmd.Flags().GetInt("proxy-port")
+	connectProxyPort, _ := cmd.Flags().GetInt("proxy-port")
+	if connectProxyPort < 0 || connectProxyPort > 65535 {
+		log.Fatal("invalid --proxy-port: %d (must be 0-65535)", connectProxyPort)
+	}
 	var connectProxyPortPtr *uint
 	if connectProxyPort > 0 {
 		port := uint(connectProxyPort)
 		connectProxyPortPtr = &port
 	}
-	devCmd.Flags().Int("proxy-port", 19081, "The port to run the HTTP CONNECT proxy server on (disabled if zero)")
+	devCmd.Flags().Int("proxy-port", 0, "The port to run the HTTP CONNECT proxy server on (disabled if zero)")

Also applies to: 319-319

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
internal/gravity/provider.go (1)

95-121: Validate minimum header length for IPv4/IPv6 before inject

Guard only checks len>=1. Use protocol-specific minimums to prevent malformed packet injection.

Apply this diff:

-	if len(payload) < 1 {
-		return
-	}
+	if len(payload) < 1 {
+		return
+	}
 	// Detect IP version from the packet header
 	version := header.IPVersion(payload)
 	var protocol tcpip.NetworkProtocolNumber

 	switch version {
 	case 4:
 		protocol = ipv4.ProtocolNumber
+		if len(payload) < header.IPv4MinimumSize {
+			p.logger.Trace("dropping IPv4 packet: too short (%d bytes)", len(payload))
+			return
+		}
 	case 6:
 		protocol = ipv6.ProtocolNumber
+		if len(payload) < header.IPv6MinimumSize {
+			p.logger.Trace("dropping IPv6 packet: too short (%d bytes)", len(payload))
+			return
+		}
 	default:
🧹 Nitpick comments (2)
cmd/dev.go (1)

129-136: Enabling logic looks fine; minor duplication of “starting …” log

Pointer gating on port > 0 is correct. Note there will be duplicate “starting CONNECT proxy …” logs here and in gravity.Start; not harmful.

internal/gravity/gravity.go (1)

171-325: Normalize domain matching and use cnet.Addresses consistently

  • Replace the mixed leading-dot and bare entries with a uniform list of leading-dot domains (e.g. add “.agentuity.ai”) and match via
    for _, d := range domains { if host == d[1:] || strings.HasSuffix(host, d) { … } }
  • Swap all network.Addresses[...] usages to cnet.Addresses[...]
  • Verify that including “agentuity.ai” is intentional
-	agentuityDomains := []string{".agentuity.io", ".agentuity.cloud", ".agentuity.run", ".agentuity.com"}
+	agentuityDomains := []string{".agentuity.io", ".agentuity.cloud", ".agentuity.run", ".agentuity.com", ".agentuity.ai"}
 	isAgentuityDomain := false
 	for _, d := range agentuityDomains {
-		if strings.HasSuffix(host, d) || host == d[1:] {
+		if host == d[1:] || strings.HasSuffix(host, d) {
 			isAgentuityDomain = true
 			break
 		}
 	}
@@
-		ip := network.Addresses["catalyst"]
+		ip := cnet.Addresses["catalyst"]
@@
-			if customip, ok := network.Addresses[part]; ok {
+			if customip, ok := cnet.Addresses[part]; ok {
@@
-			if customip, ok := network.Addresses[part]; ok {
+			if customip, ok := cnet.Addresses[part]; ok {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f22bf44 and a591b5d.

📒 Files selected for processing (3)
  • cmd/dev.go (2 hunks)
  • internal/gravity/gravity.go (7 hunks)
  • internal/gravity/provider.go (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
cmd/*.go

📄 CodeRabbit inference engine (.cursor/rules/code-generation.mdc)

Ensure CLI commands build and test the pipeline (generation → SDK loading → agent usage) without writing into source SDK

Files:

  • cmd/dev.go
🧬 Code graph analysis (2)
internal/gravity/gravity.go (1)
internal/project/project.go (1)
  • ProjectContext (379-388)
cmd/dev.go (2)
internal/gravity/gravity.go (2)
  • New (88-107)
  • Config (70-86)
internal/dev/server.go (2)
  • New (95-102)
  • ServerArgs (17-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build and Test (macos-latest)
  • GitHub Check: Analyze (go)
🔇 Additional comments (12)
cmd/dev.go (1)

142-156: Good propagation of ConnectProxyPort into gravity.Config

Config plumb-through is correct and matches internal/gravity changes.

internal/gravity/provider.go (1)

10-14: Imports align with new IP-version-aware handling

New tcpip/header/ipv4 imports are appropriate.

internal/gravity/gravity.go (10)

41-41: MTU reduction to 1280 is appropriate for IPv6 and avoids PMTUD blackholes

Good call.


45-68: Client struct additions look correct

Fields for connectProxyPort and connectProxy are well-scoped; no behavioral issues.


71-86: Config additions look correct

ConnectProxyPort is optional via pointer; fits CLI behavior.


90-106: New() wiring is consistent

All new fields are initialized properly.


382-389: Graceful shutdown of connect proxy is correct

Shutdown with timeout is appropriate; nil the field after.


656-656: Dual-stack network protocols enabled

ipv4 + ipv6 protocol registration is correct.


668-683: IPv6 address creation is correct; minor nit on error message

Address addition looks good. The error message says “create IPv6 protocol address” (ok). No action needed.


685-701: IPv4 address configuration looks correct

Proper AddrFrom4 usage with /24 prefix.


703-720: Default routes for v4 and v6 are reasonable

Creating 0/0 subnets for both families and setting route table is correct for this virtual NIC.


825-829: CONNECT proxy startup hook is placed correctly

Starting the proxy after provider connection is established makes sense.

rootCmd.AddCommand(devCmd)
devCmd.Flags().StringP("dir", "d", ".", "The directory to run the development server in")
devCmd.Flags().Int("port", 0, "The port to run the development server on (uses project default if not provided)")
devCmd.Flags().Int("proxy-port", 19081, "The port to run the HTTP CONNECT proxy server on (disabled if zero)")
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Default should be disabled to match PR description and avoid unexpected proxy start

The flag help says “disabled if zero,” but default 19081 enables it by default. Consider defaulting to 0.

Apply this diff:

-	devCmd.Flags().Int("proxy-port", 19081, "The port to run the HTTP CONNECT proxy server on (disabled if zero)")
+	devCmd.Flags().Int("proxy-port", 0, "The port to run the HTTP CONNECT proxy server on (disabled if zero)")
📝 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
devCmd.Flags().Int("proxy-port", 19081, "The port to run the HTTP CONNECT proxy server on (disabled if zero)")
devCmd.Flags().Int("proxy-port", 0, "The port to run the HTTP CONNECT proxy server on (disabled if zero)")
🤖 Prompt for AI Agents
In cmd/dev.go around line 320, the proxy-port flag currently defaults to 19081
which contradicts the help text saying "disabled if zero"; change the flag's
default value from 19081 to 0 so the proxy is disabled by default, leaving the
flag name, type, and help string unchanged.

Comment on lines 327 to 356
// startConnectProxy starts the HTTP CONNECT proxy server if configured.
func (c *Client) startConnectProxy() error {
if c.connectProxyPort == nil {
return nil
}

logger := c.logger
port := *c.connectProxyPort

logger.Debug("starting CONNECT proxy on port %d", port)

server := &http.Server{
Addr: fmt.Sprintf("127.0.0.1:%d", port),
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
c.handleConnect(w, r)
}),
ReadTimeout: 0,
WriteTimeout: 0,
}
c.connectProxy = server

go func() {
if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed {
logger.Fatal("CONNECT proxy server failed: %v", err)
}
}()

logger.Info("CONNECT proxy listening on http://127.0.0.1:%d", port)
return nil
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Pre-bind the listener to surface port conflicts synchronously (avoid Fatal in goroutine)

Currently errors in ListenAndServe are logged as Fatal inside a goroutine. Prefer binding first and returning an error to caller.

Apply this diff:

-	server := &http.Server{
+	server := &http.Server{
 		Addr: fmt.Sprintf("127.0.0.1:%d", port),
 		Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			c.handleConnect(w, r)
 		}),
 		ReadTimeout:  0,
 		WriteTimeout: 0,
 	}
 	c.connectProxy = server

-	go func() {
-		if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed {
-			logger.Fatal("CONNECT proxy server failed: %v", err)
-		}
-	}()
-
-	logger.Info("CONNECT proxy listening on http://127.0.0.1:%d", port)
-	return nil
+	ln, err := net.Listen("tcp", server.Addr)
+	if err != nil {
+		return fmt.Errorf("CONNECT proxy listen failed on %s: %w", server.Addr, err)
+	}
+	logger.Info("CONNECT proxy listening on http://%s", server.Addr)
+	go func() {
+		_ = server.Serve(ln) // error handled via Shutdown or context
+	}()
+	return nil
📝 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
// startConnectProxy starts the HTTP CONNECT proxy server if configured.
func (c *Client) startConnectProxy() error {
if c.connectProxyPort == nil {
return nil
}
logger := c.logger
port := *c.connectProxyPort
logger.Debug("starting CONNECT proxy on port %d", port)
server := &http.Server{
Addr: fmt.Sprintf("127.0.0.1:%d", port),
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
c.handleConnect(w, r)
}),
ReadTimeout: 0,
WriteTimeout: 0,
}
c.connectProxy = server
go func() {
if err := server.ListenAndServe(); err != nil && err != http.ErrServerClosed {
logger.Fatal("CONNECT proxy server failed: %v", err)
}
}()
logger.Info("CONNECT proxy listening on http://127.0.0.1:%d", port)
return nil
}
func (c *Client) startConnectProxy() error {
if c.connectProxyPort == nil {
return nil
}
logger := c.logger
port := *c.connectProxyPort
logger.Debug("starting CONNECT proxy on port %d", port)
server := &http.Server{
Addr: fmt.Sprintf("127.0.0.1:%d", port),
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
c.handleConnect(w, r)
}),
ReadTimeout: 0,
WriteTimeout: 0,
}
c.connectProxy = server
ln, err := net.Listen("tcp", server.Addr)
if err != nil {
return fmt.Errorf("CONNECT proxy listen failed on %s: %w", server.Addr, err)
}
logger.Info("CONNECT proxy listening on http://%s", server.Addr)
go func() {
_ = server.Serve(ln) // error handled via Shutdown or context
}()
return nil
}
🤖 Prompt for AI Agents
internal/gravity/gravity.go around lines 327 to 356: pre-bind the TCP listener
synchronously to detect port conflicts and return an error instead of calling
logger.Fatal inside the goroutine; do this by creating the address string, call
net.Listen("tcp", addr) and if that returns an error return it to the caller,
then assign the listener to the server (or pass the listener to server.Serve),
start a goroutine that calls server.Serve(listener) and only log/handle Serve
errors there (no logger.Fatal from the goroutine), and keep the info log after
successful bind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants