CODEWITHSUNDEEP
javascriptnode.js
quielfala
quielfala

Reputation: 589

TypeError: .then is not a function

I am having an error when the user is not admin, it doesn't display my message "Not authorized to access this page" because instead I'm getting this error in the console:

    TypeError: productController.createProduct(...).then is not a function

I have a router set up like this:

    router.post('/create',auth.verify, (req, res)=>{

    const isAdmin = auth.decode(req.headers.authorization).isAdmin;

    productController.createProduct(req.body,isAdmin).then(result => res.send(result));})

And my controller:

module.exports.createProduct = (reqBody, isAdmin) => {
  if (isAdmin === true) {
    let newProduct = new Product({
      name: reqBody.name,
      description: reqBody.description,
      price: reqBody.price,
    });

    return newProduct.save().then((result, error) => {
      return error ? false : 'Successfully created a product';
    });
  } else {
    return 'Not authorized to access this page';
  }
};

Everything works fine though when the user is an admin. I wonder if this has something to do with how I return my message when the user is not admin. Because I am using promises?

Upvotes: 2

Views: 11110

Answers (3)

Dhruv Shah
Dhruv Shah

Reputation: 1651

There are essentially two ways to address this problem.

In the first way, you can address the overall design of your API and make the code more effective

For example, if you operation is successful based on the fact that whether the user is an admin or not, you can respond with Promise.reject or Promise.resolve. But, if you are returning a Promise.reject from the createProduct function, then you should also update the way you handle the rejection in the controller.

For example:

createProduct.js

module.exports.createProduct = (reqBody, isAdmin) => {
  if (isAdmin === true) {
    let newProduct = new Product({
      name: reqBody.name,
      description: reqBody.description,
      price: reqBody.price,
    });

    return newProduct.save().then((result, error) => {
      return error ? false : 'Successfully created a product';
    });
  } else {
    // Return a promise here
    return Promise.reject('Not authorized to access this page');
  }
};

controller.js

router.post('/create',auth.verify, (req, res)=>{

    const isAdmin = auth.decode(req.headers.authorization).isAdmin;

    productController.createProduct(req.body,isAdmin)
       .then(result => res.send(result))
       .catch(error => res.send(error)); // catch will be executed due to Promise.reject

})

Or, in the second way, you can simply return a Promise.resolve from the createProduct function's else block.

Example:

return Promise.resolve("Not authorized to access this page")

And in this case, you don't need to update your controller code.

I would recommend following the .then .catch way of doing things in order to catch any error that you may face with the Product creation part. It makes error tracking easier.

Upvotes: 2

matt123miller
matt123miller

Reputation: 191

You're correct that it won't work if the user isn't an admin and that you need to return a promise. The simplest solution is to change the else statement to the following.

else{
    return Promise.resolve("Not authorized to access this page")
}

You could also return Promise.reject(new Error("Not authorized to access this page")) which would force anything consuming this function to handle an error for this scenario. That might fit your design, it might not.

Upvotes: 1

Clarity
Clarity

Reputation: 10873

You need to return a promise in both success and error cases:

module.exports.createProduct = (reqBody, isAdmin) => {
  if (isAdmin === true) {
    let newProduct = new Product({
      name: reqBody.name,
      description: reqBody.description,
      price: reqBody.price,
    });

    return newProduct.save().then((result, error) => {
      return error ? false : 'Successfully created a product';
    });
  } else {
    // Return a promise here
    return Promise.reject('Not authorized to access this page');
  }
};

Upvotes: 2

Related Questions