Request in exception

[R4c00n]

I added the request object to the ResponseException so it can be used for logging and debugging of errors is easier.

In theory this would be a major break, as the new constructor param is not optional.
Is this okay or should I maybe move it to the end and make it nullable?

[SimonFrings]

@R4c00n Thanks for opening this PR 👍

Yes, this will break current BC, but I'm still not quite convinced that this is something the project really needs. I can understand that you're left with more information for debugging purposes, but what is really gained here?

I don't want to say that this is unwanted, maybe there are some arguments on your side that will convince me otherwise. What do you think?

[R4c00n]

Hi @SimonFrings ,
Imho it's very useful to immediately see which request caused a response exception.
Otherwise it can be very hard to reproduce and investigate the error, especially if the bad response was returned by an external API.

As with the middleware feature I tried to do it in a similar way as Guzzle already does it here

[SimonFrings]

@R4c00n Like I said above, I can understand that it helps to have more insights on the causing request when a response exception is thrown, but I'm not sure if this is something for the current v1 of our HTTP component (especially if it breaks BC).

I know it's been a while since the last update in here and we're currently working towards ReactPHP v3, as you can also see in our roadmap ticket for HTTP. The plan is to first release an additional HTTP v1.10.0, as there are still some features interesting for both v1 and v3 and we can avoid doubling the work by drafting an additional release before creating the new 3.x branch.

With that said, we'll focus on getting the v1.10.0 out first, so we can then start adding the planned v3 features. The v3 roadmap for HTTP is not set in stone yet, so maybe it makes sense to keep this topic in mind while working on the next major release. We also talked about improving the overall error reporting while we're at it, but we have yet to look into ways to achieve this.

I really appreciate your work and we'll have a look at this while working on ReactPHP v3, but for now I think it's best to close this ticket. If you're currently in need of this feature, I'd suggest to use your implementation in a fork for the time being until it becomes part of our HTTP component 👍