Call a callback function inside for of loop

Question

I'm making a request with a for of loop to delete all the cart elements, but it doesn't work as expected. Loop works for first iteration but API returns an error in the second iteration.

I think it's a logical error, what's the problem with this code?

My try:

module.exports.removeAllCartItems = (token) => {
    this.getCart(token, (error, data) => {
        if(error){
            Sentry.captureException(error);
        }

        const cartItems = data.items
        console.log('------cart items----------')
        console.log(cartItems)
        console.log('------cart items----------')
        for(item of cartItems){
            let productId = item.productId
            let variantId = item.variant.product_id
            console.log('--------item one by one---------')
            console.log(item)
            console.log('--------item one by one---------')
           
            this.removeItem(token, productId, variantId, (error, data)=> {
        
                if(error) {
                    Sentry.captureException(error);             
                }
                console.log('------data-------')
                console.log(data)
                console.log('******data*******')
            } )
            
        }

        
    })
}

Remove item method:

module.exports.removeItem = (token, productId, variantId, callback) => {
    const url = urlBase + "/removeItem" + `?secretKey=${secretKey}`;
    request(
        {
            url: url,
            method: "DELETE",
            json: true,
            headers: {
                "content-type": "application/json",
                Authorization: `Bearer ${token}`,
            },
            body: {
                secretKey: `${secretKey}`,
                productId: productId,
                variantId: variantId,
            },
        },
        (error, response) => {
            if (error) {
                Sentry.captureException(error);
                callback(errMessage, undefined);
            } else {
                const data = response.body;
                callback(undefined, data);
            }
        }
    );
};

Console output:

------cart items----------
[
  {
    variant: {
      variation_values: [Object],
      price: 475,
      product_id: '883360541280',
      orderable: true
    },
    _id: '62403a55ec05ea0024e9d965',
    productId: '21736758',
    quantity: 1
  },
  {
    variant: {
      variation_values: [Object],
      price: 58.99,
      product_id: '701643411498',
      orderable: true
    },
    _id: '62403b7eec05ea0024e9d966',
    productId: '25589208',
    quantity: 1
    variation_values: { color: 'P2V', size: '42' },
    price: 475,
    product_id: '883360541280',
    orderable: true
  },
  _id: '62403a55ec05ea0024e9d965',
  productId: '21736758',
  quantity: 1
}
--------item one by one---------
--------item one by one---------
{
  variant: {
    variation_values: { color: 'JJ3WCXX', size: '9MD' },
    price: 58.99,
    product_id: '701643411498',
    orderable: true
  },
  _id: '62403b7eec05ea0024e9d966',
  productId: '25589208',
  quantity: 1
}
--------item one by one---------
------data-------
{
  _id: '6240398eec05ea0024e9d962',
  secretKey: '<secretKey>',
  userId: '623dfb884083b000243cc4a7',
  items: [
    {
      variant: [Object],
      _id: '62403a55ec05ea0024e9d965',
      productId: '21736758',
      quantity: 1
    }
  ],
  __v: 6
}
******data*******
------data-------
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<pre>Internal Server Error</pre>
</body>
</html>

******data*******

Why API returns internal server error in the second iteration, is it about my code or something else, any idea ?

I make it work with that way but still dont know why first implementation doesn't work

module.exports.removeAllCartItems = (token) => {
    this.getCart(token, async (error, data) => {
        if (error) {
            Sentry.captureException(error);
        }

        const cartItems = data.items;

        for (item of cartItems) {
            let productId = item.productId;
            let variantId = item.variant.product_id;

            await new Promise((resolve, reject) => {
                this.removeItem(token, productId, variantId, (error, data) => {
                    if (error) {
                        reject(error);
                        Sentry.captureException(error);
                    }
                    resolve(data);
                });
            });
        }
    });
};

What's that lonely this referring to in your function? That's a bug waiting to happen. — Tomalak
– Tomalak, Commented Mar 27, 2022 at 11:12
this refers to a request function in the same .js file so I access with this keyword is it problem ? — AssSoonAsPossiblee
– AssSoonAsPossiblee, Commented Mar 27, 2022 at 11:32
...since I can't see the rest of your module, I can only say that it looks very odd: When you set module.exports.removeAllCartItems = function ... that means exports can't be a class. And when exports is not a class, then this is not going to be bound to anything useful inside removeAllCartItems, or requires explicit binding via .call() or .apply(). So if it works for you, fine. I think it looks fishy. — Tomalak
– Tomalak, Commented Mar 27, 2022 at 11:39

Tomalak · Accepted Answer · 2022-03-27 11:26:29Z

All your functions (getCart(), removeItem()) are asynchronous. removeAllCartItems() needs to be asynchronous, too.

Introduce a callback into removeAllCartItems(), exactly like the other two have. Call it either when any error occurs, or when you're done (i.e., when no items to remove are left).

module.exports.removeAllCartItems = (token, callback) => {
    this.getCart(token, (error, cart) => {
        if (error) {
            Sentry.captureException(error);
            callback(error);
            return;
        }
        let remainingItems = cart.items.length;
        if (remainingItems === 0) callback();

        for (let item of cart.items) {
            this.removeItem(token, item.productId, item.variant.product_id, (error, data) => {
                if (error) {
                    Sentry.captureException(error);
                    callback(error);
                    return;
                }
                remainingItems--;
                if (remainingItems === 0) callback();
            });
        }    
    });
};

In the calling code, use the callback.

removeAllCartItems(token, (err) => {
    if (err) {
        // display error somehow
        return;
    }
    // display success somehow
});

Unrelated thought on your API design: Your current removeItem() function has this signature:

removeItem(token, productId, variantId)

it should probably have this signature instead:

removeItem(token, item)

where item can either be an item instance (convenient for loops like in this case) or possibly an item ID.

The function can then access item.productId, and item.variant.product_id or whatever else it needs internally, that should not be the calling code's concern.

Thanks for your answer. I did not design the API, just access it.
@AssSoonAsPossiblee Yeah, that was just a thought that crossed my mind. Does the rest work?
It still works for first iteration, It can delete the first item in the cart but not the second one. API returns internal server error for the second iteration as a response it can be a problem about API.
@AssSoonAsPossiblee Well I corrected the general mistake in your approach, this will become relevant later. It's impossible for me to say which error occurs, you need to share more details. My suspicion is that it's related to modifying the collection you are iterating over.
When I use promise,, I don't know why but I can delete all cart items.

Collectives™ on Stack Overflow

Call a callback function inside for of loop

1 Answer 1

8 Comments

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

8 Comments

Related