fix(tsconnect): address bridge review findings

Co-Authored-By: gpt-5.6-sol <noreply@openai.com>

cmd/tsconnect/wasm/bridge_test.go

@@ -42,10 +42,19 @@ func TestDriveHandlerThrowReturns500(t *testing.T) {
 }
 
 func TestDriveCancellationDisablesCallbacks(t *testing.T) {
-	started := make(chan struct{})
+	type handlerArgs struct {
+		req js.Value
+		res js.Value
+	}
+	started := make(chan handlerArgs, 1)
+	deferred := evalJS(t, `(() => {
+		let resolve
+		const promise = new Promise(r => { resolve = r })
+		return {promise, resolve}
+	})()`)
 	handler := js.FuncOf(func(this js.Value, args []js.Value) any {
-		close(started)
+		started <- handlerArgs{req: args[0], res: args[1]}
-		return nil
+		return deferred.Get("promise")
 	})
 	defer handler.Release()
 	fs := new(jsFileSystemForRemote)
@@ -59,8 +68,9 @@ func TestDriveCancellationDisablesCallbacks(t *testing.T) {
 		close(done)
 	}()
 
+	var jsArgs handlerArgs
 	select {
-	case <-started:
+	case jsArgs = <-started:
 	case <-time.After(time.Second):
 		t.Fatal("drive handler did not start")
 	}
@@ -71,6 +81,21 @@ func TestDriveCancellationDisablesCallbacks(t *testing.T) {
 		t.Fatal("drive handler did not stop after cancellation")
 	}
 
+	readResult := jsArgs.req.Call("readBodyChunk")
+	if readResult.Type() != js.TypeObject {
+		t.Fatalf("readBodyChunk after cancellation returned %s, want Promise", readResult.Type())
+	}
+	if result := awaitFulfilledPromise(t, readResult); !result.IsNull() {
+		t.Fatalf("readBodyChunk after cancellation = %v, want null", result)
+	}
+	lateChunk := js.Global().Get("Uint8Array").New(1)
+	js.CopyBytesToJS(lateChunk, []byte("x"))
+	jsArgs.res.Call("write", lateChunk)
+	if recorder.Body.Len() != 0 {
+		t.Fatalf("late JS write changed response body to %q", recorder.Body.String())
+	}
+	deferred.Call("resolve")
+
 	writeStarted := make(chan struct{})
 	allowWrite := make(chan struct{})
 	response := &driveResponse{
@@ -106,6 +131,23 @@ func TestDriveCancellationDisablesCallbacks(t *testing.T) {
 	}
 }
 
+func TestDriveHandlerFulfillmentCompletesRequest(t *testing.T) {
+	handler := evalJS(t, `(function() { return Promise.resolve() })`)
+	fs := new(jsFileSystemForRemote)
+	fs.setHandler(handler)
+	recorder := httptest.NewRecorder()
+	done := make(chan struct{})
+	go func() {
+		fs.ServeHTTPWithPerms(nil, recorder, httptest.NewRequest("GET", "/", nil))
+		close(done)
+	}()
+	select {
+	case <-done:
+	case <-time.After(time.Second):
+		t.Fatal("fulfilled drive handler did not complete request")
+	}
+}
+
 func TestJSStreamReaderErrorsAreSticky(t *testing.T) {
 	for _, test := range []struct {
 		name   string
@@ -142,6 +184,23 @@ func TestJSStreamReaderCloseErrors(t *testing.T) {
 	}
 }
 
+func TestJSStreamReaderCloseTimesOut(t *testing.T) {
+	r := &jsStreamReader{
+		reader:            evalJS(t, `({cancel() { return new Promise(() => {}) }})`),
+		testCancelTimeout: 10 * time.Millisecond,
+	}
+	started := time.Now()
+	if err := r.Close(); err == nil || !strings.Contains(err.Error(), "timed out") {
+		t.Fatalf("Close error = %v, want timeout", err)
+	}
+	if elapsed := time.Since(started); elapsed >= 250*time.Millisecond {
+		t.Fatalf("Close took %v, want less than 250ms", elapsed)
+	}
+	if _, err := r.Read(make([]byte, 1)); err == nil || !strings.Contains(err.Error(), "timed out") {
+		t.Fatalf("Read error after Close timeout = %v, want sticky timeout", err)
+	}
+}
+
 func TestJSStreamReaderIgnoresDuplicateSettlement(t *testing.T) {
 	reader := evalJS(t, `({
 		read() {
@@ -233,3 +292,35 @@ func awaitPromise(t *testing.T, promise js.Value) js.Value {
 		return js.Undefined()
 	}
 }
+
+func awaitFulfilledPromise(t *testing.T, promise js.Value) js.Value {
+	t.Helper()
+	type result struct {
+		value    js.Value
+		rejected bool
+	}
+	ch := make(chan result, 1)
+	resolve := js.FuncOf(func(this js.Value, args []js.Value) any {
+		ch <- result{value: args[0]}
+		return nil
+	})
+	reject := js.FuncOf(func(this js.Value, args []js.Value) any {
+		ch <- result{value: args[0], rejected: true}
+		return nil
+	})
+	defer resolve.Release()
+	defer reject.Release()
+	if _, err := callJSMethod(promise, "then", resolve, reject); err != nil {
+		t.Fatal(err)
+	}
+	select {
+	case result := <-ch:
+		if result.rejected {
+			t.Fatalf("promise rejected with %v; want fulfillment", result.value)
+		}
+		return result.value
+	case <-time.After(time.Second):
+		t.Fatal("promise did not settle")
+		return js.Undefined()
+	}
+}

cmd/tsconnect/wasm/drive.go

@@ -138,7 +138,7 @@ func (fs *jsFileSystemForRemote) ServeHTTPWithPerms(
 	readBodyChunk := js.FuncOf(func(_ js.Value, _ []js.Value) any {
 		if !response.isLive() {
 			return makePromise(func() (any, error) {
-				return nil, errors.New("drive request is closed")
+				return js.Null(), nil
 			})
 		}
 		return makePromise(func() (any, error) {
@@ -176,26 +176,35 @@ func (fs *jsFileSystemForRemote) ServeHTTPWithPerms(
 		}
 		return nil
 	})
-	rejected := js.FuncOf(func(_ js.Value, args []js.Value) any {
+	var fulfilled, rejected js.Func
+	var handlerSettled sync.Once
+	settleHandler := func(err error) {
+		handlerSettled.Do(func() {
+			select {
+			case response.doneCh <- err:
+			default:
+			}
+			readBodyChunk.Release()
+			writeHead.Release()
+			write.Release()
+			end.Release()
+			fulfilled.Release()
+			rejected.Release()
+		})
+	}
+	fulfilled = js.FuncOf(func(_ js.Value, _ []js.Value) any {
+		settleHandler(nil)
+		return nil
+	})
+	rejected = js.FuncOf(func(_ js.Value, args []js.Value) any {
 		err := errors.New("JavaScript drive handler rejected")
 		if len(args) > 0 && args[0].Type() == js.TypeString {
 			err = fmt.Errorf("JavaScript drive handler rejected: %s", args[0].String())
 		}
-		select {
+		settleHandler(err)
-		case response.doneCh <- err:
-		default:
-		}
 		return nil
 	})
 
-	defer func() {
-		readBodyChunk.Release()
-		writeHead.Release()
-		write.Release()
-		end.Release()
-		rejected.Release()
-	}()
-
 	jsReq := map[string]any{
 		"method":        r.Method,
 		"path":          r.URL.Path,
@@ -214,14 +223,13 @@ func (fs *jsFileSystemForRemote) ServeHTTPWithPerms(
 		if hasThen, err := hasJSFunctionProperty(result, "then"); err != nil {
 			handlerErr = err
 		} else if hasThen {
-			_, handlerErr = callJSMethod(result, "then", js.Undefined(), rejected)
+			_, handlerErr = callJSMethod(result, "then", fulfilled, rejected)
+		} else {
+			settleHandler(nil)
 		}
 	}
 	if handlerErr != nil {
-		select {
+		settleHandler(handlerErr)
-		case response.doneCh <- handlerErr:
-		default:
-		}
 	}
 
 	var resultErr error

cmd/tsconnect/wasm/taildrop.go

@@ -246,13 +246,16 @@ func wireTaildropFileOps(lb *ipnlocal.LocalBackend, jsObj js.Value) {
 // ReadableStreamDefaultReader. Each Read call awaits one reader.read() Promise,
 // using the channel+FuncOf pattern so Go blocks until JS delivers the chunk.
 type jsStreamReader struct {
-	mu     sync.Mutex
+	mu                sync.Mutex
-	reader js.Value
+	reader            js.Value
-	buf    []byte
+	buf               []byte
-	done   bool
+	done              bool
-	err    error
+	err               error
+	testCancelTimeout time.Duration
 }
 
+const jsStreamCancelTimeout = time.Second
+
 func (r *jsStreamReader) Read(p []byte) (int, error) {
 	r.mu.Lock()
 	if r.err != nil {
@@ -386,8 +389,19 @@ func (r *jsStreamReader) Close() error {
 	if _, err := callJSMethod(promise, "then", resolveFn, rejectFn); err != nil {
 		return r.setError(err)
 	}
-	if err := <-resultCh; err != nil {
+	timeout := jsStreamCancelTimeout
-		return r.setError(err)
+	if r.testCancelTimeout > 0 {
+		timeout = r.testCancelTimeout
+	}
+	timer := time.NewTimer(timeout)
+	defer timer.Stop()
+	select {
+	case err := <-resultCh:
+		if err != nil {
+			return r.setError(err)
+		}
+	case <-timer.C:
+		return r.setError(errors.New("JavaScript stream cancel timed out"))
 	}
 	r.mu.Lock()
 	r.done = true