microsoft / playwright

Playwright is a framework for Web Testing and Automation. It allows testing Chromium, Firefox and WebKit with a single API.
https://playwright.dev
Apache License 2.0
66.42k stars 3.63k forks source link

[Bug] page.reload() hangs when dialog.dismiss() beforeunload is called #20624

Closed Sunilcharlesedward closed 11 months ago

Sunilcharlesedward commented 1 year ago

Context:

In my testCase i have to refresh a page and verify unsaved changes, both ways. i.e await dialog.AcceptAsync() ( this reloads the page and i can verify that changes are lost). However, when i use 'await dialog.DismissAsync();' it dismisses the unsaved changes dialog but comes up with this error

waiting for navigation until "load"

Stack Trace:  Connection.InnerSendMessageToServerAsync[T](String guid, String method, Dictionary2 dictionary) line 165 Connection.WrapApiCallAsync[T](Func1 action, Boolean isInternal) Page.ReloadAsync(PageReloadOptions options) line 684

Code Snippet

public async Task BrowserRefresh() { HandlePageDialog();
await _page.ReloadAsync();
}

    public void HandlePageDialog()
    {
        _page.Dialog += async (_, dialog) =>
        {
            Assert.AreEqual("beforeunload", dialog.Type);
            **await dialog.DismissAsync();**
            //await dialog.AcceptAsync();                            
        **};**
    }

image

dgozman commented 1 year ago

@Sunilcharlesedward Try calling page.CloseAsync(new () { RunBeforeUnload = true }) instead of reload. This method will trigger the before unload dialog, and only close the page if you accept the dialog. Let me know if that helps.

Sunilcharlesedward commented 1 year ago

@Sunilcharlesedward Try calling page.CloseAsync(new () { RunBeforeUnload = true }) instead of reload. This method will trigger the before unload dialog, and only close the page if you accept the dialog. Let me know if that helps.

dgozman I have tired this and that too results in exctly the same error with same stacktrace. Moreover, my framework has Page.CloseAsync() as part of the [Teardown] i.e after every test, close the page. And in my testCase i am trying to verify whether refreshing the browser and chk unsaved functionality. Also, i have tried Control+R and F5 keys to refresh, but still no go, it even didint refresh the page i.e no dialog pops out so it wont even enter the handler.

mxschmitt commented 1 year ago

We have tests in place related to that, e.g. this one. Could you help us creating a minimal reproducible so we run into your exception?

Sunilcharlesedward commented 1 year ago

Yes, here are the steps to reproduce:

  1. NavigateTo this URL ("https://codepen.io/chriscoyier/pen/GRRJobP")
  2. Put a debug point, so that we can edit some stuff in one of the boxes.
  3. Enter some stuff e.g image

    1. do a page. reload using the functions below.

    public async Task BrowserReload() { HandlePageDialog(); await _page.ReloadAsync(); } public void HandlePageDialog() { page.Dialog += async (, dialog) => { alertMessage.Add(dialog.Type); Assert.AreEqual("beforeunload", dialog.Type); await dialog.DismissAsync(); //await dialog.AcceptAsync(); }; }

Exception seen:

Message:  System.TimeoutException : Timeout 30000ms exceeded. =========================== logs =========================== waiting for navigation until "load"

Stack Trace:  Connection.InnerSendMessageToServerAsync[T](String guid, String method, Dictionary2 dictionary) line 143 Connection.WrapApiCallAsync[T](Func1 action, Boolean isInternal) Page.ReloadAsync(PageReloadOptions options) line 684 BasePageTestHelper.BrowserReload() line 149 Helper.BrowserReload() line 93 AssignmentTask.TC4() line 60 GenericAdapter1.BlockUntilCompleted() NoMessagePumpStrategy.WaitForCompletion(AwaitAdapter awaiter) AsyncToSyncAdapter.Await(Func1 invoke) TestMethodCommand.RunTestMethod(TestExecutionContext context) TestMethodCommand.Execute(TestExecutionContext context) <>c__DisplayClass1_0.b__0() DelegatingTestCommand.RunTestMethodInThreadAbortSafeZone(TestExecutionContext context, Action action)

mxschmitt commented 1 year ago

Can we have a reproducible which does not involve Visual Studio? It looks expected that if ReloadAsync has started and then you have a breakpoint, that it will throw a timeout error if you break more than 30 seconds.

Sunilcharlesedward commented 1 year ago

I use visual studio only. However, if you try to vs code you will get the sane error. I have to put breakpoint immediately after i have navigated to url so as to type in something hence i have unsaved changes. Then after that i call page.reload. so breakpoint is after that.

In my steps to reproduce earlier, step 2 is the breakpoint and step 4 is page.reload

mxschmitt commented 1 year ago

What about using Page.PauseAsync() instead? This is the recommended way of setting a breakpoint, halting the whole program is not the best, since then no Playwright communication can happen.

Sunilcharlesedward commented 1 year ago

Potentially, we can try using Page.PauseAsync().However, whatever we use Pause or a breakpoint it's only a mechanism to edit some stuff resulting in unsaved changes alert. I don't understand how it is going to resolve the issue.

On Tue, Feb 28, 2023 at 8:48 PM Max Schmitt @.***> wrote:

What about using Page.PauseAsync() instead? This is the recommended way of setting a breakpoint, halting the whole program is not the best, since then no Playwright communication can happen.

— Reply to this email directly, view it on GitHub https://github.com/microsoft/playwright/issues/20624#issuecomment-1447961529, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALPKLLMH5KLKOR5B76F4BH3WZXJYFANCNFSM6AAAAAAUQV3DMU . You are receiving this because you were mentioned.Message ID: @.***>

mxschmitt commented 1 year ago

I tried the following and it was working for me, could you help me creating a repro which ends up in your error?

using Microsoft.Playwright;

using var playwright = await Playwright.CreateAsync();
await using var browser = await playwright.Chromium.LaunchAsync(new() { Headless = false });
var page = await browser.NewPageAsync();
await page.GotoAsync("https://codepen.io/chriscoyier/pen/GRRJobP");
Console.Write("Hello ");
// set breakpoint here.
Console.Write("World!\n");
await page.ReloadAsync();
Sunilcharlesedward commented 1 year ago

Try these steps:

using Microsoft.Playwright;
using var playwright = await Playwright.CreateAsync();
await using var browser = await playwright.Chromium.LaunchAsync(new() { Headless = false });
var page = await browser.NewPageAsync();
await page.GotoAsync("https://codepen.io/chriscoyier/pen/GRRJobP");

page.Dialog += async (_, dialog) =>
{
   // alertMessage.Add(dialog.Type);
    Assert.AreEqual("beforeunload", dialog.Type);
    await dialog.DismissAsync();
    //await dialog.AcceptAsync();
};
// set breakpoint here @page.ReloadAsync() step and type the word Hello as shown in screenshot            
await page.ReloadAsync();`

image

mxschmitt commented 1 year ago

I was able to create a minimal reproducible out of it:

using Microsoft.Playwright;
using var playwright = await Playwright.CreateAsync();
await using var browser = await playwright.Chromium.LaunchAsync(new() { Headless = false });
var page = await browser.NewPageAsync();
await page.GotoAsync("https://example.com");

// set the page content with a beforeunload dialog
await page.EvaluateAsync(@"
    window.addEventListener('beforeunload', (e) => {
        e.returnValue = 'Are you sure you want to leave?';
    });
");

page.Dialog += async (_, dialog) =>
{
    Console.WriteLine(dialog.Type);
    await dialog.DismissAsync();
    Console.WriteLine("Dialog dismissed");
};
Console.WriteLine("Reloading page");
await page.ReloadAsync();
import { chromium } from 'playwright';

(async () => {
  const browser = await chromium.launch({ headless: false });
  const page = await browser.newPage();
  await page.goto('https://example.com');

  await page.evaluate(() => {
    window.addEventListener('beforeunload', e => {
      e.returnValue = 'Are you sure you want to leave?';
    });
  });

  page.on('dialog', async dialog => {
    console.log(dialog.type());
    await dialog.dismiss();
    console.log('Dialog dismissed');
  });
  console.log('Reloading page');
  await page.reload();
})();

TL;DR: if you have an beforeunload event handle on a page and reload and let Playwright dismiss it, it will stall and timeout.

mxschmitt commented 1 year ago

If you want to workaround the issue, you can just call dialog.accept() instead.

Sunilcharlesedward commented 1 year ago

But the TestScenario here is to categorically validate that unsaved changes are not lost on page after successfully calling dialog.dismiss. Hence, its is blocked.

pavelfeldman commented 11 months ago

Why was this issue closed?

Thank you for your involvement. This issue was closed due to limited engagement (upvotes/activity), lack of recent activity, and insufficient actionability. To maintain a manageable database, we prioritize issues based on these factors.

If you disagree with this closure, please open a new issue and reference this one. More support or clarity on its necessity may prompt a review. Your understanding and cooperation are appreciated.

samdenty commented 6 months ago

@pavelfeldman is there a reason why this was closed? https://github.com/microsoft/playwright/issues/20624#issuecomment-1458143769 is a totally valid reproduction of the bug