Improve DomCrawler's "The current node list is empty"

[pevdh]

I've been using the DomCrawler to create functional tests for a little while now. While it's a very robust and easy to use component, a major pain point is whenever a test fails with an InvalidArgumentException: "The current node list is empty".

I use these kind of constructs frequently:

$crawler = $browser->request('GET', '/');

$crawler = $browser->submit($crawler->filter('#some-form')->form(), [
    /* form values */
]);

or:

$messageContent = $crawler->filter('ul li.message-content')->text();

Methods such as form() and text() will fail if the crawler cannot find the selector.

While PHPUnit of course provides the exact line number where the error occurred, I think it would improve DX a lot if the error message contained more information such as the CSS/XPath selectors that were used.

For example:

InvalidArgumentException : No nodes matched the selector '#some-form'

I would have no issue making a PR for this issue, but since it would be my first time contributing I thought I'd seek more input first.

[stof]

The issue here is that the error is not thrown by a method having the selector but by the ->form method. And not all Crawler objects are created using a selector.

[pevdh]

Yes, that is true. For example, this is what the filter() method looks like currently:

public function filter($selector)
{
        $converter = $this->createCssSelectorConverter();

        // The CssSelector already prefixes the selector with descendant-or-self::
        return $this->filterRelativeXPath($converter->toXPath($selector));
}

What I'm proposing is that Crawler keeps track of some sort of "context", that it can then provide when throwing the InvalidArgumentException. E.g.:

public function filter($selector)
{
    $converter = $this->createCssSelectorConverter();

    $crawler = $this->filterRelativeXPath($converter->toXPath($selector));
    $crawler->addContext($selector);
    return $crawler;
}

And then form() can look like:

public function form(array $values = null, $method = null)
    {
        if (!$this->nodes) {
            throw new \InvalidArgumentException(sprintf('No nodes matched the selector "%s"', $this->getContextAsString()));
        }

    /* ... omitted ... */
    return $form;
}

I'm not sure if this is feasible/how feasible this implementation is. What do you think?

[carsonbot]

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

[carsonbot]

Could I get an answer? If I do not hear anything I will assume this issue is resolved or abandoned. Please get back to me <3

[carsonbot]

Hey,

I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen!

[pedzed]

Every. Single. Time.

Take the following filter:

$crawler = new Crawler($html);
$element = $crawler->filter('[data-my-attr]');

This problem then occurs when calling a subsequent method, which, I presume, indirectly uses the private field $element->nodes whilst it is empty:

$element->attr('data-my-attr'); // InvalidArgumentException: The current node list is empty.

Symfony\Component\DomCrawler\Crawler^ {
  #uri: null
  -defaultNamespacePrefix: "default"
  -namespaces: []
  -cachedNamespaces: ArrayObject {#1556
    -storage: []
    flag::STD_PROP_LIST: false
    flag::ARRAY_AS_PROPS: false
    iteratorClass: "ArrayIterator"
  }
  -baseHref: null
  -document: DOMDocument {#1539
    XML/HTML DATA HERE
  }
  -nodes: []  // Empty <--
  -isHtml: true
  -html5Parser: Masterminds\HTML5^ {#1557
    -defaultOptions: array:2 [
      "encode_entities" => false
      "disable_html_ns" => true
    ]
    #errors: []
  }
}

Wrap it around a try-catch and it works, but it could have been much better.

try {
    $myDataAttr = $crawler->filter('[data-my-attr]')->attr('data-my-attr);
} catch (InvalidArgumentException $e) {
    $myDataAttr = null;
}

I hoped something like this would work, but unfortunately… it doesn't:

$myDataAttr = $crawler->filter('[data-my-attr]')?->attr('data-my-attr);