Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add overriden NotFoundHttpException to have access to the original response #92

Merged
merged 1 commit into from
May 4, 2017

Conversation

emodric
Copy link
Collaborator

@emodric emodric commented May 4, 2017

Now that #91 is done, it would greatly help to know what happened in legacy when a 404 error is received.

We have some siteaccesses that run in legacy mode set to false, but still display legacy 404 errors, so this would help facilitate that.

It is worthwhile to note that this behaviour (this custom 404 exception) would still be part of 2.0 release.

@emodric
Copy link
Collaborator Author

emodric commented May 4, 2017

Ping @andrerom @blankse

// we send an NotFoundHttpException to be able to trigger error page in Symfony stack.
if ($moduleResult['errorCode'] == 404) {
if ($this->notFoundHttpConversion) {
$errorMessage = isset($moduleResult['errorMessage']) ? $moduleResult['errorMessage'] : 'Not found';
throw new NotFoundHttpException($errorMessage);
$exception = new NotFoundHttpException($errorMessage);
Copy link
Contributor

Choose a reason for hiding this comment

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

can't you use custom ctor given it is custom exception?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, done!

@emodric emodric force-pushed the custom_not_found branch from 35080f1 to df75819 Compare May 4, 2017 08:52
@emodric emodric merged commit fd2f96d into master May 4, 2017
@emodric emodric deleted the custom_not_found branch May 4, 2017 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants