Why does this recursive function not run asynchronously?

Question

I have a start(node, array) function that should perform a DFS by traversing an object tree via recursive calls to an API through callMsGraph(token, end) until image properties are found at the end of the tree, at which point they are pushed to array. The function seems like it works, but I can't get the output unless I wrap it in a 2 second setTimeout which indicates the recursion is not being waited on to complete. I would want to play around with async/await more, but it's not at the top-level.

I'm not sure if the nextNode.then is doing anything or maybe callMsGraph() needs to be awaited on differently to how I know. A solution would be much appreciated.

shelfdb.data = async (accessToken) => {

    const token = accessToken;
    const endpoint = 'https://graph.microsoft.com/v1.0/sites/webgroup.sharepoint.com,23e7ef7a-a529-4dde-81ba-67afb4f44401,0fa8e0f7-1c76-4ad0-9b6e-a485f9bfd63c/drive/items/01GNYB5KPQ57RHLPZCJFE2QMVKT5U3NYY3/children'
    
    function start(node, array) {
        if(node.value.length > 0) {
            node.value.forEach(function(child) {
                var end = 'https://graph.microsoft.com/v1.0/sites/webgroup.sharepoint.com,23e7ef7a-a529-4dde-81ba-67afb4f44401,0fa8e0f7-1c76-4ad0-9b6e-a485f9bfd63c/drive/items/' + child.id + '/children';
                var nextNode = callMsGraph(token, end);
    
                nextNode.then(function(currResult) {
                    if (currResult.value.length > 0) {

                        if ('image' in currResult.value[0]) {
                            currResult.value.forEach(function(imgChild) {
                                let img = {
                                    'name': imgChild.name,
                                    'job': imgChild.parentReference.path.split("/")[6],
                                    'path': imgChild.webUrl,
                                    'id': imgChild.id
                                }
                                array.push(img);
                            })
                            // complete storing images at tail object, go one level up after loop
                            return;
                        }

                        // if no 'image' or value, go into child
                        start(currResult, array);
                    }
                }).catch(function(e) {
                    console.error(e.message);
                })
            })
        }
    
        return array;
    }
    
    
    var res = await callMsGraph(token, endpoint); // start recursion
    var output = start(res, []); 
    console.log(output); // only displays value if wrapped in setTimeout

    return output; // empty []

}

Each query to the API via callMsGraph(), returns an object like this, where subsequent queries are made with the id of each object/folder (as new endpoint) in value until an object with image property is found. The MS Graph API requires that folders are expanded at each level to access their children.

{
  id: '01GNYB5KPQ57RHLPZCJFE2QMVKT5U3NYY3'
  value: [
    {
      id: '01GNYB5KJMH5T4GXADUVFZRSITWZWNQROS',
      name: 'Folder1',
    },
    {
      id: '01GNYB5KMJKILOFDZ6PZBZYMXY4BGOI463',
      name: 'Folder2',
    }
  ]
}

This is the callMsGraph() helper:

function callMsGraph(accessToken, graphEndpoint) {
    const headers = new Headers();
    const bearer = `Bearer ${accessToken}`;

    headers.append("Authorization", bearer);

    const options = {
        method: "GET",
        headers: headers
    };

    return fetch(graphEndpoint, options)
        .then(response => response.json())
        .catch(error => {
            console.log(error);
            throw error;
        });
}

Sphygmoid

Answer 1

There is a few places where you are not handling promises returned in you code. nextNode.then if your forEach loop is just "called", next line of the code will not wait for it to complete, forEach loop will complete execution before then callbacks are called.

I changed you code a bit, but I have no way to check if it works correctly due to i would need to populate dummy data for callMsGraph but if you encounter any - tell me and I'll modify the answer

shelfdb.data = async (accessToken) => {
  const token = accessToken;
  const endpoint = 'https://graph.microsoft.com/v1.0/sites/webgroup.sharepoint.com,23e7ef7a-a529-4dde-81ba-67afb4f44401,0fa8e0f7-1c76-4ad0-9b6e-a485f9bfd63c/drive/items/01GNYB5KPQ57RHLPZCJFE2QMVKT5U3NYY3/children'

  const images = [];

  async function start(node, array) {
    if (node.value.length <= 0) return array; // or === 0 or whatever

    for (const child of node.value) {
      const end = `https://graph.microsoft.com/v1.0/sites/webgroup.sharepoint.com,23e7ef7a-a529-4dde-81ba-67afb4f44401,0fa8e0f7-1c76-4ad0-9b6e-a485f9bfd63c/drive/items/${child.id}/children`;
      const nextNode = await callMsGraph(token, end);
      if (nextNode.value.length > 0) {
        if ('image' in nextNode.value[0]) {
          const mapped = nextNode.value.map(imgChild => {
            return {
              'name': imgChild.name,
              'job': imgChild.parentReference.path.split("/")[6],
              'path': imgChild.webUrl,
              'id': imgChild.id
            }
          });
          array.push(...mapped);
        }

        // if no 'image' or value, go into child
        await start(nextNode, array);
      }
    }

    return array;
  }


  var res = await callMsGraph(token, endpoint);
  var output = await start(res, []);
  console.log(output);

  return output;
}

Also, please, feel free to add a try{} catch{} blocks in any place you need them, I skipped them

Sped

Works like a charm, definitely will avoid forEach() for asynchronous code in future. – Sphygmoid

Answer 2

The rule with promises is that once you opt into one (more likely, are forced into it by a library), all code that needs to block for a result anywhere after it also has to await. You can't "go back" to sync and if even a single piece of the promise chain between where the promise starts and where you want its result isn't awaited, the result will be unreachable^*.

Taking a snippet of the code:

function start(node, array) { // not async!
    // ..
    node.value.forEach(function(child) { // doesn't await!
        // ..
        nextNode.then(function(currResult) {
        // this promise is not hooked up to anything!
        start(...) // recurse without await!

There's no await in front of then, start doesn't return a promise and isn't awaited recursively, and forEach has no way to await its callback's asynchronous results, so each promise in the nextNode.then chain is orphaned into the void forever^*.

The solution is a structure like this:

async function start(node, array) {
    // ..
    for (const child of node.value) {
        // ..
        const currResult = await callMsGraph(token, end);
        // ..
        await start(...);
        array.push(currResult);
    }
    // returns a promise implicitly
}

// ..
await start(...);
// `array` is populated here

Or Promise.all, which runs in parallel and returns an array (which could replace the parameter array):

function start(node, array) {
    return Promise.all(node.value.map(async child => {
        const currResult = await callMsGraph(token, end);
        // ..
        await start(...);
        return currResult;
    }));
}

I'd be happy to provide a minimal, runnable example, but the code you've provided isn't runnable, so you'll have to massage this a bit to work for you. If you make sure to await everything, you're good to go (and generally avoid mixing .then and async/await--the latter seems easier for this use case).

^{* (for all practical intents and purposes)}

Condemnatory