Is there any purpose to wrapping an async function in a promise?

Question

I'm just inherited a new codebase and this pattern in javascript seems to do nothing:

/**
 * Update Data Model
 * @param {Object} model
 * @param {String} field
 * @param {Mixed} value
 */
function updateDataModel(model, field, value) {
  return new Promise((resolve) => {
    async function promiseFunc() {
      model[field] = value;
      await model.save();
      resolve(model);
    }

    return promiseFunc();
  });
}

module.exports = updateDataModel;

It's everywhere in the code base and I'm trying to be charitable, but I can't imagine any use of wrapping the async await within the promise? It's confusing and doesn't seem to do anything.

In my mind the above code pattern is basically equivalent to either:

/**
 * Update Data Model
 * @param {Object} model
 * @param {String} field
 * @param {Mixed} value
 */
async function updateDataModel(model, field, value) {
      model[field] = value;
      await model.save();
      return model
}

module.exports = updateDataModel;

---or---

/**
 * Update Data Model
 * @param {Object} model
 * @param {String} field
 * @param {Mixed} value
 */
function updateDataModel(model, field, value) {
      model[field] = value;
      const newModel = model.save().then((model)=> return model);
      return newModel;
}

module.exports = updateDataModel;

Am I missing something important here? Everything is very buggy and moving to a simpler pattern is one of the first steps the team is taking to try and improve. Error checking i somewhat difficult because the error is an unhandled promise in most cases.

Answer 1

If the save promise returns the updated model, I would just return it.

/**
 * Update Data Model
 * @param {Object} model
 * @param {String} field
 * @param {Mixed} value
 */
function updateDataModel(model, field, value) {
  model[field] = value;
  return model.save(); // Return the promise here (returns the underlying model)
}

const animal = {
  type: 'cat',
  color: 'white',
  save() {
    // Do some DB updates...
    return Promise.resolve(this);
  },
  toString() {
    return `Model(type=${this.type},color=${this.color})`;
  }
};

updateDataModel(animal, 'color', 'black').then(model => {
  console.log('Updated model:', model.toString());
});

Or:

const model = await updateDataModel(animal, 'color', 'black');
console.log('Updated model:', model.toString());

Answer 2

Since async/await is syntactic sugar around a Promise, the existing code is redundant and can be rewritten like you suggest.

Each time when an async function is called, it returns a new Promise which will be resolved with the value returned by the async function, or rejected with an exception uncaught within the async function.

Source: mdn

Answer 3

It's confusing and doesn't seem to do anything.

It actually does something: it's swallowing errors. This is utterly broken. Do not use it.

It would "work" if it was at least written as

function updateDataModel(model, field, value) {
  return new Promise((resolve) => {
    async function promiseFunc() {
      model[field] = value;
      await model.save();
      return model;
    }

    resolve(promiseFunc()); // or `promiseFunc().then(resolve, reject)
  });
}

What you inherited is just another form of the Promise constructor antipattern. Avoid it at all costs! Definitely go for the much simpler approach you suggested.