Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Allow to set arbitrary status code for Exception strategy #3421

Merged

Conversation

bakura10
Copy link
Contributor

While working on ZfrRest, I realized Exception strategy always set the response status code to 500, even if another status code was set after an error (for instance in another listener).

This PR set 500 status code if the Response did not exist, and if it already exists, it sets it to the set status code, hence not always overriding it.

$e->setResponse($response);
}
$response->setStatusCode(500);
$response->setStatusCode($response->getStatusCode());
Copy link
Member

Choose a reason for hiding this comment

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

This line can be completely skipped, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

HA. Yeah, you're right =).

@@ -158,9 +158,10 @@ public function prepareExceptionViewModel(MvcEvent $e)
$response = $e->getResponse();
if (!$response) {
$response = new HttpResponse();
$response->setStatusCode(500);
$e->setResponse($response);
}
Copy link
Member

Choose a reason for hiding this comment

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

I would test for an empty status code as well; if it's empty, we want to set it. The problem, however, is that the status code is never empty by default; it defaults to 200. Perhaps we could test for a "200" status code, and reset in that case? or allow injecting a list of "allowed" exception status codes and/or a default to use in exceptions when the current code is not in that list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For error it's between 400 and 599. We could add a test on that and set it to 500 if it's outside those bounds.

But isn't it the task of other listeners or classes to set the appropriate error code ?

@ghost ghost assigned weierophinney Jan 14, 2013
@weierophinney weierophinney merged commit f7f9c41 into zendframework:develop Jan 14, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants