Exceptions thrown in chained promises are wrapped

[thewilkybarkid]

Q	A
Bug?	yes
New Feature?	no
Version	v1.1.1

Actual Behavior

When chaining a promise and throwing an exception, the exception is wrapped inside an Exception with the message Invalid exception returned from Guzzle6.

Expected Behavior

That my exception is thrown directly.

Steps to Reproduce

$httpClient->sendAsyncRequest(new Request('GET', 'https://www.example.com/'))
    ->then(function (ResponseInterface $response) {
        throw new \Exception('Some problem');
    })
    ->wait();

... results in a RuntimeException with the message Invalid exception returned from Guzzle6 (with my Exception as previous).

Possible Solutions

The exception wrapping only makes sense when the actual request is being made (anything beyond that is not Guzzle). Either then return a different promise type in Promise::then()

guzzle6-adapter/src/Promise.php

Line 79 in a56941f

return new static($this->promise->then($onFulfilled, $onRejected), $this->request);

or stop catching any exception

guzzle6-adapter/src/Promise.php

Lines 64 to 68 in a56941f

	} elseif ($reason instanceof \Exception) {
	$this->exception = new \RuntimeException('Invalid exception returned from Guzzle6', 0, $reason);
	} else {
	$this->exception = new \UnexpectedValueException('Reason returned from Guzzle6 must be an Exception', 0, $reason);
	}

(Related, this will also hide any fatal errors (eg TypeError) behind the meaningless 'Reason returned from Guzzle6 must be an Exception' exception.)

[Nyholm]

@joelwurtz Can I get your opinion on this?

[joelwurtz]

That's because we only allow HttpException / ResponseInterface in our promise chain, if you want to deal with others types you have to return your own promise implementation (or an existing one) that does not limit you to those types.

However there is indeed some errors, (like relying on \Exception instead of \Throwable), and also we should return HttpException (and not \RuntimeException | \UnexpectedValueException)

[thewilkybarkid]

Not according to https://github.com/php-http/promise/blob/1cc44dc01402d407fc6da922591deebe4659826f/src/Promise.php#L64-L66.

I don't understand the reasoning for limiting the type. You're suggesting turning it back into a Guzzle (or whatever) promise?

[murilozilli]

Has this moved forward at all? Having issues with request exceptions on promises not being rejected and not being catched by catch (RequestException $re), I am not sure this is why.

#0 /app/vendor/guzzlehttp/guzzle/src/Middleware.php(66): GuzzleHttp\Exception\RequestException::create(Object(GuzzleHttp\Psr7\Request), Object(GuzzleHttp\Psr7\Response))
#1 /app/vendor/guzzlehttp/promises/src/Promise.php(203): GuzzleHttp\Middleware::GuzzleHttp\{closure}(Object(GuzzleHttp\Psr7\Response))
#2 /app/vendor/guzzlehttp/promises/src/Promise.php(156): GuzzleHttp\Promise\Promise::callHandler(1, Object(GuzzleHttp\Psr7\Response), Array)
#3 /app/vendor/guzzlehttp/promises/src/TaskQueue.php(47): GuzzleHttp\Promise\Promise::GuzzleHttp\Promise\{closure}()
#4 /app/vendor/guzzlehttp/promises/src/Promise.php(246): GuzzleHttp\Promise\TaskQueue->run(true)
#5 /app/vendor/guzzlehttp/promises/src/Promise.php(223): GuzzleHttp\Promise\Promise->invokeWaitFn()
#6 /app/vendor/guzzlehttp/promises/src/Promise.php(267): GuzzleHttp\Promise\Promise->waitIfPending()
#7 /app/vendor/guzzlehttp/promises/src/Promise.php(225): GuzzleHttp\Promise\Promise->invokeWaitList()
#8 /app/vendor/guzzlehttp/promises/src/Promise.php(62): GuzzleHttp\Promise\Promise->waitIfPending()
#9 /app/vendor/guzzlehttp/guzzle/src/Client.php(131): GuzzleHttp\Promise\Promise->wait()
#10 /app/vendor/guzzlehttp/guzzle/src/Client.php(89): GuzzleHttp\Client->request('get', 'someurl/123', Array)

Line 203 of the promise(Stack trace #1) is $promise->resolve($handler[$index]($value));:

            if (isset($handler[$index])) {
                $promise->resolve($handler[$index]($value));
            } elseif ($index === 1) {
                // Forward resolution values as-is.
                $promise->resolve($value);
            } else {
                // Forward rejections down the chain.
                $promise->reject($value);
            }