Authentication - $http's .then() success callback called instead of error callback

Question

This may just come from a misunderstanding of how to best do authentication in a MEAN stack app, or my lack of knowledge in how promises and $http's .then() method works, but whenever I try to authenticate to my backend Node server with incorrect credentials, it is calling the success callback of $http's .then() method instead of the error callback. Here's my setup:

I'm using the jsonwebtoken and express-jwt packages, AngularJS interceptors to add the token the to request and check for status 401 responseErrors, a TokenService that sets/removes, etc JWTs, and a UserService to handle login, logout, etc.

From debugging, here's what's happening:

1. Request to login is sent out
2. Server catches request, looks for specified user and can't find them in database. Returns 401 error with JSON object including error message, etc.
3. HttpInterceptor engages the responseError method, correctly sees it's a status 401, removes any possible existing token, redirects to /login screen, and returns $q.reject(response).
4. UserService.login() correctly uses the error callback and does a return response.
5. Problem - the success callback in my login.js .login() method runs, instead of the second, error callback one. I have a feeling this has to do with what is discussed in this article about promise chaining, but my expertise has its limit right here and I can't quite understand what I should be doing next to tell the next callback in the chain that the previous one had an error...

Here's my setup:

Express:

authRoutes.js

authRoutes.post("/login", function (req, res) {

    User.findOne({username: req.body.username}, function (err, user) {
        if (err) res.status(500).send(err);
        if (!user) {
            res.status(401).send({success: false, message: "User with the provided username was not found"})
        } else if (user) {
            bcrypt.compare(req.body.password, user.password, function (err, match) {
                if (err) throw (err);
                if (!match) res.status(401).json({success: false, message: "Incorrect password"});
                else {
                    var token = jwt.sign(user, config.secret, {expiresIn: "24h"});
                    res.json({token: token, success: true, message: "Here's your token!"})
                }
            });
        }
    });
});

From debugging, when I log in with incorrect credentials, it's correctly hitting the res.status(401).send(...) line, so this part seems to be okay.

Angular:

app.js (includes HttpInterceptor)

var app = angular.module("TodoApp", ["ngRoute"]);

app.factory("AuthInterceptor", ["$q", "$location", "TokenService", function ($q, $location, TokenService) {
    return {
        request: function (config) {
            var token = TokenService.getToken();
            if (token) {
                config.headers = config.headers || {};
                config.headers.Authorization = "Bearer " + token
            }
            return config;
        },
        responseError: function (response) {
            if (response.status === 401) {
                TokenService.removeToken();
                $location.path("/login");
            }
            return $q.reject(response);
        }
    }
}]);

app.config(function ($routeProvider, $httpProvider) {
    $httpProvider.interceptors.push('AuthInterceptor');

    $routeProvider
        .when("/", {
            templateUrl: "landing/landing-page.html"
        });
});

userService.js

var app = angular.module("TodoApp");

app.service("UserService", ["$http", "TokenService", function ($http, TokenService) {

    this.signup = function (user) {
        return $http.post("http://localhost:8080/auth/signup", user).then(function (response) {
            return response;
        }, function (response) {
            return response;
        });
    };

    this.login = function (user) {
        return $http.post("http://localhost:8080/auth/login", user).then(function (response) {
            if (response.data.success) TokenService.setToken(response.data.token);
            return response;
        }, function (response) {
            return response;
        })
    };

    this.isAdmin = function (user) {
        return user.admin;
    };
}]);

login.js (Where the problem seems to manifest itself)

var app = angular.module("TodoApp");

app.config(function ($routeProvider) {
    $routeProvider
        .when("/login", {
            templateUrl: "auth/login.html",
            controller: "LoginController"
        })
});

app.controller("LoginController", ["$scope", "$http", "$location", "UserService", "TokenService", function ($scope, $http, $location, UserService, TokenService) {

    $scope.login = function (user) {
        UserService.login(user).then(function (response) {
            $location.path("/todo");
        }, function (response) {
            console.log("There was a problem: " + response);
        });
    }
}]);

That last part, UserService.login(user).then(function (response) { $location.path("/todo"); is the line that is running and trying to redirect the user to the list of Todo items, when I want it to run that console.log("There was a problem: " + response); line instead...

Like I said above, I have a feeling it has to do with chaining promises and how errors are handled partway through the chain instead of bubbling down through the chain. Not sure if I need to add a .catch() block like the site I mentioned above says to do. And even if that is the answer, I'm not entirely sure how to write that.

If there is a better way I should be organizing this, I'm definitely open to suggestions as well. I have to teach this to a class of students and want to make sure I'm teaching good practices.

Thanks in advance for the help!

Answer 1

Take a closer look at this part of your code:

this.login = function (user) {
    return $http.post("http://localhost:8080/auth/login", user).then(function (response) {
        if (response.data.success) TokenService.setToken(response.data.token);
        return response;
    }, function (response) {
        return response;
    })
}

Here you've provided error callback with a return value which is passed to the next callback in promise chain. The source of your confusion is that you still need to return rejected promise of throw from the callback if you want the error to propagate further. Otherwise, it effectively means that you've recovered from error situation and the next step in the flow will be success. This is what you have now.

In your case you either remove error callback altogether

return $http.post("http://localhost:8080/auth/login", user).then(function (response) {
    if (response.data.success) TokenService.setToken(response.data.token);
    return response;
});

... or make sure you return failed promise

return $http.post("http://localhost:8080/auth/login", user).then(function (response) {
    if (response.data.success) TokenService.setToken(response.data.token);
    return response;
}, function (response) {
    return $q.reject(response);
});

... or throw:

return $http.post("http://localhost:8080/auth/login", user).then(function (response) {
    if (response.data.success) TokenService.setToken(response.data.token);
    return response;
}, function (response) {
    throw new Error(response);
});

Answer 2

Have you tried to use $q.reject in error cases for then() calls?

E.g.

// remember to add $q to deps

this.login = function (user) {
    return $http.post("http://localhost:8080/auth/login", user).then(function (response) {
        if (response.data.success) TokenService.setToken(response.data.token);
        return response;
    }, function (response) {
        $q.reject(response);
    })
};

Related docs: https://docs.angularjs.org/api/ng/service/$q