[11.x] feat: Adding catch callback to Pipeline

[mathiasgrimm]

Adding a catch callback to the Pipeline

(new Pipeline(new Container))
    ->send($std)
    ->through([
        function ($std, $next) {
            return $next($std);
        },
        function ($std) {
            throw new Exception('My Exception: '.$std->value);
        },
    ])->catch(function ($std, Throwable $e) {
        $std->value = 100;

        return $std;
    })
    ->then(function ($std) {
        return $std;
    });

[morloderex]

Have you considered what should happen in cases where multiple calls to the catch method exists?

[taylorotwell]

Drafting pending comment above @mathiasgrimm

[mathiasgrimm]

Have you considered what should happen in cases where multiple calls to the catch method exists?

I works the same way as the finally method. If called more than once it will replace the previous callback.

[shaedrich]

Have you considered what should happen in cases where multiple calls to the catch method exists?

I works the same way as the finally method. If called more than once it will replace the previous callback.

So, if multiple callbacks would be wanted, both finally and catch could have this implemented in a separate PR, I assume 🤔

[mathiasgrimm]

Have you considered what should happen in cases where multiple calls to the catch method exists?

I works the same way as the finally method. If called more than once it will replace the previous callback.

So, if multiple callbacks would be wanted, both finally and catch could have this implemented in a separate PR, I assume 🤔

The Finally is already merged and released, including all tests. I only added an "out of scope" test for the finally method but this PR is 99% for the catch, and 100% implemented like the finally.
The additional test for the finally could be done in a separated PR.

[shaedrich]

Have you considered what should happen in cases where multiple calls to the catch method exists?

One thing, however, that would make sense for catch but not that much for finally is, calling the callback that matches the argument type:

(new Pipeline(new Container))
    ->send($std)
    ->through([
        function ($std, $next) {
            return $next($std);
        },
        function ($std) {
            throw new ExceptionB('My Exception: '.$std->value);
        },
    ])->catch(function ($std, ExceptionA $e) { // this is not called
        Log::error('Exception A', $std);;

        return $std;
    })->catch(function ($std, ExceptionB $e) { // this is called
        $std->value = 100;

        return $std;
    })->catch(function ($std, ExceptionB $e) { // this might also be called
        $std->value = 100;

        return $std;
    })->catch(function ($std, Throwable $e) { // default case if nothing else is matched
        $std->value = 100;

        return $std;
    })
    ->then(function ($std) {
        return $std;
    });

[mathiasgrimm]

Have you considered what should happen in cases where multiple calls to the catch method exists?

One thing, however, that would make sense for catch but not that much for finally is, calling the callback that matches the argument type:

(new Pipeline(new Container))
    ->send($std)
    ->through([
        function ($std, $next) {
            return $next($std);
        },
        function ($std) {
            throw new ExceptionB('My Exception: '.$std->value);
        },
    ])->catch(function ($std, ExceptionA $e) { // this is not called
        Log::error('Exception A', $std);;

        return $std;
    })->catch(function ($std, ExceptionB $e) { // this is called
        $std->value = 100;

        return $std;
    })->catch(function ($std, ExceptionB $e) { // this might also be called
        $std->value = 100;

        return $std;
    })->catch(function ($std, Throwable $e) { // default case if nothing else is matched
        $std->value = 100;

        return $std;
    })
    ->then(function ($std) {
        return $std;
    });

Interesting @shaedrich.

I've looked at other parts of the framework, such as:

1. \Illuminate\Bus\PendingBatch::catch
2. \Illuminate\Events\QueuedClosure::catch
3. \Illuminate\Foundation\Bus\PendingChain::catch

And they all allow for multiple catch callbacks.

I've also looked into the only implementation of finally except for the one in the Pipelines:

1. \Illuminate\Bus\PendingBatch::finally

It also uses an array for callbacks.

We need to be consistent throughout the entire framework. I think the first thing we need to address is to make sure the Pipeline finally[/catch] methods adhere to the existing implementations in the framework.

Secondly, I see your suggestion as a potentially good idea, but it would make sense to be a separate PR that would introduce this feature of "scoped catch" for all catch methods.

To summarize what I think needs to be done now:

1. Create a separate PR to introduce that extra test for the finally method that is present in this PR.
2. Create a separate PR that uses arrays for the finally callback
3. Rework this current PR so that it uses arrays for callbacks and extract the out-of-scope test into a separate PR.

What do you think @taylorotwell ?

[shaedrich]

Sounds good, @mathiasgrimm

btw, the scoped catch was inspired by how exception handling is done