0

I'm making an application using the GitHub api and I'm having trouble with async functions. I'm new to using async so I would appreciate the help a lot. Here's the code I've written till now:

const getFiles = async function(token, reponame) {
  var gh = new GitHub({
    token: token
  });

  reponame = reponame.split("/");
  const repo = gh.getRepo(reponame[0], reponame[1]);

  let head = new Headers();
  head.append("Authorization: ", "token " + token);

  const getContents = new Promise((res, rej) => {
    repo.getContents(null, "content", true, (err, files) => {
      if (err) rej(err);
      else return files;
    }).then(files => {
      let promises = [
        files.map(
          file =>
            new Promise(res => {
              fetch(file.downloadURL).then(body => {
                res(body.text);
              });
            })
        )
      ];

      const retFiles = [];
      await Promise.all(promises.map(promise => retFiles.push(promise)));
      res(retFiles)
    });
  });

  return getContents;
};

The error I'm getting is unexpected reserved word at the line where I used await. Thanks in advance

Kartik Nair
  • 25
  • 2
  • 3
  • 1
    Welcome to SO! You can't use `await` unless it's directly inside of a function with the `async` keyword. Change `files => {` to `async files => {`. Also, it's a little odd to make `let promises` a 2d array--just assign the result of `map` directly to it, otherwise you have a single-element array with the promises array inside of it. – ggorlen Mar 16 '20 at 16:30
  • 2
    There are a lot of problems with your code, in addition to the one pointed out by ggorlen `Array.prototype.push` returns the *length of the array*, which is almost certainly not what you want. Return values from async functions are automatically wrapped in a promise. The fetch API already returns a promise, no need to build one manually there either. It's not clear *why* you're using async/await when you're doing a lot of .then chaining, or why you're using .then chaining when you're using async/await. Your headers appear to be unused. You *assign to a function parameter*, etc. etc. – Jared Smith Mar 16 '20 at 16:34
  • @ggorlen thank you for your help. I don't know why I was making a 2d array. I think I'm losing my mind. Anyways managed to figure it out, [here's the working code](https://stackoverflow.com/a/60710249/12785202). – Kartik Nair Mar 16 '20 at 17:12

4 Answers4

0

So I'm going to make some assumptions here, so correct me if I'm wrong and I'll fix them. Hopefully in doing so, I can help clarify your understanding.

An easy way of thinking about async/await is replacing the need for .then(callback). I prefer to use await if in an async function.

const getFiles = async function(token, reponame) {
  try {
    var gh = new GitHub({
      token: token
    });
    reponame = reponame.split("/");

    // edit: I removed await from the line below as your original code
    //       did not treat it as a promise
    const repo = gh.getRepo(reponame[0], reponame[1]); 

    // unused code from your post
    let head = new Headers();
    head.append("Authorization: ", "token " + token);

    // the await below assumes that repo.getContents 
    // will return a promise if a callback is not provided
    const files = await repo.getContents(null, "content", true); 

    // updating the code below so that the file requests run in parallel.
    // this means that all requests are going to fire off basically at once
    // as each fetch is called
    const fileRequests = files.map(file => fetch(file.downloadURL))

    // you wont know which failed with the below.
    const results = (await Promise.all(fileRequests)).map(res => res.text)
    return results
  } catch (err) {
    // handle error or..
    throw err;
  }
};

This code is untested. I haven't used github's api so I'm best-guessing what each call is doing. If gh.getRepo or repo.getContents do not return promises then some adjustment would be needed.

In the event the github library you are using will not return a promise if a callback is not provided: 

const getFiles = async function(token, reponame) {
  try {
    var gh = new GitHub({
      token: token
    });
    reponame = reponame.split("/");
    const repo = await gh.getRepo(reponame[0], reponame[1]); 
    let head = new Headers();
    head.append("Authorization: ", "token " + token);

    const getContents = new Promise((res, rej) => {
      repo.getContents(null, "content", true, (err, files) => {
        if (err) { 
          return rej(err);
        }
        res(files)
      })
    })
    const fileRequests = (await getContents).map(file => fetch(file.downloadURL))
    return (await Promise.all(fileRequests)).map(res => res.text)
  } catch (err) {
    // handle error or..
    throw err;
  }
};

here's an example using async/await that uses a new promise to promisify a callback: 

const content = document.getElementById("content")
const result = document.getElementById("result")

async function example(){
  content.innerHTML = 'before promise';
  const getContents = new Promise((res, rej) => {
    setTimeout(()=> {
      res('done')
    }, 1000)
  })
  const res = await getContents
  content.innerHTML = res
  return res
}
example().then((res)=> {
  result.innerHTML = `I finished with <em>${res}</em> as a result`
})
<div id="content"></div>
<div id="result"></div>

Here's why I initially wrote the answer with a for...of loop that awaited each request. All promises are executed basically at once:

const content = document.getElementById("content")
const result = document.getElementById("result")
async function example() {
  const promises = []
  content.innerHTML = 'before for loop. each promise updates the content as it finishes.'
  for(let i = 0; i < 20; i++){
    const promise = new Promise((resolve, reject) => {
      setTimeout(()=> {
        content.innerHTML = `current value of i: ${i}`
        resolve(i)
      }, 1000)
    })
    promises.push(promise)
  }
  const results = await Promise.all(promises)
  return results
}

example().then(res => result.innerHTML= res.join(', '))
  content:
  <div id="content"></div>
  
  result:
  <div id="result"></div>
Chance
  • 11,043
  • 8
  • 61
  • 84
  • Thanks for your help I managed to [figure it out](https://stackoverflow.com/a/60710249/12785202). And just to clarify no the GitHub api doesn't return Promises but rather the function's last argument is meant to be a callback. – Kartik Nair Mar 16 '20 at 17:09
  • @KartikNair Unless you are using a very old version, I sincerely doubt that's the case. My guess is if you remove the callback function it will return a promise. – Chance Mar 16 '20 at 17:14
  • @ggorlen I actually meant to make note of that. Without knowing how many files are coming back, I took the more conservative path with this example. I was also mostly focused on trying to illustrate how async/await works. Having said that, you're right. This is different than what OP initially wrote. I'll update the code to be more in line with what he presented. – Chance Mar 16 '20 at 18:17
  • @KartikNair I updated my answer to be more in line with what you originally had. I appended a solution in the event that a promise is not returned when a callback is omitted (again, I doubt that). – Chance Mar 16 '20 at 20:06
  • Thank you @Chance you were completely right. My bad. Thanks for the info I'll use it in combination with the accepted answer to make my function. – Kartik Nair Mar 19 '20 at 13:06
0

I figured it out this is the revised code:

const getFiles = async function(token, reponame) {
  var gh = new GitHub({
    token: token
  });

  reponame = reponame.split("/");
  const repo = gh.getRepo(reponame[0], reponame[1]);

  let files = new Promise((res, rej) => {
    repo.getContents(null, "content", true, (err, files) => {
      if (err) rej(err);
      else res(files);
    });
  });

  let content = new Promise(res => {
    files.then(files => {
      const promises = files.reduce((result, file) => {
        if (file.name.endsWith(".md")) {
          result.push(
            new Promise((res, rej) => {
              repo.getContents(null, file.path, true, (err, content) => {
                if (err) rej(err);
                else
                  res({
                    path: file.path,
                    content: content
                  });
              });
            })
          );
        }
        return result;
      }, []);

      console.log(promises);

      res(
        Promise.all(
          promises.map(promise =>
            promise.then(file => {
              return file;
            })
          )
        )
      );
    });
  });

  return await content;
};

I still don't know if this is the "right" way to do it but it's working.

Dharman
  • 30,962
  • 25
  • 85
  • 135
Kartik Nair
  • 25
  • 2
  • 3
0

The await keyword can only be used with an async function. If you notice, that your await Promise.all(promises.map(promise => retFiles.push(promise))); is inside a function that passes files param in .then . Just make that function async and await will work inside the scope. Try the below code.

 const getFiles = async function(token, reponame) {
  var gh = new GitHub({
    token: token
  });

  reponame = reponame.split("/");
  const repo = gh.getRepo(reponame[0], reponame[1]);

  let head = new Headers();
  head.append("Authorization: ", "token " + token);

  const getContents = new Promise((res, rej) => {
    repo.getContents(null, "content", true, (err, files) => {
      if (err) rej(err);
      else return files;
    }).then( async (files) => {
      let promises = [
        files.map(
          file =>
            new Promise(res => {
              fetch(file.downloadURL).then(body => {
                res(body.text);
              });
            })
        )
      ];

      const retFiles = [];
      await Promise.all(promises.map(promise => retFiles.push(promise)));
      res(retFiles)
    });
  });

  return getContents;
};
ggorlen
  • 44,755
  • 7
  • 76
  • 106
Imtiyaz Shaikh
  • 425
  • 3
  • 7
0

There are many issues here. The code is very complicated; there's no need for all of these promises and layers of indirection and nesting to achieve what you need.

The pattern you're trying to do is very common:

  • Make a request to get a list of entities (files, users, urls...).
  • For each entity returned in that list, make another request to get more information for it.
  • Return the result as a promise (it has to be a promise, since async functions can only return promises).

The way to do this is to break the problem into stages. Use the await and async keywords instead of .then in most cases. To make the example reproducible, I'll use a scenario where we want to get user profiles for the most recent n gists created on GitHub--this is fundamentally equivalent to what you're doing and I'll leave it to you to extrapolate.

The first step is to get the initial list of entities (recently created gists):

const res = await fetch("https://api.github.com/gists/public");
const gists = await res.json();

Next, for each gist in our array of gists from 0..n, we need to fire off a request. It's important to ensure we're not serializing anything here using await:

const requests = gists.slice(0, n).map(gist =>
  fetch(`https://api.github.com/users/${gist.owner.login}`)
);

Now that all the requests are in flight, we need to wait until they complete. This is where Promise.all comes in:

const responses = await Promise.all(requests);

Last step is to get the JSON from each response, which requires another Promise.all:

return await Promise.all(responses.map(e => e.json()));

This is our final result which can be returned. Here's the code:

const getRecentGistCreators = async (n=1) => {
  try {
    const res = await fetch("https://api.github.com/gists/public");
    const gists = await res.json();
    const requests = gists.slice(0, n).map(gist =>
      fetch(`https://api.github.com/users/${gist.owner.login}`)
    );
    const responses = await Promise.all(requests);
    return await Promise.all(responses.map(e => e.json()));
  }
  catch (err) {
    throw err;
  }
};

(async () => {
  try {
    for (const user of await getRecentGistCreators(5)) {
      const elem = document.createElement("div");
      elem.textContent = user.name;
      document.body.appendChild(elem);
    }
  }
  catch (err) {
    throw err;
  }
})();

As a note of improvement, it'd be better to ask for JSON in the same stage as requests complete using an extra promise, but for simplicity's sake, we'll do it in two serial steps. As a design point, it's probably good to break the overburdened getRecentGistCreators into two separate steps as well.

ggorlen
  • 44,755
  • 7
  • 76
  • 106