Browser hangs indefinitely if new_page() fails

[tbakerx]

I'm seeing an issue that appears to be similar to the closed issue #52 in which browsers failing to open to new_page don't use the browser's timeout and instead hang indefinitely. It should be noted that I'm on v0.3.1 and so should have the fix from PR #56.

I have a timeout on the new_page() call (longer than the browser's request timeout).
If this timeout is hit and I move on, the browser refuses to allow any other requests until it is torn down.

Here's a minimal repro:

use futures_util::stream::{StreamExt};

use chromiumoxide::browser::{Browser, BrowserConfig};
use tokio::time::{timeout, sleep, Duration};

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    println!("Started...");

    let (browser, mut handler) = Browser::launch(
        BrowserConfig::builder()
            .request_timeout(Duration::from_secs(3))
            .build()?,
    )
    .await?;

    tokio::spawn(async move {
        loop {
            let _ = handler.next().await.unwrap();
        }
    });

    // Navigation to a valid sample page succeeds
    attempt_navigation("https://example.com".to_owned(), &browser).await;
    
    // Navigation to invalid url fails (and should). This is a silly example, it 
    // is simply to demonstrate the behaviour. When the browser cannot successfully
    // open the page, it hangs indefinitely. Follow up requests (even those that
    // previously worked) will fail until the browser is torn down
    println!("Attempting to navigate to invalid page, should fail and timeout");
    attempt_navigation("https://%wxample.com".to_owned(), &browser).await;

    println!("Sleeping for a few seconds to allow timeout to happen");
    sleep(Duration::from_secs(5)).await;

    // Attempting the same page navigation that previously succeeded.
    // -> should now fail
    println!("Re-attempting to open previously successful page...");
    attempt_navigation("https://example.com".to_owned(), &browser).await;

    return Ok(());
}

async fn attempt_navigation(url: String, browser: &Browser) {
    let page_result = timeout(Duration::from_secs(5), browser.new_page(&url)).await;
    let _page = match page_result {
        Ok(_page) => println!("Page {} navigated to successfully", &url),
        Err(_) => println!("Request for new page timeout"),
    };
    return ;
}

[mattsse]

Great catch,
I tried to debug this, it has definitely something to do with the % sign me thinks, try using a & instead, that works...
What internally happens is that

• we send Sending MethodCall { id: CallId(1), method: "Target.createTarget", session_id: None, params: Object({"url": String("https://%wxample.com")}) }
• this crashes the websocket which is not handled gracefully, also no reconnect

Curious if this is a bug in the WS implementation or if chromium does not like it and crashes.

I think we can mitigate this by validating that the url in question is in fact a valid url

[tbakerx]

I believe you're right, I added simple URL parsing in my project as the workaround for now and seems to be sufficient