Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

-32000 Not attached to an active page panic when ERR_CONNECTION_REFUSED #700

Closed
samymassoud opened this issue Aug 25, 2022 · 8 comments
Closed
Labels
question Questions related to rod

Comments

@samymassoud
Copy link

Rod Version: v0.107.3

Issue

try to navigate to any site that will trigger ERR_CONNECTION_REFUSED, then defer page close, the panic will happen.

Sample code

func TestLab(t *testing.T) {
        browser := rod.New().MustConnect()

	// Even you forget to close, rod will close it after main process ends.
	defer browser.MustClose()

	// Create a new page
	p := browser.MustPage("http://localhost")
        defer p.MustClose()

        p.Timeout(15 * time.Second).
			MustNavigate(input).
			CancelTimeout()
}

What you got

[running]:
github.com/labstack/echo/v4/middleware.RecoverWithConfig.func1.1.1()
        /Users/samymassoud/go/pkg/mod/github.com/labstack/echo/v4@v4.7.2/middleware/recover.go:93 +0x124
panic({0x105176780, 0x14000088090})
        /opt/homebrew/Cellar/go/1.18.4/libexec/src/runtime/panic.go:838 +0x204
github.com/go-rod/rod/lib/utils.glob..func2({0x105176780?, 0x14000088090?})
        /Users/samymassoud/go/pkg/mod/github.com/go-rod/rod@v0.107.3/lib/utils/utils.go:59 +0x28
github.com/go-rod/rod.genE.func1({0x14000020080?, 0x140001d8000?, 0x105227258?})
        /Users/samymassoud/go/pkg/mod/github.com/go-rod/rod@v0.107.3/must.go:36 +0x68
github.com/go-rod/rod.(*Page).MustClose(0x140001e8000)
        /Users/samymassoud/go/pkg/mod/github.com/go-rod/rod@v0.107.3/must.go:330 +0x90

Issue

The reference to the opened page should be retained, so it can be closed properly, also as a precaution, the MustClose() method should check that the page is not nil.

@samymassoud samymassoud added the question Questions related to rod label Aug 25, 2022
@rod-robot
Copy link

Please fix the format of your markdown:

3 MD022/blanks-around-headings/blanks-around-headers Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "## Issue"]
25 MD022/blanks-around-headings/blanks-around-headers Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "## What you got"]
26 MD031/blanks-around-fences Fenced code blocks should be surrounded by blank lines [Context: "```"]
26 MD040/fenced-code-language Fenced code blocks should have a language specified [Context: "```"]
38 MD031/blanks-around-fences Fenced code blocks should be surrounded by blank lines [Context: "```"]
39:9 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]
39 MD022/blanks-around-headings/blanks-around-headers Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Above] [Context: "## Issue"]
39 MD024/no-duplicate-heading/no-duplicate-header Multiple headings with the same content [Context: "## Issue"]

generated by check-issue

@ysmood ysmood added bug When you are sure about it's a bug and removed question Questions related to rod labels Aug 27, 2022
@ysmood
Copy link
Member

ysmood commented Aug 27, 2022

Rod will automatically close the page if the navigation fails, please read the source code of Browser.Page, it's very easy to understand:

rod/browser.go

Lines 188 to 193 in 67a64b6

defer func() {
// If Navigate or PageFromTarget fails we should close the target to prevent leak
if err != nil {
_, _ = proto.TargetCloseTarget{TargetID: target.TargetID}.Call(b)
}
}()

Your code won't execute the line of defer p.MustClose(), so MustClose doesn't have to check if the page is nil or not.

@ysmood ysmood added question Questions related to rod and removed bug When you are sure about it's a bug labels Aug 27, 2022
@ysmood
Copy link
Member

ysmood commented Aug 27, 2022

I can't reproduce your issue, the page is automatically closed with your code, it works fine to me. If you run it in headful mode, you should see a page show up and close quickly, watch it carefully.

@samymassoud
Copy link
Author

samymassoud commented Aug 27, 2022

Ok, I thought the issue is with the standard Page load, it happens with Stealth though.

Please check if this close the page for you

func main() {
	u := launcher.New().Headless(false).MustLaunch()
	b := rod.New().ControlURL(u).MustConnect()

	url := "http://localhost"
	//p := b.MustPage(url)
	p := stealth.MustPage(b)
	defer p.Close()
	err := rod.Try(func() { //Wait Max 15 Sec
		p = p.Timeout(15 * time.Second).
			MustNavigate(url).
			CancelTimeout()
	})
	if err != nil {
		//fmt.Println(err.Error())
	}
	for {
		//Do nothing
	}
}

@ysmood
Copy link
Member

ysmood commented Aug 27, 2022

Works fine to me, try the code below, you have to wait for the wrong navigation is settled:

package main

import (
	"fmt"

	"github.com/go-rod/rod"
	"github.com/go-rod/rod/lib/defaults"
	"github.com/go-rod/rod/lib/utils"
	"github.com/go-rod/stealth"
)

func main() {
	defaults.Show = true

	b := rod.New().MustConnect()

	p := stealth.MustPage(b)
	rod.Try(func() {
		p.MustNavigate("http://localhost")
	})

	p.MustWaitNavigation() // watch out for this line
	p.MustClose()

	fmt.Println("page closed")
	utils.Pause()
}

I think this is a bug of chromium, it doesn't allow us to close a page while it's navigating.
Maybe we can add some protection code to make it easier for beginners.

@ysmood
Copy link
Member

ysmood commented Aug 27, 2022

Why you don't see the error is because your code is not checking the error of defer p.Close(), it actually returns an error that tells you the close action has failed.

Please always check errors returned by functions, or use MustClose if you are lazy.

@ysmood ysmood closed this as completed in 635c06d Aug 27, 2022
@samymassoud
Copy link
Author

Yes, that is the issue, if you want to control the time used to load the page using the timeout and modify the block

err := rod.Try(func() { //Wait Max 15 Sec
		p = p.Timeout(15 * time.Second).
			MustNavigate(URL).
                         MustWaitNavigation(). //Notice here in the timeout block, it will not work
			CancelTimeout()
	})

But if moved outside the above block, it will work as in your example, but I will have to wait for the Page to Load, which can be time-consuming for some websites.

@ysmood
Copy link
Member

ysmood commented Aug 28, 2022

About your last code, I think you should learn more about how panic works in golang. It's not related to rod.

Use the lastest version of rod, it should work now.

alexferrari88 pushed a commit to alexferrari88/rod that referenced this issue Aug 28, 2022
alexferrari88 pushed a commit to alexferrari88/rod that referenced this issue Aug 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Questions related to rod
Projects
None yet
Development

No branches or pull requests

3 participants