Fix failure check for ParallelExecution

[thomascorthals]

An endpoint failure for one of the results with ParallelExecution should throw an HttpException just like it does for a regular request. The surest way to check this is the error number of the handle.

The null check in Curl::getResponse() is useless for this because curl_multi_getcontent() doesn't return null on failure (it does when CURLOPT_RETURNTRANSFER is false). Checking for an empty string doesn't distinguish between a failure and a successful HEAD request. Checking for the presence of actual response headers is quite cumbersome with ext_curl.

Changed the error check in the Http adapter too to keep the flow similar with the other adapters.

[thomascorthals]

An example to illustrate what has changed.

<?php

require_once(__DIR__.'/init.php');

htmlHeader();

// create a client instance and autoload the customize request plugin
$client = new Solarium\Client($adapter, $eventDispatcher, $config);
$client->getEndpoint()->setHost('server.invalid');

$query = $client->createSelect()->setQuery('inStock:true');

$parallel = $client->getPlugin('parallelexecution');
$parallel->addQuery('foobar', $query);

$results = $parallel->execute();

var_dump($results['foobar']);

htmlFooter();

With the current release:

object(Solarium\QueryType\Select\Result\Result)#11 (10) {
  ["response":protected]=>
  object(Solarium\Core\Client\Response)#15 (4) {
    ["headers":protected]=>
    array(1) {
      [0]=>
      string(13) "HTTP/1.1 0 OK"
    }
    ["body":protected]=>
    string(0) ""
    ["statusCode":protected]=>
    int(0)
    ["statusMessage":protected]=>
    string(2) "OK"
  }
  ["data":protected]=>
  NULL
  ["query":protected]=>
  object(Solarium\QueryType\Select\Query\Query)#8 (10) {
    ["options":protected]=>
    array(8) {
      ["handler"]=>
      string(6) "select"
      ["resultclass"]=>
      string(39) "Solarium\QueryType\Select\Result\Result"
      ["documentclass"]=>
      string(41) "Solarium\QueryType\Select\Result\Document"
      ["query"]=>
      string(12) "inStock:true"
      ["start"]=>
      int(0)
      ["rows"]=>
      int(10)
      ["fields"]=>
      string(7) "*,score"
      ["omitheader"]=>
      bool(true)
    }
    ["helper":protected]=>
    NULL
    ["params":protected]=>
    array(0) {
    }
    ["localParameters":"Solarium\Core\Query\AbstractQuery":private]=>
    object(Solarium\Core\Query\LocalParameters\LocalParameters)#14 (1) {
      ["parameters":"Solarium\Core\Query\LocalParameters\LocalParameters":private]=>
      array(0) {
      }
    }
    ["tags":protected]=>
    array(0) {
    }
    ["fields":protected]=>
    array(2) {
      ["*"]=>
      bool(true)
      ["score"]=>
      bool(true)
    }
    ["sorts":protected]=>
    array(0) {
    }
    ["filterQueries":protected]=>
    array(0) {
    }
    ["components":protected]=>
    array(0) {
    }
    ["componentTypes":protected]=>
    array(15) {
      ["morelikethis"]=>
      string(31) "Solarium\Component\MoreLikeThis"
      ["spellcheck"]=>
      string(29) "Solarium\Component\Spellcheck"
      ["suggest"]=>
      string(28) "Solarium\Component\Suggester"
      ["debug"]=>
      string(24) "Solarium\Component\Debug"
      ["spatial"]=>
      string(26) "Solarium\Component\Spatial"
      ["facetset"]=>
      string(27) "Solarium\Component\FacetSet"
      ["dismax"]=>
      string(25) "Solarium\Component\DisMax"
      ["edismax"]=>
      string(26) "Solarium\Component\EdisMax"
      ["highlighting"]=>
      string(44) "Solarium\Component\Highlighting\Highlighting"
      ["grouping"]=>
      string(27) "Solarium\Component\Grouping"
      ["distributedsearch"]=>
      string(36) "Solarium\Component\DistributedSearch"
      ["stats"]=>
      string(30) "Solarium\Component\Stats\Stats"
      ["queryelevation"]=>
      string(33) "Solarium\Component\QueryElevation"
      ["rerankquery"]=>
      string(30) "Solarium\Component\ReRankQuery"
      ["analytics"]=>
      string(38) "Solarium\Component\Analytics\Analytics"
    }
  }
  ["parsed":protected]=>
  bool(false)
  ["responseHeader":protected]=>
  NULL
  ["numfound":protected]=>
  NULL
  ["maxscore":protected]=>
  NULL
  ["nextcursormark":protected]=>
  NULL
  ["documents":protected]=>
  NULL
  ["components":protected]=>
  NULL
}

With the proposed changes:

object(Solarium\Exception\HttpException)#15 (9) {
  ["message":protected]=>
  string(76) "Solr HTTP error: HTTP request failed, Could not resolve host: server.invalid"
  ["string":"Exception":private]=>
  string(0) ""
  ["code":protected]=>
  int(0)
  ["file":protected]=>
  string(50) "C:\repos\solarium\src\Core\Client\Adapter\Curl.php"
  ["line":protected]=>
  int(57)
  ["trace":"Exception":private]=>
  array(2) {
    [0]=>
    array(6) {
      ["file"]=>
      string(68) "C:\repos\solarium\src\Plugin\ParallelExecution\ParallelExecution.php"
      ["line"]=>
      int(188)
      ["function"]=>
      string(11) "getResponse"
      ["class"]=>
      string(33) "Solarium\Core\Client\Adapter\Curl"
      ["type"]=>
      string(2) "->"
      ["args"]=>
      array(2) {
        [0]=>
        object(CurlHandle)#10 (0) {
        }
        [1]=>
        string(0) ""
      }
    }
    [1]=>
    array(6) {
      ["file"]=>
      string(59) "C:\repos\solarium\examples\test-parallelexecution-failure.php"
      ["line"]=>
      int(16)
      ["function"]=>
      string(7) "execute"
      ["class"]=>
      string(51) "Solarium\Plugin\ParallelExecution\ParallelExecution"
      ["type"]=>
      string(2) "->"
      ["args"]=>
      array(0) {
      }
    }
  }
  ["previous":"Exception":private]=>
  NULL
  ["statusMessage":protected]=>
  string(59) "HTTP request failed, Could not resolve host: server.invalid"
  ["body":protected]=>
  NULL
}

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cb4ea0a) 97.49% compared to head (29534ac) 97.49%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1099      +/-   ##
==========================================
- Coverage   97.49%   97.49%   -0.01%     
==========================================
  Files         389      389              
  Lines       10148    10141       -7     
==========================================
- Hits         9894     9887       -7     
  Misses        254      254              

Flag	Coverage Δ
unittests	97.49% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
src/Core/Client/Adapter/Curl.php	97.59% <100.00%> (-0.17%)	⬇️
src/Core/Client/Adapter/Http.php	100.00% <100.00%> (ø)
src/Plugin/ParallelExecution/ParallelExecution.php	100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[thomascorthals]

The reason I consider this a bugfix rather than a change is that the "empty" Result doesn't actually work.

Fatal error: Uncaught Solarium\Exception\UnexpectedValueException: Solr JSON response could not be decoded in C:\repos\solarium\src\Core\Query\Result\Result.php:124
Stack trace:
#0 C:\repos\solarium\src\QueryType\Select\ResponseParser.php(36): Solarium\Core\Query\Result\Result->getData()
#1 C:\repos\solarium\src\Core\Query\Result\QueryType.php(82): Solarium\QueryType\Select\ResponseParser->parse(Object(Solarium\QueryType\Select\Result\Result))
#2 C:\repos\solarium\src\QueryType\Select\Result\Result.php(96): Solarium\Core\Query\Result\QueryType->parseResponse()
#3 C:\repos\solarium\examples\test-parallelexecution-failure.php(18): Solarium\QueryType\Select\Result\Result->getNumFound()
#4 {main}
  thrown in C:\repos\solarium\src\Core\Query\Result\Result.php on line 124

If you don't check if you're dealing with a Result or an HttpException with my proposed fix, you'll see right away that that's the problem.

Fatal error: Uncaught Error: Call to undefined method Solarium\Exception\HttpException::getNumFound() in C:\repos\solarium\examples\test-parallelexecution-failure.php:18
Stack trace:
#0 {main}
  thrown in C:\repos\solarium\examples\test-parallelexecution-failure.php on line 18