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

How to detect navigation? #229

Open
plocket opened this issue Jun 6, 2021 · 4 comments
Open

How to detect navigation? #229

plocket opened this issue Jun 6, 2021 · 4 comments
Labels
enhancement New feature or request

Comments

@plocket
Copy link
Collaborator

plocket commented Jun 6, 2021

I'm not sure it's possible because sometimes the page reloads the same page before moving on. I think there's another issue for this somewhere. If this wasn't the case, we may be able to use the sought var to test for the uniqueness of the page.

@BryceStevenWilley
Copy link
Collaborator

#190 in the other issue.

@plocket plocket self-assigned this Jul 29, 2024
@plocket
Copy link
Collaborator Author

plocket commented Jul 29, 2024

Using page.waitForRequest and page.waitForResponse plus some info about the behavior of the internals has yielded some good results. We're also detecting different invalidation feedback in different ways. We're also experimenting with using puppeteer promises and Promise.race with newer puppeteer because it has AbortControllers for some of their async operations, though not all, so it may get tricky. Here are some da dev suggestions, though I've implemented some of these already in my current efforts:

daPageLoad event is triggered when there is a GET request to /interview or /start or /run. It [daPageLoad] is triggered when a new screen is loaded. A new screen is loaded when the user clicks a Continue button or launches an action by clicking a hyperlink or equivalently when the action_perform() JavaScript function runs an action. The screen is also loaded when reload causes the screen to refresh. /checkin events do not load a new page; if they did the screen would reload every time someone changed the value of a field, and once every six seconds. However it is possible to call background_response('refresh') from check in code, or use background_action() with the second argument 'refresh' to send a signal for the browser to refresh the screen, which would cause a daPageLoad event.

The browser is not told whether the screen it is showing is influenced by validation code resulting in an exception. If the validation code attached errors to fields, you could inspect the jQuery Validation Plugin using JavaScript. If it caused a message to appear at the top of the screen, you could query the DOM for the presence of an error message at the top of the screen.

If we can figure out how to abort waiting for the daPageLoad jQuery event, we can possibly use that alone.

[Otherwise] I think we can detect the GET [for initial navigation to an interview and POSTs, all from (to?) /interview, /run, and /start]. For 200s, we can wait for the daPageLoad jQuery event. I think we can look into doing that in a page.waitForFunction to see if it takes an abort signal. I'm not sure page.evaluate does. Either one probably has to contain a new Promise, and I'm not sure how to pass the signal through to that Promise, but maybe if daPageLoad doesn't happen at that point, it really should error.

Then there's more specific issues after a 200:

  • The infinite page loop. For that, we need to check against relevant differences in the DOM (e.g. not cookies). We could try to look for the target var, but not everyone will have that. The page id is also not reliable as that could be repeated legitimately.
  • For validation code we need to look at the HTML as well to find that element, but even when it's not visible, so we might use the HTML from the response json .body property with cheerio.

For 500s, we can just look for the DOM loading. Those can race against finding invalid value message elements (client-side validation) that do prevent any request from being made. We then have to abort the promises that don't get fulfilled.

Not sure about 300s. [For interviews, they'd be like 200s. Maybe [the sign in 305 check] needs to be separate. On failure, sign in gets a 200, so we might have to use the url [for confirmation] there. I'm not sure what else we could detect [about sign in failure]. Maybe changes in the DOM.

Regarding background actions mentioned above, we may leave those undetected for now, only looking for immediate navigation when tapping an element. The first uses check in, so might not be a GET. If the second does GET a 200 that would race against invalidation elements that get detected first before the page reloads, I'm not sure how we'd deal with that. Both use POST, redraw the screen, and cause a daPageLoad event. There is no full reloading, though.

@plocket
Copy link
Collaborator Author

plocket commented Jul 29, 2024

tl;dr: background actions/responses use POST and don't totally reload the screen, though they may cause a redraw and do cause a daPageLoad event. [I edited the above comment to reflect that.]

More from da dev about background action behavior:

daPageLoad happens whenever a screen is drawn. There are lots of different contexts in which a screen is drawn. You can always count on daPageLoad being triggered after a screen is drawn.

To refresh the screen, background_action() relies on the browser calling /checkin periodically. The browser calls /checkin every time a field changes value and also every six seconds. (It does this regardless of whether there is a check in modifier on the question.) The response to /checkin may instruct the browser to refresh the screen. So background_action() does not have an immediate effect on the browser. For there to be an immediate effect, websockets would need to be used. All that background_action() does is update a key in Redis, which gets checked by the code that runs when the /checkin endpoint is next accessed.

background_response() and background_action() work basically the same way. The JavaScript in the browser effectuates a refresh by sending an asynchronous POST request to the /interview endpoint, and the response contains the information necessary to redraw the screen.

@plocket
Copy link
Collaborator Author

plocket commented Aug 7, 2024

This is dragging out a bit longer than expected. I'm going to set this aside for the moment in favor of closing up the sign-in bug. In the meantime, some notes:

The Plan:

Make a "I try to continue but cannot continue" or something. It's too complex to otherwise detect correct failed navigation (for when authors want to check that they have correctly prevented the user from moving forward). We can keep the step that checks for an invalid answer, but change it so that it doesn't have to know about whether the previous click navigated or not. Otherwise we have to track state through multiple Steps in a complex way.

Detect type of navigation or non-navigation:

  1. Start waiting for stuff listed below
  2. Wait for a click
  3. Race these:
    • Detect a successful navigation to an interview page - 200, maybe 305 (for sign in and other redirects to the interview). Specifically that it goes to an interview page so it can race with ones that don't go to an interview page1.
    • Detect a 305 that navigates to a non-interview page. That way, we can attach a daPageLoad listener to just the responses that should require a daPageLoad event, thus creating a proper race1.
    • Detect a 500 of some kind (504, 502, etc.)
    • Detect client-side invalid answer warnings.
  4. Whichever wins, abort the other promises - in node or on the browser - and remove the jQuery event listeners. Puppeteer methods we need, like response, have abort signals. You can see one in the repl.it. JS has its own that we can use on the browser, as shown in the repl.it link.
  5. Store if we navigated or not.
  6. Store the HTML to compare to future HTML and identify if we've got a simple infinite page loop or to detect a complex multi-page infinite page loop. This requires keeping track of multiple page loops. We can't count on page ids or trigger vars, which may not exist. Remember that pages triggered with mandatory: True won't have a trigger var.
  7. Potentially error on infinite page loops (allow 20 repeat appearances or something)
  8. Check if we were supposed to navigate or not. Fail or succeed based on that.

I think that's approximately it. For messy start of implementation see https://github.com/plocket/ALKilnReview/tree/229_930_navigation. Note co-authors there and be sure to credit them in new work.

Footnotes

  1. Imagine a race between [click, a_200s_or_305_response, daPageLoad] and just [click, a_200s_or_305_response]. They listen to the same 2 promises at the start, so the one with daPageLoad always loses. Instead, imagine a race between [click, interview_200s_or_305, daPageLoad] and [click, non_interview_305]. Those are actually different and can properly race each other. 2

@plocket plocket removed their assignment Aug 7, 2024
@plocket plocket added the enhancement New feature or request label Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants