0

I have the following code writed in nodejs express and firebase

route.js

try{
const test = await invoiceData.setAssignsInvoiced();
res.json({
      status: true,
      message: "Successful Invoice Generation"
    });
}catch (e) {
    res.status(500).json({
      status: false,
      message: "Internal Server Error",
      data: e
    });
  }

InvoicesStorage.js

setAssignsInvoiced = async() => {
    return new Promise(async (resolve,reject)=>{
        try {
            await _getAssignsForInvoiced(this);
            this.assignsForInvoiced.forEach(async assing => {
                let aux = assing._key.path.segments.length;
                let ref = assing._key.path.segments[aux - 1];
                await _updateAssignsToInvoiced(assing.data(),ref);
            });    
            resolve(true)  
        } catch (error) {
            console.error(error)
            reject(error)
        }    
    })
  };


const _updateAssignsToInvoiced = async (assing, ref) => {
  try {
    const { invoiceNum } = assing.data(); //Here's an intentional error
    await db
      .collection("leadAsign")
      .doc(ref)
      .update({
        invoiced: true,
        updateDate: Date.now() - 240 * 60 * 1000,
        invoiceNum
    });
  } catch (error) {
    console.error(error);
    throw new Error("Error at update to invoiced assigns");
  }
};

How I hope it works: According to me, I should throw out a synchronous error because my code has "await" and stop the system.

The answer I have: the code runs asynchronously, that is, after calling the function the "await" has no effect and answers a "res.json" with status 200 and it is only after it throws the next error.

TypeError: assing.data is not a function
    at _updateAssignsToInvoiced (D:\$Workzone\gd_fridays_h\src\controllers\invoices\InvoicesStorage.js:90:35)
    at D:\$Workzone\gd_fridays_h\src\controllers\invoices\InvoicesStorage.js:55:23
    at Array.forEach (<anonymous>)
    at D:\$Workzone\gd_fridays_h\src\controllers\invoices\InvoicesStorage.js:51:37
true
POST /generateSingle 200 5182.650 ms - 57
(node:5600) UnhandledPromiseRejectionWarning: Error: Error at update to invoiced assigns
    at _updateAssignsToInvoiced (D:\$Workzone\gd_fridays_h\src\controllers\invoices\InvoicesStorage.js:102:11)
    at D:\$Workzone\gd_fridays_h\src\controllers\invoices\InvoicesStorage.js:55:23
    at Array.forEach (<anonymous>)
    at D:\$Workzone\gd_fridays_h\src\controllers\invoices\InvoicesStorage.js:51:37
(node:5600) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 4)
(node:5600) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

2 Answers2

1

async/await doesn't work as you're expecting it to inside a forEach loop. An abundance of info on that specific issue here: https://stackoverflow.com/a/37576787/4043746

To fix your problem, you could use a for/of loop:

setAssignsInvoiced = async () => {
  return new Promise(async (resolve, reject) => {
    try {
      await _getAssignsForInvoiced(this)

      for (const assign of this.assignsForInvoiced) {
        let aux = assign._key.path.segments.length
        let ref = assign._key.path.segments[aux - 1]
        await _updateAssignsToInvoiced(assign.data(), ref)
      }

      resolve(true)
    } catch (error) {
      console.error(error)
      reject(error)
    }
  })
}

However, I'd also be tempted to suggest not returning a promise, as you're essentially doing that due to it being an async function. Something like this should work and is cleaner imo:

setAssignsInvoiced = async () => {
  try {
    await _getAssignsForInvoiced(this)

    for (const assign of this.assignsForInvoiced) {
      let aux = assign._key.path.segments.length
      let ref = assign._key.path.segments[aux - 1]
      await _updateAssignsToInvoiced(assign.data(), ref)
    }
  } catch (error) {
    console.error(error)
    // Re-throwing the error to pass the error down, just like you've
    // done inside your _updateAssignsToInvoiced function's catch
    throw new Error('Error setting assigns')
  }
}
Andy Mardell
  • 1,132
  • 8
  • 13
  • 1
    I am really very grateful for your recommendations, I have a lot to improve and you have given me a great contribution, this works in an excellent way – Kem N. Briceño Feb 22 '20 at 15:27
1

Async/await inside a forEach() loop will not wait until all the async operations inside the loop is completed. One approach would be using Promise.all() like so:

const setAssignsInvoiced = async () => {
  try {
    await _getAssignsForInvoiced(this);
    await _updateAssignsList(this.assignsForInvoiced);
    return true;
  } catch (error) {
    console.error(error);
    return new Error(error);
  }
};

const _updateAssignsList = assignsList => {
  return Promise.all(
    assignsList.map(async assign => {
      let aux = assign._key.path.segments.length;
      let ref = assign._key.path.segments[aux - 1];
      return await _updateAssignsToInvoiced(assign.data(), ref);
    })
  );
};

I've just extracted the async loop process to a separate function which return a Promise.

Adithya Sreyaj
  • 1,710
  • 10
  • 22