0

This is what my code looks like:

  userUpdate(req: Request, res: Response) {
    this.userTaskObj.userUpdate(req.params.id, req.body).then(() => {
      res.status(200).json({
        status: 'OK',
        message: 'User updated',
      });
    })
      .catch((err) => ErrorHandler.getErrorMessage('User', res, err));
  }

(below function is inside UserTask class)

  userUpdate(id: string, data: any) {
    let query: Promise<any>;
    if (data.provides) {
      query = User.findByIdAndUpdate(id, {$addToSet: {provides: data.provides}}).exec();
    } else {
      query = User.findByIdAndUpdate(id, data).exec();
    }
    return new Promise((resolve, reject) => {
      resolve(query);
      // query.then(res => resolve(res))
      // .catch(err => reject(err));
    });
  }

I thought I wouldn't able to catch errors from the query using resolve alone. But it does catch mongoose errors. When I replaced it with the commented part, that too works but the errors are caught inside the catch() which is rejected. How is the code catching errors without the catch() ?

anandsr-dev
  • 67
  • 2
  • 5
  • 1
    Why do you wrap the query with another Promise? Try return query instead of return new Promise... – hong4rc Mar 30 '22 at 11:22
  • That's a coding standard followed in my company. We do it for all "Task" class functions. – anandsr-dev Mar 30 '22 at 11:42
  • @anandsr-dev This makes absolutely zero sense and [is actually harmful](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it). A promise is not a "Task"!!! – Bergi Mar 30 '22 at 12:24

1 Answers1

0

.catch() runs when the promise is rejected. An error results in rejecting the promise. As a consequence:

(new Promise(()=>{throw Error()}).then(()=>{
  console.log("this will not display because the promise was rejected beforehand")
}))
.catch(()=>{
  console.log("this will run");
})

In your case, when the mangoose error occurs, this.userTaskObj.userUpdate(req.params.id, req.body) has already returned, and the returned object is a promise instance.

This promise instance fails to resolve according to the query failure, so it is eventually rejected. Indeed, Promise.resolve(Promise.reject()) results in a rejected promise [Note: as commented by others, this is anti-pattern].

Hence the .catch() takes over.

Vincent
  • 453
  • 3
  • 6
  • No, `resolve(Promise.reject())` is not an antipattern. `new Promise`, when not needed, is. – Bergi Mar 30 '22 at 12:56