Exceptions

[sagikazarmark]

Exceptions in promises

If I understand it well, in case of async requests the async request method should NEVER EVER throw an exception, but return and reject a promise.

This is currently not the case:
https://github.com/php-http/guzzle6-adapter/blob/master/src/Guzzle6Promise.php#L66

SPL or not

Although we agreed that SPL exceptions can be used to determine exceptions not closely related to the domain, we might want to reconsider this.

The point of the common Exception interface is that every exception can be caught. On the other side: creating a custom version of every SPL exception which implements our interface seems to be dumb.

The ideal solution would be creating domain exceptions for each case we would use an SPL one.

For example in this case:
https://github.com/php-http/guzzle6-adapter/blob/master/src/Guzzle6Promise.php#L66

This decision should take effect in the Promise interface as well: currently it is restricted to our Exception interface.

Update: Using anonymus classes would be a good solution, but that is only available in PHP 7. Maybe we could place custom extensions in one of our utils repo.

[joelwurtz]

If I understand it well, in case of async requests the async request method should NEVER EVER throw an exception, but return and reject a promise.

It's more complicated than that, you have 2 kinds of exception :

• An exception related to the worklow of the async, (i.e. network failure, socket timeout, error in client for example)
• Exception not related to the workflow (Like not respecting the specification, a parse / fatal exception in php, etc ....)

The first case is a reason and should be pass to the onRejected callback, the second one should throw when it happens

And so the example in guzzle is correct in my mind.

[dbu]

is this what other promise implementations do?

it seems to make sense to me that totally unexpected problems should not be handled by the chain of promises, but i am not a promise expert.

[sagikazarmark]

Makes sense I guess. Although I am not completely sure: what happens when an exception is thrown DURING the workflow, but not closely related to it? Is it a reason or not?

[joelwurtz]

This is also a major debate in the javascript community about if all exceptions are reason or only specific one.

IMO only exception not related to the "code" (timeout, bad server implementation, http things, ...) are reasons, other ones are errors from the developer and does not make sense as a reason, and so they should throw exception as implementation is broken.

[joelwurtz]

@sagikazarmark There is no difference between exception during the workflow or not as the promise has normally only a catch for specific exception.

[sagikazarmark]

I see. In this case what should we typehint in the async client and in the promise interface?

And BTW: What should we typehint in the HttpClient?

Currently the typehint says our Exception interface, which is not true: we throw Runtime and other exceptions as well. So should we stick to our interface? It would mean that we need custom extensions of the SPL Exceptions (which is probably not a big deal). With PHP 7 coming, we can use anonymus classes for this.

Or we can allow literally any exceptions to be thrown without a custom extension?

[joelwurtz]

We can allow to throw anything, however only exception which are an instance of our Exception will be used in then chain callaback, if it's not our exception the chain is broken (because something bad happen and there is no need to continue the chain)

[dbu]

i agree with joel that everything that is an exception that can sensibly be handled must implement the exception interface, but on setup errors and the like, there is no need for error handling so we don't care what is thrown. the phpdoc and doc should reflect this better, however.

[sagikazarmark]

Back to the original question: what exceptions should we allow or should we restrict them at all? Or just put to phpdocs: our exception for domain errors, and SPL exception for anything else?

[dbu]

imho everything that could reasonably be catched by the caller should implement our exception interface. we can extend spl if needed.
things like invalid setup that is never caught because it can not be dealt with at runtime can throw whatever exception makes sense (and technically won't matter as its not caught anyways).

[sagikazarmark]

So probably we shouldn't even mention it in the phpdoc?

What about async client. Currently no exception is mentioned there. Our exceptions should be available from the promise, but not thrown.

[dbu]

i would still mention the fact that exceptions can be thrown on setup errors and similar situations that "should never happen"