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

Already on GitHub? Sign in to your account

Dispatch error should be preventable #6024

Merged
merged 3 commits into from Apr 14, 2014

Conversation

Projects
None yet
2 participants
Member

Ocramius commented Mar 22, 2014

Please see the tests.

It is currently impossible for a listener of Zend\Mvc\MvcEvent::EVENT_DISPATCH_ERROR to do following:

$event->setError(''); // resetting app state (not an error)
$event->setRouteMatch(...); // setting a route match (allows for redirecting to a controller in case of routing failure)
$event->stopPropagation();

This PR fixes that

@Ocramius Ocramius added the Mvc label Mar 22, 2014

Member

Ocramius commented Mar 22, 2014

@weierophinney assigning you since you probably know why that early return $this; was in place.

@Ocramius Ocramius added the enhancement label Apr 2, 2014

@weierophinney weierophinney commented on the diff Apr 14, 2014

library/Zend/Mvc/Application.php
@@ -306,7 +306,6 @@ public function run()
if ($event->getError()) {
return $this->completeRequest($event);
}
@weierophinney

weierophinney Apr 14, 2014

Owner

I'd argue that if we're not going to return out of the parent conditional, this conditional can be removed, as it's repeated immediately below.

Owner

weierophinney commented Apr 14, 2014

@weierophinney assigning you since you probably know why that early return $this; was in place.

Honestly, I cannot quite remember. On review today, I don't see any good reason why, and clearly that use case is not covered by the tests, as your change doesn't lead to new failures.

@weierophinney weierophinney added this to the 2.3.1 milestone Apr 14, 2014

weierophinney added a commit that referenced this pull request Apr 14, 2014

Merge pull request #6024 from Ocramius/hotfix/mvc-dispatch-error-shou…
…ld-be-preventable

Dispatch error should be preventable

weierophinney added a commit that referenced this pull request Apr 14, 2014

weierophinney added a commit that referenced this pull request Apr 14, 2014

@weierophinney weierophinney merged commit bb0be6d into zendframework:master Apr 14, 2014

1 check passed

default The Travis CI build passed
Details

weierophinney added a commit that referenced this pull request Apr 14, 2014

@Ocramius Ocramius deleted the Ocramius:hotfix/mvc-dispatch-error-should-be-preventable branch Apr 14, 2014

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