Merge pull request #50 from wirepair/ready_ch_pass_err

Ready ch pass err

.github/workflows/build_gcd_v2.yml

@@ -0,0 +1,19 @@
+on: [push, pull_request]
+name: Test
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+    - name: Install Go
+      uses: actions/setup-go@v2
+      with:
+        go-version: 1.16.x
+    - name: Install Packages
+      run: |
+        sudo apt-get -qq update
+        sudo apt-get install -y build-essential chromium-browser
+    - name: Checkout code
+      uses: actions/checkout@v2
+    - name: Test
+      run: |
+        cd v2 && go test -v -race ./...

CHANGELOG.md

@@ -1,4 +1,5 @@
 # Changelog (2021)
+- 2.1.5 (June 8th) Fix race condition on error and endpoint
 - 2.1.4 (June 8th) Added a chrome exit handler from @camswords.
 - 2.1.3 (June 4th): Fix a potential blocked channel if chrome fails to start and debug port probe fails
 - 2.1.2 (May 23rd): Update to chrome version 90.0.4430.212

v2/gcd.go

@@ -44,7 +44,7 @@ import (
 
 var json = jsoniter.ConfigCompatibleWithStandardLibrary
 
-var GCDVERSION = "v2.1.4"
+var GCDVERSION = "v2.1.5"
 
 var (
 	ErrNoTabAvailable = errors.New("no available tab found")
@@ -85,7 +85,7 @@ type Gcd struct {
 	addr                string
 	profileDir          string
 	deleteProfile       bool
-	readyCh             chan struct{}
+	readyChErr          chan error
 	apiEndpoint         string
 	flags               []string
 	env                 []string
@@ -100,9 +100,9 @@ type Gcd struct {
 // Give it a friendly name.
 func NewChromeDebugger(opts ...func(*Gcd)) *Gcd {
 	c := &Gcd{processLock: &sync.RWMutex{}}
-	c.timeout = 15
+	c.timeout = time.Second * 15
 	c.host = "localhost"
-	c.readyCh = make(chan struct{})
+	c.readyChErr = make(chan error)
 	c.terminatedHandler = nil
 	c.onChromeExitHandler = nil
 	c.flags = make([]string, 0)
@@ -267,11 +267,10 @@ func (c *Gcd) startProcess() error {
 		}
 	}()
 
-	var err error
-	go func() {
-		err = c.probeDebugPort()
-	}()
-	<-c.readyCh
+	go func(endpoint string) {
+		c.probeDebugPort(endpoint)
+	}(c.apiEndpoint)
+	err := <-c.readyChErr
 
 	return err
 }
@@ -295,8 +294,7 @@ func (c *Gcd) PID() int {
 // removeProfileDir if deleteProfile is true
 func (c *Gcd) removeProfileDir() {
 	if c.deleteProfile {
-		// let chrome shutdown completely before deleting
-		time.Sleep(1 * time.Second)
+		time.Sleep(time.Second * 1)
 		if err := os.RemoveAll(c.profileDir); err != nil {
 			c.logger.Println("error deleting profile directory", err)
 		}
@@ -312,11 +310,10 @@ func (c *Gcd) ConnectToInstance(host string, port string) error {
 	c.addr = fmt.Sprintf("%s:%s", c.host, c.port)
 	c.apiEndpoint = fmt.Sprintf("http://%s/json", c.addr)
 
-	var err error
-	go func() {
-		err = c.probeDebugPort()
-	}()
-	<-c.readyCh
+	go func(endpoint string) {
+		c.probeDebugPort(endpoint)
+	}(c.apiEndpoint)
+	err := <-c.readyChErr
 
 	return err
 }
@@ -450,27 +447,26 @@ func (c *Gcd) ActivateTab(target *ChromeTarget) error {
 }
 
 // probes the debugger report and signals when it's available.
-func (c *Gcd) probeDebugPort() error {
+func (c *Gcd) probeDebugPort(endpoint string) error {
 	ticker := time.NewTicker(time.Millisecond * 100)
-	timeoutTicker := time.NewTicker(time.Second * c.timeout)
+	timeoutTicker := time.NewTicker(c.timeout)
 
 	defer func() {
 		ticker.Stop()
 		timeoutTicker.Stop()
-		c.readyCh <- struct{}{}
 	}()
 
 	for {
 		select {
 		case <-ticker.C:
-			resp, err := http.Get(c.apiEndpoint)
+			resp, err := http.Get(endpoint)
 			if err != nil {
 				continue
 			}
 			defer resp.Body.Close()
-			return nil
+			c.readyChErr <- nil
 		case <-timeoutTicker.C:
-			return fmt.Errorf("Unable to contact debugger at %s after %d seconds, gave up", c.apiEndpoint, c.timeout)
+			c.readyChErr <- fmt.Errorf("Unable to contact debugger at %s after %d seconds, gave up", c.apiEndpoint, c.timeout)
 		}
 	}
 }

v2/gcd_test.go

@@ -59,16 +59,20 @@ func testCleanUp() {
 
 func TestDeleteProfileOnExit(t *testing.T) {
 	if runtime.GOOS == "windows" {
-		t.Skip("windows will hold on to the process handle too long")
+		//t.Skip("windows will hold on to the process handle too long")
 	}
-	debugger := NewChromeDebugger(WithDeleteProfileOnExit())
+
+	debugger := NewChromeDebugger(WithDeleteProfileOnExit(),
+		WithFlags([]string{"--headless"}),
+	)
 
 	profileDir := testRandomTempDir(t)
 	err := debugger.StartProcess(testPath, profileDir, testRandomPort(t))
 	if err != nil {
-		t.Fatalf("error starting chrome")
+		t.Fatalf("error starting chrome: %s\n", err)
 	}
 	debugger.ExitProcess()
+	time.Sleep(3 * time.Second)
 	if stat, err := os.Stat(profileDir); err == nil {
 		t.Fatalf("error temporary profileDir still exists: %s\n", stat.Name())
 	}
@@ -90,7 +94,7 @@ func TestGetPages(t *testing.T) {
 
 func TestEnv(t *testing.T) {
 	var ok bool
-	debugger = NewChromeDebugger(WithEnvironmentVars([]string{"hello=youze", "zoinks=scoob"}))
+	debugger = NewChromeDebugger(WithEnvironmentVars([]string{"hello=youze", "zoinks=scoob"}), WithFlags([]string{"--headless"}))
 	debugger.StartProcess(testPath, testRandomTempDir(t), testRandomPort(t))
 	defer debugger.ExitProcess()
 
@@ -578,7 +582,7 @@ func testExtensionStartup(t *testing.T) {
 
 	sep := string(os.PathSeparator)
 	extensionPath := "--load-extension=" + path + sep + "testdata" + sep + "extension" + sep
-	debugger = NewChromeDebugger(WithFlags([]string{extensionPath}))
+	debugger = NewChromeDebugger(WithFlags([]string{extensionPath, "--headless"}))
 
 	debugger.StartProcess(testPath, testRandomTempDir(t), testRandomPort(t))
 }