Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions src/Illuminate/Pipeline/Pipeline.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ class Pipeline implements PipelineContract
*/
protected $passable;

/**
* The callback to be executed on failure pipeline.
*
* @var Closure
*/
protected $onFailure;

/**
* The array of class pipes.
*
Expand Down Expand Up @@ -182,6 +189,10 @@ protected function carry()

return $this->handleCarry($carry);
} catch (Throwable $e) {
if ($this->onFailure) {
return call_user_func($this->onFailure, $pipe);
Copy link
Contributor

@rodrigopedra rodrigopedra Jun 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to also send the exception to the onFailure method, for logging purposes, for example.

I'd also suggest to send it as the first parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have any opinion on that, let's see what will Taylor say.

Copy link
Contributor

@michael-rubel michael-rubel Jun 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rezaamini-ir is usage of call_user_func() over $callable() still makes sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michael-rubel Didn't get it, what's wrong with call_user_func?
Could you please be more specific?

Copy link
Contributor

@michael-rubel michael-rubel Jun 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rezaamini-ir I meant using of more modern approach ($this->onFailure)($pipe) instead of call_user_func($this->onFailure, $pipe).
Look at this line: https://github.com/michael-rubel/laravel-enhanced-pipeline/blob/main/src/Pipeline.php#L236

}

return $this->handleException($passable, $e);
}
};
Expand Down Expand Up @@ -244,6 +255,19 @@ public function setContainer(Container $container)
return $this;
}

/**
* Set callback to be executed on failure pipeline.
*
* @param Closure $catch
* @return $this
*/
public function onFailure(Closure $callback)
{
$this->onFailure = $callback;

return $this;
}

/**
* Handle the value returned from each pipe before passing it to the next.
*
Expand Down
22 changes: 22 additions & 0 deletions tests/Pipeline/PipelineTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,20 @@ public function testPipelineThrowsExceptionOnResolveWithoutContainer()
});
}

public function testExceptionIsHandledByOnFailureMethodInPipeline()
{
$result = (new Pipeline(new Container))
->send('data')
->through(PipelineWithException::class)
->onFailure(function () {
return 'error';
})->then(function ($piped) {
return $piped;
});

$this->assertEquals('error', $result);
}

public function testPipelineThenReturnMethodRunsPipelineThenReturnsPassable()
{
$result = (new Pipeline(new Container))
Expand Down Expand Up @@ -279,3 +293,11 @@ public function handle($piped, $next, $parameter1 = null, $parameter2 = null)
return $next($piped);
}
}

class PipelineWithException
{
public function handle($piped, $next)
{
throw new \Exception('Foo');
}
}