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

New race condition: url changes, but page is not a new page #190

Closed
plocket opened this issue Apr 2, 2021 · 1 comment · Fixed by #201
Closed

New race condition: url changes, but page is not a new page #190

plocket opened this issue Apr 2, 2021 · 1 comment · Fixed by #201
Assignees
Labels
bug Something isn't working

Comments

@plocket
Copy link
Collaborator

plocket commented Apr 2, 2021

Currently we're checking for the url to change. Why is that? Because we had an issue where puppeteer thought the next 'page' had loaded, but it was still the same page. (This begs the question of whether we should even use .waitForNavigation(), but maybe that's a different issue.)

Now we're running across the url changing while the same page is loaded (or reloaded?) before it moves on to the next page. We need something unique to detect a true change in a page.

Current suggestion is the sought var in the _events hidden input field value. That should be unique, though we cannot be sure of that.

Note: We do not have a clear way to reproduce this bug, but it did happen consistently [ in DBD2 ].

Note: We thought of using this to detect an infinite screen (a da-specific bug) - when the sought var doesn't change, but the url changes, it may be an 'infinite screen'. That won't work exactly because of the case we just ran into. Maybe detecting multiple consecutive page changes with the same sought var would be sufficient.

@plocket
Copy link
Collaborator Author

plocket commented Apr 5, 2021

Another option is to change scope.afterStep() to use a while. This is what the whole function would look like with this:

  afterStep: async function afterStep(scope, options = {waitForShowIf: false, navigated: false, waitForTimeout: 0}) {
    /* Common things to do after a step, including take care of errors.
    *    TODO: discuss splitting into `afterField` and `afterStep` or something. Or just change name
    *    TODO: Update to latest cucumberjs, which as `AfterStep` */

    // Use a while loop until the below does not error with `Error: Execution context was destroyed`
    let error_id_elem;
    while ( true ) {
      try {
        error_id_elem = await scope.page.$('#da-retry');
        break;
      } catch ( err ) {}
    }

    // If there's an error, throw it
    if ( error_id_elem ) {
      let error_elem = await scope.page.$('blockquote')
      let error_handle = await error_elem.getProperty( 'textContent' );
      let error_text = await error_handle.jsonValue();

      throw error_text;
    }

    // RESET
    if ( options.navigated ) { scope.navigated = true; }
    else { scope.navigated = false; }

    // WAITING
    if ( options.waitForShowIf ) { await scope.waitForShowIf(scope); }
    // Plain old waiting. Yeah, I abstracted it into here so only
    // one thing was being called at the end
    if ( options.waitForTimeout ) { await scope.page.waitFor( options.waitForTimeout ); }
  },  // Ends afterStep

This might be an ok temporary measure.

@plocket plocket added the bug Something isn't working label Apr 5, 2021
plocket added a commit that referenced this issue Apr 6, 2021
Too many fixes. Fixes #190, but other bugs found. Outlined in CHANGELOG.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant