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

Element API sends further HTTP requests in failed chained commands #4077

Open
dikwickley opened this issue Mar 1, 2024 · 10 comments · May be fixed by #4219
Open

Element API sends further HTTP requests in failed chained commands #4077

dikwickley opened this issue Mar 1, 2024 · 10 comments · May be fixed by #4219

Comments

@dikwickley
Copy link
Contributor

dikwickley commented Mar 1, 2024

Description of the bug/issue

Further chained commands in the new element API sends HTTP request even when preceding command has failed.

sample test

describe('Ecosia.org Demo', function() {

  before(browser => {
    browser
      .navigateTo('https://www.ecosia.org/');
  });

  it('Demo test ecosia.org', function(browser) {
    browser
      .waitForElementVisible('body')
      .element
      .find('.wrong_selector')
      .getProperty('innerHTML');

  });

  after(browser => browser.end());
});

image image

Branched off from issue: #3991
Reference comment: #3991 (comment)

Potential Solution

If the previous element is null, we should not send any further HTTP requests for the subsequent chained commands. Notice in the image above we are hitting /element/null/... endpoint, that should not happen.

Nightwatch.js Version

3.3.7

Node Version

No response

Browser

No response

Operating System

No response

Additional Information

No response

@dikwickley dikwickley changed the title Further chained commands in element api send HTTP request with element id as null Further chained commands in element api send HTTP request with element id as null when previous command has failed Mar 1, 2024
@dikwickley dikwickley changed the title Further chained commands in element api send HTTP request with element id as null when previous command has failed Element API sends further HTTP requests in failed chained commands Mar 1, 2024
@Chamara00
Copy link

@dikwickley can you assign this to me?

@garg3133
Copy link
Member

garg3133 commented Mar 2, 2024

Sure @Chamara00, please free to investigate the issue and propose a solution here. If the solution looks good or close enough, we'll assign the issue to you.

@Chamara00
Copy link

Chamara00 commented Mar 2, 2024

@garg3133 @dikwickley My proposed solution

  1. Before executing any subsequent chained commands, Nightwatch.js should check if the reference to the previous element is null.
  2. If the previous element reference is null, Nightwatch.js should log a warning or error message to indicate that the element was not found. It should then skip the execution of further commands.
  3. Nightwatch.js should stop the execution of subsequent chained commands to prevent sending further HTTP requests.
  4. Nightwatch.js should provide feedback to users or test scripts indicating that the operation failed due to the absence of the element.

sampe code

`function executeChainedCommand(browser, command) {
    if (browser.lastElementReference === null) {
        console.error('Previous element reference is null. Skipping chained command execution.');
        return; // Halt execution of chained command
    }

    // Proceed with executing the chained command
    command.execute();
}

// Example usage:
executeChainedCommand(browser, element.find('.info'));
executeChainedCommand(browser, element().getElementProperty('innerHTML'));
`

@Chamara00
Copy link

@dikwickley @garg3133 please can you me a feedback for my solution. If it's good assign this issue to me

@gravityvi
Copy link
Member

@Chamara00 could raise a PR with that solution? I am little skeptical if the above approach would work

@Chamara00
Copy link

Hey, Please review my PR @garg3133 @gravityvi #4088

@uditrajput03
Copy link
Contributor

uditrajput03 commented Mar 22, 2024

I have tried to solve this issue but unable to solve it but I got something, take it as a pinch of salt.

  1. We can work on the direct provided solution that If the response from previous webElement is null then stop executing it further for either of these executeMethod or runQueuedCommand. I have worked on this but I can't find corresponding value to the element-6066-11e4-a52e-4f735466cecf something like this, to be precise finding when it returns value to be null.
  2. We can involves an error stack and pass it in this to the chained methods and check whether it is empty or not before executing the next method.
    Eventually, we can suppress these errors but it is better to stop further executing chained requests.

Moreover, I think working on abortOnFailure rejection can solve this issue perfectly as the use case of the user want to run further chained commands on abortOnFailure set to false and and abort it on true

@AbhisekOmkar
Copy link

can you share the file path

@garg3133
Copy link
Member

garg3133 commented Apr 1, 2024

We can work on the direct provided solution that If the response from previous webElement is null then stop executing it further for either of these executeMethod or runQueuedCommand. I have worked on this but I can't find corresponding value to the element-6066-11e4-a52e-4f735466cecf something like this, to be precise finding when it returns value to be null.

It actually passes a WebElementPromise to the methods present in method-mappings.js file, and then executes the corresponding method directly on it without resolving the Promise first. So, we'd need to resolve the WebElementPromise first somewhere to get that null value if the element does not exist.

Moreover, I think working on abortOnFailure rejection can solve this issue perfectly as the use case of the user want to run further chained commands on abortOnFailure set to false and and abort it on true

@uditrajput03 What do you mean by this? When abortOnFailure is set to false, we don't want to execute the further chained commands on Element API because it won't make any sense if the element itself is not found, but we would want to continue execution of any commands/assertions following that particular chained block of Element API commands. For ex.

  await browser.element('.invalid_selector').getText(); // getText shouldn't run here for any value of `abortOnFailure`

  await browser.getTitle(); // should continue executing in case `abortOnFailure` is set to `false`.

@uditrajput03
Copy link
Contributor

We can work on the direct provided solution that If the response from previous webElement is null then stop executing it further for either of these executeMethod or runQueuedCommand. I have worked on this but I can't find corresponding value to the element-6066-11e4-a52e-4f735466cecf something like this, to be precise finding when it returns value to be null.

It actually passes a WebElementPromise to the methods present in method-mappings.js file, and then executes the corresponding method directly on it without resolving the Promise first. So, we'd need to resolve the WebElementPromise first somewhere to get that null value if the element does not exist.

Moreover, I think working on abortOnFailure rejection can solve this issue perfectly as the use case of the user want to run further chained commands on abortOnFailure set to false and and abort it on true

@uditrajput03 What do you mean by this? When abortOnFailure is set to false, we don't want to execute the further chained commands on Element API because it won't make any sense if the element itself is not found, but we would want to continue execution of any commands/assertions following that particular chained block of Element API commands. For ex.

  await browser.element('.invalid_selector').getText(); // getText shouldn't run here for any value of `abortOnFailure`

  await browser.getTitle(); // should continue executing in case `abortOnFailure` is set to `false`.

Got it now , Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants