WIP: session: Ignore DetachFromTarget error if target is gone (mafred…

…ri#110)

rpcc.Conn.Close calls Target.DetachFromTarget before closing the
websocket connection, which fails if the target is already closed.
This is a bit inconvenient because we have to ignore errors from
rpcc.Conn.Close when we close the target in advance, which means that
we might miss other interesting errors.

This change updates the detach function to ignore errors on
Target.DetachFromTarget calls if it is because the target is already
gone.

Also adds a unit test to test this behavior.

FIXME: The unit test still does not pass.

session/session.go

@@ -123,6 +123,10 @@ func dial(ctx context.Context, id target.ID, tc *cdp.Client, detachTimeout time.
 		if err == context.DeadlineExceeded {
 			return fmt.Errorf("session: detach timed out for session %s", s.ID)
 		}
+		// Ignore "No session with given id" error which might be returned if the target is already closed.
+		if rerr, ok := err.(*rpcc.ResponseError); ok && rerr.Code == -32602 {
+			return nil
+		}
 		return errors.Wrapf(err, "session: detach failed for session %s", s.ID)
 	}
 

session/session_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/mafredri/cdp/internal/testutil"
 	"github.com/mafredri/cdp/protocol/page"
 	"github.com/mafredri/cdp/protocol/runtime"
+	"github.com/mafredri/cdp/protocol/target"
 	"github.com/mafredri/cdp/session"
 )
 
@@ -194,6 +195,40 @@ func TestManager_NewOnClosedConn(t *testing.T) {
 	}
 }
 
+func TestManager_CloseTarget(t *testing.T) {
+	checkBrowser(t)
+
+	ctx, cancel := context.WithTimeout(context.TODO(), 10*time.Second)
+	defer cancel()
+
+	c := testutil.NewClient(ctx, t)
+	defer c.Conn.Close()
+
+	m, err := session.NewManager(c.Client)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer m.Close()
+
+	p := c.NewPage(ctx)
+
+	conn, err := m.Dial(ctx, p.ID())
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() {
+		// Even if the target is already closed, Conn.Close should not return an error.
+		if err := conn.Close(); err != nil {
+			t.Error(err)
+		}
+	}()
+
+	if _, err := c.Client.Target.CloseTarget(ctx, target.NewCloseTargetArgs(p.ID())); err != nil {
+		t.Fatal(err)
+	}
+	time.Sleep(10 * time.Millisecond) // Give time to close the target.
+}
+
 var (
 	browserFlag = flag.Bool("browser", false, "Test with browser")
 )