pool: error quickly if we find a port in use
Before the fix, the added test would give a Pool.Allocate error like: pool could not connect to 9000: timeout waiting for initial target The actual underlying error, which can only be seen if one inspects chrome's stderr, is that it failed to bind to the debugging protocol port if it was already in use. This is of course an issue with the environment that chromedp is being run under, since it was given a port range that wasn't available. However, the confusing error can lead to developers wasting their time instead of spotting the error quickly. Unfortunately, there doesn't seem to be a way to have Chrome exit immediately if it can't bind to the given port. So, instead of relying on it, check if the current process can bind to the port first. Add a test too, where we grab the first port in the pool range, and check that we get an error that's not confusing. Fixes #253.
This commit is contained in:
parent
bf52fed0d3
commit
98d4b0de6e
13
pool.go
13
pool.go
|
@ -4,6 +4,7 @@ import (
|
|||
"context"
|
||||
"fmt"
|
||||
"log"
|
||||
"net"
|
||||
"sync"
|
||||
|
||||
"github.com/chromedp/chromedp/runner"
|
||||
|
@ -70,6 +71,18 @@ func (p *Pool) Allocate(ctxt context.Context, opts ...runner.CommandLineOption)
|
|||
|
||||
r := p.next(ctxt)
|
||||
|
||||
// Check if the port is available first. If it's not, Chrome will print
|
||||
// an "address already in use" error, but it will otherwise keep
|
||||
// running. This can lead to Allocate succeeding, while the chrome
|
||||
// process isn't actually listening on the port we need.
|
||||
l, err := net.Listen("tcp", fmt.Sprintf("localhost:%d", r.port))
|
||||
if err != nil {
|
||||
// we can't use this port, e.g. address already in use
|
||||
p.errf("pool could not allocate runner on port %d: %v", r.port, err)
|
||||
return nil, err
|
||||
}
|
||||
l.Close()
|
||||
|
||||
p.debugf("pool allocating %d", r.port)
|
||||
|
||||
// create runner
|
||||
|
|
43
pool_test.go
Normal file
43
pool_test.go
Normal file
|
@ -0,0 +1,43 @@
|
|||
package chromedp
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestAllocatePortInUse(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// take a random available port
|
||||
l, err := net.Listen("tcp4", "localhost:0")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer l.Close()
|
||||
|
||||
ctxt, cancel := context.WithCancel(context.Background())
|
||||
defer cancel()
|
||||
|
||||
// make the pool use the port already in use via a port range
|
||||
_, portStr, _ := net.SplitHostPort(l.Addr().String())
|
||||
port, _ := strconv.Atoi(portStr)
|
||||
pool, err := NewPool(PortRange(port, port+1))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
c, err := pool.Allocate(ctxt)
|
||||
if err != nil {
|
||||
want := "address already in use"
|
||||
got := err.Error()
|
||||
if !strings.Contains(got, want) {
|
||||
t.Fatalf("wanted error to contain %q, but got %q", want, got)
|
||||
}
|
||||
} else {
|
||||
t.Fatal("wanted Allocate to error if port is in use")
|
||||
c.Release()
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue
Block a user