{"action":"opened","issue":{"url":"https://api.github.com/repos/userfrosting/UserFrosting/issues/783","repository_url":"https://api.github.com/repos/userfrosting/UserFrosting","labels_url":"https://api.github.com/repos/userfrosting/UserFrosting/issues/783/labels{/name}","comments_url":"https://api.github.com/repos/userfrosting/UserFrosting/issues/783/comments","events_url":"https://api.github.com/repos/userfrosting/UserFrosting/issues/783/events","html_url":"https://github.com/userfrosting/UserFrosting/issues/783","id":251338107,"number":783,"title":"ExceptionHandler should generate appropriate content type for generic responses","user":{"login":"alexweissman","id":5004534,"avatar_url":"https://avatars2.githubusercontent.com/u/5004534?v=4","gravatar_id":"","url":"https://api.github.com/users/alexweissman","html_url":"https://github.com/alexweissman","followers_url":"https://api.github.com/users/alexweissman/followers","following_url":"https://api.github.com/users/alexweissman/following{/other_user}","gists_url":"https://api.github.com/users/alexweissman/gists{/gist_id}","starred_url":"https://api.github.com/users/alexweissman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alexweissman/subscriptions","organizations_url":"https://api.github.com/users/alexweissman/orgs","repos_url":"https://api.github.com/users/alexweissman/repos","events_url":"https://api.github.com/users/alexweissman/events{/privacy}","received_events_url":"https://api.github.com/users/alexweissman/received_events","type":"User","site_admin":false},"labels":[{"id":646764537,"url":"https://api.github.com/repos/userfrosting/UserFrosting/labels/architecture","name":"architecture","color":"0052cc","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2017-08-18T19:30:16Z","updated_at":"2017-08-18T19:30:16Z","closed_at":null,"body":"I'm still not 100% satisfied with our exception-handling system.  Right now, the first thing that `ExceptionHandler::handle` does is to check if error debugging is currently enabled:\r\n\r\n```\r\n        if ($this->displayErrorDetails) {\r\n            $response = $this->renderDebugResponse();\r\n        } else {\r\n            // Write exception to log\r\n            $this->writeToErrorLog();\r\n\r\n            // Render generic error page\r\n            $response = $this->renderGenericResponse();\r\n        }\r\n```\r\n\r\n`renderDebugResponse` invokes the appropriate `Renderer` class to generate a debugging message based on the requested content type.  Oddly, some `Renderer` classes check the value of `displayErrorDetails` again in their logic, even though we've already done this check in `handle`.  Seems weird.\r\n\r\nAlso, `renderGenericResponse` doesn't do any checking of the requested content type at all!  It appears to always generate content for an HTML response, even if some other type was requested.\r\nOne thing we could do is change the `ErrorRenderer` interface so that it has `renderGeneric` and `renderDebug` methods, and call the appropriate method from within the exception handler.  Alternatively, we could replicate some functionality in `ExceptionHandler::renderGenericResponse` to check the content type and generate the appropriate type of response."}}
{"actor":{"display_login":"alexweissman"}}