Skip to content

Commit

Permalink
rethrow exceptions in case of domain exceptions/errors
Browse files Browse the repository at this point in the history
  • Loading branch information
@fduch
fduch committed Feb 7, 2017
1 parent edcc016 commit 4a7876b
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 27 deletions.
17 changes: 14 additions & 3 deletions Actions/TransitionApplier.php
View file
Expand Up @@ -70,25 +70,30 @@ public function applyTransition(WorkflowContext $workflowContext, $transition)
*/
public function applyTransitions(WorkflowContext $workflowContext, array $transitions = [], $cascade = false)
{
if (!$transitions) {
return;
}

$workflow = $workflowContext->getWorkflow();
$subject = $workflowContext->getSubject();
$loggerContext = $workflowContext->getLoggerContext();

$this->logger->debug('Resolved workflow for subject', $loggerContext);

$applied = false;
foreach ($transitions as $transition) {
try {
// We do not call Workflow:can method here due to performance reasons in order to prevent
// execution of all the doCan-listeners (guards) in case when transition can be applied.
// Therefore we catch LogicException and interpreting it as case when transition can not be applied
$workflow->apply($subject, $transition);
$this->logger->info(sprintf('Workflow successfully applied transition "%s"', $transition), $loggerContext);

$applied = true;
if (!$cascade) {
break;
}
} catch (WorkflowLogicException $e) {
// transition cannot be applied because of it is not allowed
// transition cannot be applied because it is not allowed
$this->logger->info(
sprintf('Workflow transition "%s" cannot be applied due to it is not allowed', $transition),
$loggerContext
Expand All @@ -98,12 +103,18 @@ public function applyTransitions(WorkflowContext $workflowContext, array $transi
sprintf('Workflow cannot apply transition "%s" due to exception. Details: %s', $transition, $e->getMessage()),
$loggerContext
);
throw $e;
} catch (Throwable $e) {
$this->logger->critical(
sprintf('Workflow cannot apply transition "%s" due to exception. Details: %s', $transition, $e->getMessage()),
sprintf('Workflow cannot apply transition "%s" due to error. Details: %s', $transition, $e->getMessage()),
$loggerContext
);
throw $e;
}
}

if (!$applied) {
$this->logger->warning('All transitions to apply are not allowed', $loggerContext);
}
}
}
37 changes: 22 additions & 15 deletions Tests/Actions/TransitionApplierTest.php
View file
Expand Up @@ -24,13 +24,16 @@ class TransitionApplierTest extends PHPUnit_Framework_TestCase
/**
* @dataProvider exceptionAndLogLevelProvider
*/
public function testUnavailabilityToPerformTransitionIsReportedByLogger(\Exception $exception, $loggerLevel)
public function testUnavailabilityToPerformTransitionIsReportedByLogger(\Exception $exception, $loggerLevel, $throw = true)
{
$transitionName = "t1";
$subject = new \StdClass();

$workflow = $this->getMockBuilder(Workflow::class)->disableOriginalConstructor()->getMock();
$workflow->expects(self::once())->method('apply')->with($subject, $transitionName)->willThrowException($exception);
if ($throw) {
$this->setExpectedException(get_class($exception));
}

$workflowContext = new WorkflowContext($workflow, $subject, "id");

Expand All @@ -49,34 +52,38 @@ public function exceptionAndLogLevelProvider()
{
return [
// workflow logic exception should trigger info-level logging
[new LogicException(), "info"],
[new LogicException(), "info", false],
[new \Exception(), "error"]
];
}

/**
* @dataProvider transitionEnvironmentProvider
*/
public function testSeveralTransitionsHandledCorrectly(array $transitions = [], array $appliedTransitions = [], $cascade = false)
public function testSeveralTransitionsHandledCorrectly(array $transitions = [], array $transitionsToBeTriedToApply = [], $cascade = false)
{
$workflowName = "w1";
$subject = new \StdClass();

$workflow = $this->getMockBuilder(Workflow::class)->disableOriginalConstructor()->getMock();

$appliedTransitionCount = count($appliedTransitions);

$callIndex = 0;
$transitionsToApplyCount = 0;
$workflow->expects(self::at($callIndex))->method('getName');
$callIndex++;
foreach ($appliedTransitions as $transitionName => $applicationResult) {
foreach ($transitionsToBeTriedToApply as $transitionName => $applicationResult) {
$invocationMocker = $workflow->expects(self::at($callIndex))->method('apply')->with($subject, $transitionName);
if ($applicationResult instanceof \Exception) {
$invocationMocker->willThrowException($applicationResult);
$transitionsToApplyCount++;
if (is_array($applicationResult)) {
$throw = $applicationResult[1];
$exception = $applicationResult[0];
$invocationMocker->willThrowException($exception);
if ($throw) {
$this->setExpectedException(get_class($exception));
break;
}
}
$callIndex++;
}
$workflow->expects($this->exactly($appliedTransitionCount))->method('apply');
$workflow->expects($this->exactly($transitionsToApplyCount))->method('apply');

$workflowContext = new WorkflowContext($workflow, $subject, "id");

Expand All @@ -92,10 +99,10 @@ public function transitionEnvironmentProvider()
{
return [
[['t1', 't2'], ['t1' => true]],
[['t1', 't2'], ['t1' => new \Exception(), 't2' => true], true],
[['t1', 't2'], ['t1' => new \Exception(), 't2' => true], false],
[['t1', 't2'], ['t1' => new LogicException(), 't2' => true], true],
[['t1', 't2'], ['t1' => new LogicException(), 't2' => true], false],
[['t1', 't2'], ['t1' => [new \Exception(), true], 't2' => true], true],
[['t1', 't2'], ['t1' => [new \Exception(), true], 't2' => true], false],
[['t1', 't2'], ['t1' => [new LogicException(), false], 't2' => true], true],
[['t1', 't2'], ['t1' => [new LogicException(), false], 't2' => true], false],
];
}
}
17 changes: 11 additions & 6 deletions Trigger/Event/AbstractListener.php
View file
Expand Up @@ -143,29 +143,34 @@ final public function dispatchEvent(Event $event, $eventName)
abstract protected function handleEvent($eventName, Event $event, $eventConfigForWorkflow, WorkflowContext $workflowContext);

/**
* Allows to execute any listener callback with internal errors and exceptions caught
* in order to make possible next execution
* Allows to execute any listener callback with control of internal errors and exceptions handling.
* For now all the errors and exceptions are logged and rethrown.
* There is an ability to configure exception catching in the future in order to make possible next execution.
*
* @param \Closure $closure closure to be executed safely
* @param string $eventName event name
* @param WorkflowContext $workflowContext workflow context
* @param string $activity description of the current listener activity (required for logging
* purposes)
*
* @throws Exception|Throwable in case of failure
*/
protected function executeSafely(\Closure $closure, $eventName, WorkflowContext $workflowContext, $activity = 'react')
protected function execute(\Closure $closure, $eventName, WorkflowContext $workflowContext, $activity = 'react')
{
try {
call_user_func($closure);
} catch (Exception $e) {
$this->logger->error(
sprintf('Cannot %s on event "%s". Details: %s', $activity, $eventName, $e->getMessage()),
sprintf('Cannot %s on event "%s" due to exception. Details: %s', $activity, $eventName, $e->getMessage()),
$workflowContext->getLoggerContext()
);
throw $e;
} catch (Throwable $e) {
$this->logger->critical(
sprintf('Cannot %s on event "%s". Details: %s', $activity, $eventName, $e->getMessage()),
sprintf('Cannot %s on event "%s" due to error. Details: %s', $activity, $eventName, $e->getMessage()),
$workflowContext->getLoggerContext()
);
throw $e;
}
}

Expand Down Expand Up @@ -200,7 +205,7 @@ private function retrieveSubjectFromEvent(Event $event, $eventName, $workflowNam
}
} catch (Exception $e) {
$error = sprintf(
"Cannot retrieve subject from event '%s' by evaluating expression '%s'. Error: '%s'. Please check retrieving expression",
"Cannot retrieve subject from event '%s' by evaluating expression '%s'. Exception: '%s'. Please check retrieving expression",
$eventName,
$subjectRetrievingExpression,
$e->getMessage()
Expand Down
2 changes: 1 addition & 1 deletion Trigger/Event/ActionListener.php
View file
Expand Up @@ -79,7 +79,7 @@ protected function handleEvent($eventName, Event $event, $eventConfigForWorkflow
$actions = $this->prepareActions($actions, $event, $workflowContext);

foreach ($actions as $action) {
$this->executeSafely(
$this->execute(
function () use ($workflowContext, $action) {
$this->actionExecutor->execute($workflowContext, $action->getName(), $action->getArguments());
},
Expand Down
2 changes: 1 addition & 1 deletion Trigger/Event/ExpressionListener.php
View file
Expand Up @@ -72,7 +72,7 @@ protected function handleEvent($eventName, Event $event, $eventConfigForWorkflow
{
$expression = $this->supportedEventsConfig[$eventName][$workflowContext->getWorkflow()->getName()]['expression'];

$this->executeSafely(
$this->execute(
function () use ($expression, $event, $workflowContext) {
$this->actionLanguage->evaluate($expression, ['event' => $event, 'workflowContext' => $workflowContext]);
},
Expand Down
2 changes: 1 addition & 1 deletion Trigger/Event/SchedulerListener.php
View file
Expand Up @@ -80,7 +80,7 @@ protected function handleEvent($eventName, Event $event, $eventConfigForWorkflow
$actions = $this->prepareActions($actions, $event, $workflowContext, true);

foreach ($actions as $scheduledAction) {
$this->executeSafely(
$this->execute(
function () use ($workflowContext, $scheduledAction) {
$this->actionScheduler->scheduleAction($workflowContext, $scheduledAction);
},
Expand Down

0 comments on commit 4a7876b

Please sign in to comment.