Request Timeout exception

[ghost]

Currently if the request times out, a generic RuntimException will be the promise rejection reason. However when you have many requests running concurrently in the background, you do not know which one actually timed out, unless you add specific handling code, which is for background tasks not desired or of interest. Instead a specific global error handler is used to report failures (in my case Sentry).

I propose to introduce a new class RequestTimeoutException that inherits from RuntimeException, which has similar to ResponseException a getRequest method to get the timed out request. Doing this will allow us to be backwards compatible, but also we can get the failed request as further error information, regardless of the scope we're in and have direct access to the request or not.

Would you accept a PR for this change?

[clue]

@CharlotteDunois Thanks for reporting, I can definitely see some use for something like this 👍

That being said, I'm not sure this is limited to RequestTimeoutException only, so I wonder if this really makes a lot of sense to have in here. Additionally, is a "timeout" special in a way that makes it "deserve" a dedicated exception class? Is much gained over adding your specific error handler to include the relevant info yourself?

Similar cases have been suggested before (#123 but also friends-of-reactphp/mysql#96 and possibly others), so I'm open for input 👍

[ghost]

@clue Thanks for getting back to me.

That being said, I'm not sure this is limited to RequestTimeoutException only, so I wonder if this really makes a lot of sense to have in here. Additionally, is a "timeout" special in a way that makes it "deserve" a dedicated exception class?

Yes. It's by sense an error that can have external influence (i.e. network), which should be derived into a more specific exception class than a generic one that can mean anything.

Is much gained over adding your specific error handler to include the relevant info yourself?

That would mean adding a specific promise rejection handler everywhere, and if you require dependencies where the request isn't openly exposed (i.e. because it's a library internal request), you can't attach it there. Depending on how the failure is handled, you may be able to handle the error, but you still don't know which request. A dedicated exception with origin exposure would help here tracking down and pinning issues fast and efficiently.

Similar cases have been suggested before (#123 but also friends-of-reactphp/mysql#96 and possibly others), so I'm open for input 👍

#123 tried to re-introduce the TimeoutException from the promise-timer library, which the mentioned changeset overshadows by a generic RuntimeException. (32de457#diff-0a115916bfb9a531456af2e93caca732R91)

I'm not familiar with the MySQL PR background, but it seems to have a similar goal.

[clue]

Again thanks for bringing this up! I have to close this one as per #177 now that future development will focus on https://github.com/reactphp/http instead. If you feel this makes sense to port over, please file a new ticket over in the new repo and I'm happy to help review this 👍