How to refactor an async function with Promise inside of it-CodePudding

Having this async function that returns a Promise:

async function doSomething(userId) {
  return new Promise(async (resolve, reject) => {
    const query = 'my-query';

    const roomAggregate = Room.aggregate(query).allowDiskUse(true);
    Room.aggregatePaginate(roomAggregate, async function (err, data) {
      if (err || !data) {
        throw err;
        return {
          success: false,
          rooms: [],
        };
      } else {
        const rooms = [];
        for (let room of data.docs) {
          const roomId = room._id;
          room = await computeAThing(room, {
            loadWriteUps: false,
            loadCreators: false,
          });
          room.userCompleted = await computeBThing(userId, roomId);
          rooms.push(room);
        }
        return resolve({
          rooms,
          success: true,
          paginator: {
            // some data related to pagination
          },
        });
      }
    });
  });
}

I'm not sure if it really needs to contain new Promise inside as it is already declared as async function. Is it mandatory in this case?

Because when that part was removed and at the end instead of return resolve({...}) it is only return {...} it seems to not settle.

Here is the modified code with new Promise and different return:

async function doSomething(userId) {
  const query = 'my-query';

  const roomAggregate = Room.aggregate(query).allowDiskUse(true);
  Room.aggregatePaginate(roomAggregate, async function (err, data) {
    if (err || !data) {
      throw err;
      return {
        success: false,
        rooms: [],
      };
    } else {
      const rooms = [];
      for (let room of data.docs) {
        const roomId = room._id;
        room = await computeAThing(room, {
          loadWriteUps: false,
          loadCreators: false,
        });
        room.userCompleted = await computeBThing(userId, roomId);
        rooms.push(room);
      }
      return {
        rooms,
        success: true,
        paginator: {
          // some data related to pagination
        },
      };
    }
  });
}

This method is used somewhere else in this way:

 const myInfo = await someObj.doSomething('myUserId');

and then checked:

if (myInfo.success) { ... }

For the first way of writing the function it works fine, for the second one myInfo is undefined and it throws and error that cannot read success of undefined.

Is something missing from the implementation?

CodePudding user response：

For the second version I can see that you actually not returning anything from doSomething so I think you should do the below:

  async function doSomething(userId) {
  const query = 'my-query';

  const roomAggregate = Room.aggregate(query).allowDiskUse(true);
  const obj = Room.aggregatePaginate(roomAggregate, async function (err, data) {
    if (err || !data) {
      throw err;
      return {
        success: false,
        rooms: [],
      };
    } else {
      const rooms = [];
      for (let room of data.docs) {
        const roomId = room._id;
        room = await computeAThing(room, {
          loadWriteUps: false,
          loadCreators: false,
        });
        room.userCompleted = await computeBThing(userId, roomId);
        rooms.push(room);
      }
      return {
        rooms,
        success: true,
        paginator: {
          // some data related to pagination
        },
      };
    }
  });
  return obj;
}

In general, you don't need to explicitly return a Promise in an async function as by default the returned value will be wrapped in a Promise , whatever it is. You just need to return something