Search code examples
javascriptangularjsobjecterror-handlingangular-factory

How to return $http.post() object with factory function?


When I tried to return a $http.post() object from a factory function with Angular, it throws me this error:

enter image description here

Below is my code:

AngularUtils.js

Fenrir.factory('AngularUtils', function ($http, $log, $window) {

var AngularUtils = {
    save: function(url, obj, errors, not_show_error_toast) {            
        not_show_error_toast = typeof not_show_error_toast !== 'undefined' ? not_show_error_toast : false;
        loaderShow();
        if (angular.isDefined(obj.id)) {
            return $http.put(url + obj.id + '/', obj).
                then(function onSuccess(response) {
                    loaderHide();
                    angular.extend(obj, response);
                    errors = {};
                }), function onError(response) {
                    loaderHide();
                    handleErrors(response, status, errors, not_show_error_toast);
                };
        } else {
            return this.create(url, obj, errors, not_show_error_toast);
        }
    },
    create: function(url, obj, errors, not_show_error_toast) {            
        not_show_error_toast = typeof not_show_error_toast !== 'undefined' ? not_show_error_toast : false;
        return $http.post(url, obj).
            then(function onSuccess(response) {
                loaderHide();
                angular.extend(obj, response);
                errors = {};
            }), function onError(response) {
                loaderHide();
                handleErrors(response, status, errors, not_show_error_toast);
            };
    },
}

   return AngularUtils
})

Admin.js

var Fenrir = angular.module('Fenrir', []);

Fenrir.controller('AdminController', function ($scope, $http, AngularUtils) {
    $scope.createScreen = function () {
        AngularUtils.save('/admin/saveScreen', $scope.record, '', '').then(function (hubs) {
            $scope.hubs = hubs;
            console.log('In ctrl ' + $scope.hubs);
        });
    }
})

How do I return the $http.post() object properly?


Solution

  • Looking closely at the error message:

    TypeError: AngularUtils.save(...).then is not a function
    

    This is saying that there is no function named then() that is returned from AngularUtils.save(...). Which indicates that the save() function did execute, but it did not return a promise as expected.

    So your factory method did execute, but because you did not return a promise we can only speculate as to which of the pathways the execution finally resulted in.

    Update: syntax error in original script

    In trying to justify my solution for you, I found that your $put().then() response handler had a syntax issue, there was a bracket in the wrong place, the onError function is supposed to be the second argument to the then function, not after it. Your code should look like this:

    return $http.put(url + obj.id + '/', obj).
                then(function onSuccess(response) {
                    loaderHide();
                    angular.extend(obj, response);
                    errors = {};
                }, function onError(response) {
                    loaderHide();
                    handleErrors(response, status, errors, not_show_error_toast);
                });
    

    Subtle, but important, if you do not have strict compilation on, then the factory function was simply not compiled correctly, which could cause your error.


    In factory methods where one or some pathways will return async promise deferrals (futures) then the default pattern, although verbose, is to return a single deferral wrapped around the whole method execution, then for each of the response pathways you can choose to resolve or reject the response through the original promise that was returned from the function.

    there are different libraries and techniques to do this, see AngularJs and Promises with the $http Service by Rick Strahl for a walkthrough of the common implementations.

    save: function(url, obj, errors, not_show_error_toast) {            
        var defer = $.Deferred();
    
        not_show_error_toast = typeof not_show_error_toast !== 'undefined' ? not_show_error_toast : false;
    
        if (angular.isDefined(obj.id)) {
            loaderShow(); // NOTE: let create manage the loader start if it needs to
            $http.put(url + obj.id + '/', obj).
                then(function onSuccess(response) {
                    loaderHide();
                    // HACK: because .then is used instead of .success, you probably want the response.data, please review this
                    angular.extend(obj, response.data);
    
                    defer.resolve(obj);
                }, function onError(response) {
                    loaderHide();
                    handleErrors(response, status, errors, not_show_error_toast);
    
                    defer.reject(response);
                });
        } else {
            // If create is a deferral, it should be safe to return it directly
            return this.create(url, obj, errors, not_show_error_toast);
        }
    
        return defer.promise();
    },
    

    Note: Only call a start or show utility or state operation in the same scope that also calls the reciprocal end/hide/stop/close. The code can work if you do not follow this rule but it gets harder to maintain as your project evolves, for that reason I've moved the loaderShow(); call into the branch that we know will always call loaderHide(). This functional scope can not make the assumption that the create() function will eventually call loaderHide() for us.

    This is a common, but bad habbit we all need to grow out of ;)