Chromedp: dom.GetDocument() returns node id -32000?

Created on 9 Jan 2020  路  4Comments  路  Source: chromedp/chromedp

What versions are you running?

$ go list -m github.com/chromedp/chromedp
github.com/chromedp/chromedp v0.5.2
$ google-chrome --version
Google Chrome 78.0.3904.108
$ go version
go version go1.12.5 darwin/amd64

What did you do? Include clear steps.

Searching on Github with lots of results:

package main

import (
  "context"
  "fmt"
  "github.com/chromedp/cdproto/dom"
  "github.com/chromedp/chromedp"
)

func main() {
  ctx, cancel :=
    chromedp.NewContext(context.Background())
  defer cancel()

  sel := `//input[@name="q"]`
  tasks := chromedp.Tasks{
    chromedp.Navigate(
      "https://github.com/search"),
    chromedp.WaitVisible(sel),
    chromedp.SendKeys(sel, "linux\n"),
    chromedp.WaitReady("body", chromedp.ByQuery),
    chromedp.ActionFunc(
      func(ctx context.Context) error {
        node, err := dom.GetDocument().Do(ctx)
        if err != nil {
          panic(err)
        }
        res, err := dom.GetOuterHTML().
          WithNodeID(node.NodeID).Do(ctx)
        if err != nil {
          panic(err)
        }
        fmt.Printf("html=%s\n", res)
        return nil
      }),
  }
  err := chromedp.Run(ctx, tasks)
  if err != nil {
    panic(err)
  }
}

What did you expect to see?

Raw HTML dump of the result page.

What did you see instead?

The code above throws an error on a negative node, could there be a 16/32 bit int limitation being triggered somewhere?

panic: Could not find node with given id (-32000)

goroutine 1 [running]:
main.main.func1(0x1588080, 0xc0002b8030, 0x0, 0x0)
    github.go:33 +0x1c2
github.com/chromedp/chromedp.ActionFunc.Do(0x1504350, 0x1588080, 0xc0002b8030, 0x0, 0x0)
    go/pkg/mod/github.com/chromedp/[email protected]/chromedp.go:383 +0x3a
github.com/chromedp/chromedp.Tasks.Do(0xc00015a050, 0x5, 0x5, 0x1588080, 0xc0002b8030, 0x100d8b8, 0x30)
    go/pkg/mod/github.com/chromedp/[email protected]/chromedp.go:393 +0x72
github.com/chromedp/chromedp.Tasks.Do(0xc0000fbf78, 0x1, 0x1, 0x1588080, 0xc0002b8030, 0xc0001361b0, 0x1588080)
    go/pkg/mod/github.com/chromedp/[email protected]/chromedp.go:393 +0x72
github.com/chromedp/chromedp.Run(0x1588080, 0xc000170720, 0xc0000fbf78, 0x1, 0x1, 0x1581fa0, 0xc000030180)
    go/pkg/mod/github.com/chromedp/[email protected]/chromedp.go:229 +0x146
main.main()
    github.go:39 +0x444

Most helpful comment

After some debugging, I've figured out what's going on - you have a data race.

Here is the timeline for this to happen:

1) You navigate to the search page, enter the word, and enter the newline, which triggers a navigation to the results page at some point in the future
2) This asynchronous navigation sends a bunch of events, such as "hey, I've navigated to a new document"
3) You wait for body to be ready; this happens instantly after, so most of the times, it finishes instantly too, as it matches the body in the initial page, not the results page. No further events need to be processed.
4) You grab the document root of the original page, which is also near instant
5) To obtain the result from 4, we must process its result from a recent event, thus most likely processing the result from 1-2, and changing what the current document is.
6) You do a query with the node ID from 4 (the original document), but we're now on the new document, and the node ID is invalid (node IDs are unique across documents)

It makes sense that nearly 100% of the time on a fast connection, you hit this error.

SendKeys can't possibly know if it should trigger a navigation, and thus wait for it to complete. This is similar to a click; sometimes the actual decision to navigate is up to javascript.

So the bug, technically, is in your code. Though I admit that the situation is confusing, the error is useless, and we could be offering better APIs and docs here.

You can replace your WaitReady call with the following, which is exactly what chromedp.Navigate does to wait for a navigation to finish:

chromedp.ActionFunc(func(ctx context.Context) error {
        ch := make(chan struct{})
        lctx, cancel := context.WithCancel(ctx)
        chromedp.ListenTarget(lctx, func(ev interface{}) {
                if _, ok := ev.(*page.EventLoadEventFired); ok {
                        cancel()
                        close(ch)
                }
        })
        select {
        case <-ch:
                return nil
        case <-ctx.Done():
                return ctx.Err()
        }
}),

With this, the program works for me nearly 100% of the time.

I should note that this would still be racy, because if the SendKeys above somehow finishes immediately, or the ActionFunc above takes a long time to start, the program could deadlock forever. But I should also note that running the ActionFunc in parallel with SendKeys is also racy, if the page was just finishing a load when you reach both actions - as ActionFunc could finish with that previous load instead of the load from the SendKeys running next to it.

In any case - the chunk of code above is a reasonable solution for you right now. I've been reluctant to add API for it, because I haven't found a way that isn't racy (see above). Puppeteer has an extremely similar API at https://github.com/puppeteer/puppeteer/blob/master/docs/api.md#pagewaitfornavigationoptions, which you're supposed to use with Promise.All. So they go with the second option of the ones I show above. I'm not sure why they haven't run into races.

All 4 comments

Thanks for the report. This is great, because the same bug was reported in https://github.com/chromedp/chromedp/issues/338, but we didn't have a way to reproduce the bug. I can definitely reproduce it here.

After some debugging, I've figured out what's going on - you have a data race.

Here is the timeline for this to happen:

1) You navigate to the search page, enter the word, and enter the newline, which triggers a navigation to the results page at some point in the future
2) This asynchronous navigation sends a bunch of events, such as "hey, I've navigated to a new document"
3) You wait for body to be ready; this happens instantly after, so most of the times, it finishes instantly too, as it matches the body in the initial page, not the results page. No further events need to be processed.
4) You grab the document root of the original page, which is also near instant
5) To obtain the result from 4, we must process its result from a recent event, thus most likely processing the result from 1-2, and changing what the current document is.
6) You do a query with the node ID from 4 (the original document), but we're now on the new document, and the node ID is invalid (node IDs are unique across documents)

It makes sense that nearly 100% of the time on a fast connection, you hit this error.

SendKeys can't possibly know if it should trigger a navigation, and thus wait for it to complete. This is similar to a click; sometimes the actual decision to navigate is up to javascript.

So the bug, technically, is in your code. Though I admit that the situation is confusing, the error is useless, and we could be offering better APIs and docs here.

You can replace your WaitReady call with the following, which is exactly what chromedp.Navigate does to wait for a navigation to finish:

chromedp.ActionFunc(func(ctx context.Context) error {
        ch := make(chan struct{})
        lctx, cancel := context.WithCancel(ctx)
        chromedp.ListenTarget(lctx, func(ev interface{}) {
                if _, ok := ev.(*page.EventLoadEventFired); ok {
                        cancel()
                        close(ch)
                }
        })
        select {
        case <-ch:
                return nil
        case <-ctx.Done():
                return ctx.Err()
        }
}),

With this, the program works for me nearly 100% of the time.

I should note that this would still be racy, because if the SendKeys above somehow finishes immediately, or the ActionFunc above takes a long time to start, the program could deadlock forever. But I should also note that running the ActionFunc in parallel with SendKeys is also racy, if the page was just finishing a load when you reach both actions - as ActionFunc could finish with that previous load instead of the load from the SendKeys running next to it.

In any case - the chunk of code above is a reasonable solution for you right now. I've been reluctant to add API for it, because I haven't found a way that isn't racy (see above). Puppeteer has an extremely similar API at https://github.com/puppeteer/puppeteer/blob/master/docs/api.md#pagewaitfornavigationoptions, which you're supposed to use with Promise.All. So they go with the second option of the ones I show above. I'm not sure why they haven't run into races.

Lots of good findings, thanks for that! Guess I had assumed that the two calls in the sequence

node, err := dom.GetDocument().Do(ctx)
dom.GetOuterHTML().WithNodeID(node.NodeID).Do(ctx)

were running on the client, but looks like they're two separate calls sent to the browser, which might have changed content in between. Maybe I'm doing something overly complicated here, all I wanted is to retrieve the entire raw HTML displayed by the browser at a given point in time -- is there an easier way to get that, and in just one (non-racy) call, preferably?

Well, it depends on what you're after. If you just want the HTML node data, you can dump it straight from the Node Go type: https://godoc.org/github.com/chromedp/chromedp#example-package--DocumentDump

You can also retrieve the outer HTML in one single step: https://godoc.org/github.com/chromedp/chromedp#example-package--RetrieveHTML

That last action takes a query, but you can simply do chromedp.OuterHTML("body", &res, chromedp.ByQuery). If you look at the source of that, it uses a single command to the browser, so it's non-racy. And it uses the first node to match the selection, so it's fine for your purposes.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

howesteve picture howesteve  路  3Comments

aymericbeaumet picture aymericbeaumet  路  5Comments

opennota picture opennota  路  4Comments

zhaobingss picture zhaobingss  路  4Comments

seanchann picture seanchann  路  3Comments