nightwatch icon indicating copy to clipboard operation
nightwatch copied to clipboard

Element API sends further HTTP requests in failed chained commands

Open dikwickley opened this issue 5 months ago • 10 comments

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: https://github.com/nightwatchjs/nightwatch/issues/3991 Reference comment: https://github.com/nightwatchjs/nightwatch/issues/3991#issuecomment-1969444042

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 avatar Mar 01 '24 18:03 dikwickley

@dikwickley can you assign this to me?

Chamara00 avatar Mar 02 '24 04:03 Chamara00

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.

garg3133 avatar Mar 02 '24 10:03 garg3133

@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 avatar Mar 02 '24 12:03 Chamara00

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

Chamara00 avatar Mar 02 '24 12:03 Chamara00

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

gravityvi avatar Mar 03 '24 14:03 gravityvi

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

Chamara00 avatar Mar 04 '24 15:03 Chamara00

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

uditrajput03 avatar Mar 22 '24 11:03 uditrajput03

can you share the file path

AbhisekOmkar avatar Mar 31 '24 06:03 AbhisekOmkar

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`.

garg3133 avatar Apr 01 '24 09:04 garg3133

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

uditrajput03 avatar Apr 01 '24 10:04 uditrajput03