Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timeouts no longer throw TimeoutException #123

Closed
AllenJB opened this issue Jan 17, 2019 · 3 comments
Closed

Timeouts no longer throw TimeoutException #123

AllenJB opened this issue Jan 17, 2019 · 3 comments

Comments

@AllenJB
Copy link

AllenJB commented Jan 17, 2019

return \React\Promise\Timer\timeout($deferred->promise(), $timeout, $this->loop)->then(null, function ($e) {
if ($e instanceof TimeoutException) {
throw new \RuntimeException(
'Request timed out after ' . $e->getTimeout() . ' seconds'
);
}
throw $e;
});

Related to 32de457

Timeouts no longer throw a TimeoutException, instead throwing a generic RuntimeException. This makes it more difficult to programmatically handle timeouts.

Could this change be reverted or is there a specific reason for it? Could the code throw a more specific exception instead please?

@clue
Copy link
Owner

clue commented Apr 3, 2019

@AllenJB Thanks for reporting, I understand where you're coming from and can see some use case for this 👍

Out of interest, can you provide a gist / example to highlight how you plan to use this?

Also, afaict this project never threw a TimeoutException unless I'm mistaken? This means this is not about what "no longer happens", but more about a future addition?

Are you aware of any similar APIs that handle this with a specific timeout exception? I'd like to flesh out a proper API and see how this is commonly used before jumping on the implementation.

It's my understanding a timeout exception is one possible source of problems and I'm not currently sold on the idea that this particular problem is "special" in a way that it warrants some special exception logic.

@clue
Copy link
Owner

clue commented May 17, 2019

Ping @AllenJB, can you update the status here, does the problem persist or did you mange to resolve this in the meantime?

@clue
Copy link
Owner

clue commented Jun 19, 2019

I'm having to close this for now as it hasn't received any input in a while and it's unlikely this will get traction any time soon. Please come back with more details if you think this is still relevant and we can reopen this +1

Thank you for your effort nonetheless, keep it up! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants