Merge pull request #1781 from appwrite/fix/go-cli-local-function-readiness

Fix Go CLI local function readiness checks

Author: ChiragAgg5k

templates/go-cli/internal/docker/docker.go

@@ -299,13 +299,11 @@ func (c *Client) Start(ctx context.Context, options StartOptions) (wait func() e
 	return func() error { return <-exited }, nil
 }
 
-// waitForPort polls until the runtime inside the container answers.
+// waitForPort polls until the published port accepts a connection.
 //
-// A bare TCP connect is not enough: docker's port proxy accepts connections
-// while the runtime behind it is still unpacking code, so `run` announced
-// success several seconds early. Any reply counts, including a 500 -- only a
-// connection that closes without one is not-ready-yet. 100 attempts at 100ms
-// covers a cold start without hanging a broken one forever.
+// Readiness must stop at the transport boundary. Sending an HTTP request here
+// executes the user's function before `run` has started and rejects valid
+// functions that do not answer that synthetic request.
 func waitForPort(ctx context.Context, port int) error {
 	address := net.JoinHostPort("127.0.0.1", strconv.Itoa(port))
 
@@ -317,8 +315,10 @@ func waitForPort(ctx context.Context, port int) error {
 		default:
 		}
 
-		err := probe(address)
+		connection, err := net.DialTimeout("tcp", address, time.Second)
 		if err == nil {
+			connection.Close()
+
 			return nil
 		}
 		lastErr = err
@@ -329,34 +329,6 @@ func waitForPort(ctx context.Context, port int) error {
 	return fmt.Errorf("timed out waiting for port %d: %w", port, lastErr)
 }
 
-// probe sends the smallest well-formed request that gets an answer.
-func probe(address string) error {
-	connection, err := net.DialTimeout("tcp", address, time.Second)
-	if err != nil {
-		return err
-	}
-	defer connection.Close()
-
-	if err := connection.SetDeadline(time.Now().Add(time.Second)); err != nil {
-		return err
-	}
-
-	// HTTP/1.0 so the server closes rather than holding the connection open for
-	// a keep-alive that nothing here will use.
-	if _, err := connection.Write([]byte("GET / HTTP/1.0\r\n\r\n")); err != nil {
-		return err
-	}
-
-	// One byte is the whole signal. Reading the status line would mean parsing
-	// it, and the reply's CONTENT is not what is being checked.
-	answer := make([]byte, 1)
-	if _, err := connection.Read(answer); err != nil {
-		return fmt.Errorf("no reply from the runtime: %w", err)
-	}
-
-	return nil
-}
-
 // PortAvailable reports whether a port can be published.
 //
 // Both loopback addresses are checked, because `localhost` is not one address: a

templates/go-cli/internal/docker/docker_test.go

@@ -1,6 +1,7 @@
 package docker
 
 import (
+	"context"
 	"net"
 	"os"
 	"path/filepath"
@@ -362,53 +363,43 @@ func TestFindPortSkipsAPortInUse(t *testing.T) {
 	}
 }
 
-// The readiness probe requires a REPLY, not just an accept. docker's published
-// port is proxied, and the proxy accepts while the runtime behind it is still
-// unpacking code -- which is how `run` announced its URL in the middle of the
-// container's startup log.
-func TestReadinessProbeRejectsAnAcceptWithNoReply(t *testing.T) {
+// Readiness is a transport check, not a function invocation. A function may
+// validly ignore GET /, require browser headers, stream indefinitely, or have
+// side effects. Accepting the connection is enough to prove the published port
+// is reachable.
+func TestWaitForPortDoesNotRequireAnHTTPReply(t *testing.T) {
 	listener, err := net.Listen("tcp", "127.0.0.1:0")
 	if err != nil {
 		t.Fatal(err)
 	}
 	defer listener.Close()
 
-	// Accepts and closes without answering, as a proxy with no backend does.
-	go func() {
-		for {
-			connection, err := listener.Accept()
-			if err != nil {
-				return
-			}
-			connection.Close()
-		}
-	}()
-
-	if err := probe(listener.Addr().String()); err == nil {
-		t.Error("a connection that never answered was treated as ready")
-	}
-}
-
-func TestReadinessProbeAcceptsAnyReply(t *testing.T) {
-	listener, err := net.Listen("tcp", "127.0.0.1:0")
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer listener.Close()
-
-	// 500 is what the dev server answers without the runtime secret, and it is
-	// still proof that something is listening.
+	accepted := make(chan struct{})
 	go func() {
 		connection, err := listener.Accept()
 		if err != nil {
 			return
 		}
 		defer connection.Close()
-		_, _ = connection.Write([]byte("HTTP/1.0 500 Internal Server Error\r\n\r\n"))
+		close(accepted)
+
+		// Keep the connection open without sending a reply. The old readiness
+		// probe blocked on this until its read deadline and rejected the runtime.
+		time.Sleep(time.Second)
 	}()
 
-	if err := probe(listener.Addr().String()); err != nil {
-		t.Errorf("a listening runtime was treated as not ready: %v", err)
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+	defer cancel()
+
+	port := listener.Addr().(*net.TCPAddr).Port
+	if err := waitForPort(ctx, port); err != nil {
+		t.Fatalf("an accepted connection was treated as not ready: %v", err)
+	}
+
+	select {
+	case <-accepted:
+	case <-ctx.Done():
+		t.Fatal("readiness did not connect to the published port")
 	}
 }