Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Commit

Permalink
fix(core): Add custom 400 and 404 error messages (#1547)
Browse files Browse the repository at this point in the history
* Added 400 and 404 custom error messages

* nicer error message views

* Sign Up & Sign In error responses

Changed the error responses returned from the Sign Up & Sign In API
calls to use 422 rather than 400.

For insight into why this change was made:
#1510 (comment)

For reference on why to use 422 over 400:
https://www.bennadel.com/blog/2434-http-status-codes-for-invalid-data-400-vs-422.htm
  • Loading branch information
mleanos authored Oct 8, 2016
1 parent 8645b24 commit 6be12f8
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 9 deletions.
18 changes: 16 additions & 2 deletions modules/core/client/config/core.client.routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,31 @@
.state('not-found', {
url: '/not-found',
templateUrl: 'modules/core/client/views/404.client.view.html',
controller: 'ErrorController',
controllerAs: 'vm',
params: {
message: function($stateParams) {
return $stateParams.message;
}
},
data: {
ignoreState: true,
pageTitle: 'Not-Found'
pageTitle: 'Not Found'
}
})
.state('bad-request', {
url: '/bad-request',
templateUrl: 'modules/core/client/views/400.client.view.html',
controller: 'ErrorController',
controllerAs: 'vm',
params: {
message: function($stateParams) {
return $stateParams.message;
}
},
data: {
ignoreState: true,
pageTitle: 'Bad-Request'
pageTitle: 'Bad Request'
}
})
.state('forbidden', {
Expand Down
18 changes: 18 additions & 0 deletions modules/core/client/controllers/error.client.controller.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
(function () {
'use strict';

angular
.module('core')
.controller('ErrorController', ErrorController);

ErrorController.$inject = ['$stateParams'];

function ErrorController($stateParams) {
var vm = this;
vm.errorMessage = null;

// Display custom message if it was set
if ($stateParams.message) vm.errorMessage = $stateParams.message;
}
}());

Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
function responseError(rejection) {
if (!rejection.config.ignoreAuthModule) {
switch (rejection.status) {
case 400:
$injector.get('$state').go('bad-request', { message: rejection.data.message });
break;
case 401:
// Deauthenticate the global user
Authentication.user = null;
Expand All @@ -25,6 +28,9 @@
case 403:
$injector.get('$state').transitionTo('forbidden');
break;
case 404:
$injector.get('$state').go('not-found', { message: rejection.data.message });
break;
}
}
// otherwise, default behaviour
Expand Down
7 changes: 5 additions & 2 deletions modules/core/client/views/400.client.view.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
<h1>Bad Request</h1>
<div class="page-header">
<h1>Bad Request</h1>
</div>
<div class="alert alert-danger" role="alert">
<span class="glyphicon glyphicon-exclamation-sign" aria-hidden="true"></span>
<span class="sr-only">Error:</span>
You made a bad request
<span ng-if="vm.errorMessage" ng-bind="vm.errorMessage"></span>
<span ng-if="!vm.errorMessage">You made a bad request</span>
</div>
4 changes: 3 additions & 1 deletion modules/core/client/views/403.client.view.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
<h1>Forbidden</h1>
<div class="page-header">
<h1>Forbidden</h1>
</div>
<div class="alert alert-danger" role="alert">
<span class="glyphicon glyphicon-exclamation-sign" aria-hidden="true"></span>
<span class="sr-only">Error:</span>
Expand Down
7 changes: 5 additions & 2 deletions modules/core/client/views/404.client.view.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
<h1>Page Not Found</h1>
<div class="page-header">
<h1>Page Not Found</h1>
</div>
<div class="alert alert-danger" role="alert">
<span class="glyphicon glyphicon-exclamation-sign" aria-hidden="true"></span>
<span class="sr-only">Error:</span> Page Not Found
<span ng-if="vm.errorMessage" ng-bind="vm.errorMessage"></span>
<span ng-if="!vm.errorMessage">Page Not Found</span>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ exports.signup = function (req, res) {
// Then save the user
user.save(function (err) {
if (err) {
return res.status(400).send({
return res.status(422).send({
message: errorHandler.getErrorMessage(err)
});
} else {
Expand All @@ -55,7 +55,7 @@ exports.signup = function (req, res) {
exports.signin = function (req, res, next) {
passport.authenticate('local', function (err, user, info) {
if (err || !user) {
res.status(400).send(info);
res.status(422).send(info);
} else {
// Remove sensitive data before login
user.password = undefined;
Expand Down

2 comments on commit 6be12f8

@hyperreality
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure this one is ready? I don't have time to check right now, but I recall that there were lots of 400 errors fired by the app for other purposes, that we don't necessarily want to redirect.

@mleanos
Copy link
Member Author

@mleanos mleanos commented on 6be12f8 Oct 9, 2016

Choose a reason for hiding this comment

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

@hyperreality I'll look into it. Thanks.

Please sign in to comment.