Search code examples
javascriptreactjsloopspromisefetch

Two looped async calls followed by a sync call


I need to edit previously saved messages that contain attachments. In the edit functionality, the user should be able to edit message title or text, replace existing attachments or delete attachments.

Scenario: User made changes to message text, replaced one attachment and deleted 2 attachments and hit Save. OnClick of Save button, the following actions need to happen

  1. Replace and Delete API calls async first
  2. If replace and delete is successful then api call to save changes to text

This is where I'm at..

.then of Promise.all in replace and delete is executed before the loop promise finishes, resulting in the outer promise (in handleSave) getting a resolved promise always.

deleteData and replaceData are arrays containing the data to be replaced/deleted

const delete = () => {
    if (deleteData.length > 0) {
      return Promise.all([
        deleteData.map(data => fetch(url)
          .then((response) => {
            if (!response.ok) {
              throw new Error('Delete Failed');
            }
          })
        ),
      ]).then(() => Promise.resolve('Deleted'));
    }
    return Promise.resolve('Nothing to delete');
  };

const replace = () => {
    if (replaceData.length > 0) {
      return Promise.all([
        replaceData.map(data => fetch(url)
          .then((response) => {
            if (response.status === 400) {
              response.text().then((error) => {
                const errorObject = JSON.parse(error);
                throw new Error(error);
              });
            } else if (!response.ok) {
              throw new Error('Update Failed');
            }
          })
        ),
      ]).then(() => Promise.resolve('Replaced'));
    }
    return Promise.resolve('Nothing to replace');
  };

const saveMessage = () => {
    fetch(saveUrl)
        .then((response) => {
            if (response.status === 400) {
              response.text().then((error) => {
                const errorObject = JSON.parse(error);
                throw new Error(error);
              });
            } else if (!response.ok) {
              throw new Error('Save Failed');
            }
        });
}

const handleSave = () => {
  Promise.all([replace, delete])
     .then(() => {
       saveMessage();
      }).catch((e) => {
        //Catch action
      })
  }

Solution

  • To your Promise.alls, you're currently passing an array containing a single item, where that item is an array of Promises. As a result, the Promise.all resolves immediately.

    Pass an array of Promises to Promise.all instead - that is, just the .map, not [data.map(...)]. Change:

    return Promise.all([
      deleteData.map(data => fetch(url)
    

    to

    return Promise.all(
      deleteData.map(data => fetch(url)
    

    and

    return Promise.all([
      replaceData.map(data => fetch(url)
    

    to

    return Promise.all(
      replaceData.map(data => fetch(url)
    

    Also, inside replace, it isn't returning the inner .text Promise when there's a 400 error, so its mapped Promise.all won't reject when you want it to. Change:

    if (response.status === 400) {
      response.text().then((error) => {
        const errorObject = JSON.parse(error);
        throw new Error(error);
      });
    

    to

    if (response.status === 400) {
      return response.json().then((error) => {
        throw new Error(error);
      });
    

    (note that you can use .json() instead of JSON.parse)

    Also, for your saveMessage errors to be handled in the catch, you should return each of its Promises too:

    const saveMessage = () => {
      return fetch(saveUrl)
        .then((response) => {
          if (response.status === 400) {
            return response.json().then((error) => {
              throw new Error(error);
            });
          } else if (!response.ok) {
            throw new Error('Save Failed');
          }
        });
    };