TypeScript: Inconsistent behavior when replacing image source using Promise.all

Question

For my SPFx webpart (using React/TypeScript) I am trying to replace the source of some images as the initially fetched source must be retrieved first using another Microsoft Graph API call.

I fetch all images inside the HTML which will be later rendered using temporalDivElement.querySelectorAll("img") and then checking them if the source needs to be replaced. If this is the case I then call the Graph API to fetch the new image source (or a replacement span node, if the image cannot be fetched). As I have to iterate over all images, I first collect all those requests to the Graph API in an array of Promises and then later execute them using Promise.all().

This is my current code:

  public parseAndFixBodyContent(messageId: string, bodyContent: string) {
    return new Promise<string>(async (resolve, reject) => {
      let temporalDivElement = document.createElement("div");
      temporalDivElement.innerHTML = bodyContent;
      // For hosted content images: fetch hosted content with Graph API, convert to BLOB and then replace existing src attribute with BLOB value
      const imgTags = temporalDivElement.querySelectorAll("img");
      const hostedImagesReplacementPromises: Promise<void>[] = [];
      imgTags.forEach((img) => {
        if (img.src &&
          img.src.indexOf(`/messages/${messageId}/hostedContents/`) > -1) {
            // Hosted Content url found, try to fetch image through API
            let hostedContentApiUrl: string = img.src;
            hostedImagesReplacementPromises.push(this.replaceHostedContentImageSource(hostedContentApiUrl, img));
          }
      });

      Promise.all(hostedImagesReplacementPromises)
      .then(() => {
        resolve(temporalDivElement.innerHTML);
      });
    });
  }

  public replaceHostedContentImageSource(hostedContentApiUrl: string, image: HTMLImageElement) {
    return new Promise<void>(async (resolve, reject) => {
      this.getHostedContentAsBlob(hostedContentApiUrl).then((imageBlobUrl) => {
        image.src = imageBlobUrl;
        resolve();
      })
      .catch(error => {
        // We could not fetch the hosted content for the image
        let missingImageInfo = document.createElement("span");
        missingImageInfo.innerText = `(${strings.ImageNotAvailable})`;
        image.parentNode.replaceChild(missingImageInfo, image);
        resolve();
      });
    });
  }
  
  public getHostedContentAsBlob(hostedContentApiUrl: string) {
    return new Promise<string>(async (resolve, reject) => {
      this.context.msGraphClientFactory
        .getClient()
        .then((client: MSGraphClient): void =>{
          client
            .api(hostedContentApiUrl)
            .version("beta")
            .responseType('blob')
            .get((error, response: Blob, rawResponse?: any) => {
              if (rawResponse.status == 200 && response) {
                const imageUrl: string = URL.createObjectURL(response);
                resolve(imageUrl);
              } else {
                reject(new Error(strings.ErrorCouldNotGetHostedConent));
              }
            });
          })
        .catch(error => {
          reject(error);
        });
    });
  }

This code does sometimes work and sometimes not at all and sometimes it works for half of the images and not the other half. For example I use it on the same two channel replies that have hosted content images in them and sometimes I get both images, then I get only one image and the other one hasn't been replaced at all (not even the SPAN tag with the information that the replacement failed) or both haven't been processed. It is like sometimes the promises don't get executed or at least not at the right time before rendering.

I don't see what's wrong with my code but I guess there is some timing issue here?

Answer 1

I was approached, if I ever found a solution for this issue.

To my surprise I have unfortunately forgotten that I posted this question but on the brighter side I seem to have solved the issue a day later.

I cannot remember fully but looking at the new code I believe it was indeed a timing issue or more detailed the issue comes from trying to solve the promises inside a forEach loop.

I'm not yet very proficient in JS/React but from my understanding promises are also async, so this code I previously used is a big no-no:

  // Some message body content needs to be prepared for the display (fetch hosted content, etc.)
  slicedChannelTopics.forEach((t) => {
    this.parseAndFixBodyContent(t.message.id, t.message.content).then((transformedContent) => {
      if (t.message.contentType && t.message.contentType == 'html' && t.message.content) {
        t.message.content = transformedContent;
      }
    })
    .catch(parseError => {
      console.log(parseError);
    });
  });

I changed this to first collect all promises and then solve them using Promise.all(...):

let promisesForParseAndFixBodyContent = topicReplies.map((reply) => 
  {
    return this.parseAndFixBodyContent(reply.message, MessageType.Reply);
  });
  Promise.all(promisesForParseAndFixBodyContent).then(() => {
    resolve(topicReplies);
  });

Since making this change, the issue with loading the hosted content was gone.