[11.x] retry func - catch "Throwable" instead of Exception

[sethsandaru]

Hi team,

rescue has been catching Throwable for a while but not for retry. An Error would easily break the retry helper.

This PR updates that.

[nunomaduro]

This is a breaking change, so needs to target 12.x. Before doing anything, wait for Taylor's review.

[sethsandaru]

thanks, good to know 😄

[sethsandaru]

@taylorotwell please have a look and let us know if this should be targeted to master

[devajmeireles]

@sethsandaru , why not this?

catch (Exception|Throwable $e) {

[sethsandaru]

@devajmeireles it's unnecessary to do so, Exception implements Throwable and using union, it still introduces breaking changes

[antonkomarev]

@sethsandaru what is the reason to retry errors? Errors happen on programming mistakes. For example, if there is TypeError, you should just interrupt all execution and go fix a code.

[antonkomarev]

@taylorotwell @driesvints are you sure this is a good idea? Error types are not meant to be retried. It's a sign of a persistent issue in the code, not runtime issues which may fail eventually.

For example, I will pass the string to the method where the param must be an integer. If we will retry this call 10 times, this code will fail 10 times.

And as @nunomaduro mentioned earlier, this is a breaking change.

[sethsandaru]

@sethsandaru what is the reason to retry errors? Errors happen on programming mistakes. For example, if there is TypeError, you should just interrupt all execution and go fix a code.

@antonkomarev from my projects, I want to retry at least 1 regardless of Errors/Exceptions (libraries, low-level, etc). We'd never know when Error/Exception happens and there would be "Error" on runtime (3rd-party stuff, WS calls, internal, etc)

This change would somehow make retry consistent with the retry from Queue Job command - if you specify the retry times, it will always retry on any Error/Exception

Stuff like you mentioned "TypeError", it's already been battle-tested via unit testing & strict validation in my project. I guess other projects would follow this as well.

[antonkomarev]

@sethsandaru The queue needs to handle it to mark job as failed. Because code may be changed between jobs handling. And we could repeat execution later when code will be fixed.

On the other side retry helper will never receive a modified code between tries, because all iterations are done in a single process.

[driesvints]

Maybe function retry($times, callable $callback, $sleepMilliseconds = 0, $when = null, string $errorType = Exception::class) so you can pass whatever you want to $errorType could help?

retry(3, fn () => 'foo', errorType: Throwable::class)