Skip to content

Commit

Permalink
stop retrying for invalid selectors (such as "#a:b")
Browse files Browse the repository at this point in the history
It does not make sense to retry the query for invalid selector
because it will always fail.

But the CDP hides the detail error for invalid selector, and only
gives us "DOM Error while querying". We will stop retrying when
we see this error message.
  • Loading branch information
ZekeLu committed Aug 5, 2023
1 parent 3454942 commit 852a0b0
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 1 deletion.
16 changes: 15 additions & 1 deletion query.go
Expand Up @@ -9,6 +9,7 @@ import (
"sync"
"time"

"github.com/chromedp/cdproto"
"github.com/chromedp/cdproto/cdp"
"github.com/chromedp/cdproto/css"
"github.com/chromedp/cdproto/dom"
Expand Down Expand Up @@ -189,7 +190,20 @@ func (s *Selector) Do(ctx context.Context) error {
}

ids, err := s.by(ctx, fromNode)
if err != nil || len(ids) < s.exp {
if err != nil {
var e *cdproto.Error
// When the selector is invalid (for example, "#a:b" or "#3"), it will
// always fail with "DOM Error while querying". It does not make sense
// to retry in this case.
// Maybe "DOM Error while querying" is also used for other errors other
// than invalid selector. But the response does not contain anything
// else that can be used to distinguish them. So we have to go with it.
if errors.As(err, &e) && e.Message == "DOM Error while querying" {
return true, err
}
return false, nil
}
if len(ids) < s.exp {
return false, nil
}
nodes, err := s.wait(ctx, frame, execCtx, ids...)
Expand Down
30 changes: 30 additions & 0 deletions query_test.go
Expand Up @@ -251,6 +251,36 @@ func TestRetryInterval(t *testing.T) {
}
}

func TestNoRetryForInvalidSelector(t *testing.T) {
t.Parallel()

ctx, cancel := testAllocate(t, "table.html")
defer cancel()

ctx, cancel = context.WithTimeout(ctx, time.Second)
defer cancel()

tests := []struct {
name string
sel string
by QueryOption
wantErr string
}{
{`pseudo class`, `#a:b`, ByQuery, "DOM Error while querying (-32000)"},
{`leading number`, `#3`, ByQuery, "DOM Error while querying (-32000)"},
{`empty selector`, ``, ByQuery, "DOM Error while querying (-32000)"},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
var nodes []*cdp.Node
if err := Run(ctx, Nodes(test.sel, &nodes, test.by)); err.Error() != test.wantErr {
t.Fatalf("want error %v, got error: %v", test.wantErr, err)
}
})
}
}

func TestByJSPath(t *testing.T) {
t.Parallel()

Expand Down

0 comments on commit 852a0b0

Please sign in to comment.