fix Maximum call stack size exceeded

[ghost]

hello. I met a problem of stackover flow as blow.

$ github-changes -v -o myorg-inc -r api-repo -b master -a --only-pulls --use-commit-body -z Asia/Tokyo
fetching commits
pulled commit data for tag -  1.90.0
pulled commit data for tag -  1.89.0
pulled commit data for tag -  1.81.0
 .
 .
 .
fetched 5000 commits
fetched 5100 commits
fetched 5200 commits
fetched 5300 commits
fetched all commits
error RangeError: Maximum call stack size exceeded
    at Array.forEach (native)
    at getAllReachableCommits (C:\Users\myname\AppData\Roaming\npm\node_modules\github-changes\bin\index.js:364:31)
    at C:\Users\myname\AppData\Roaming\npm\node_modules\github-changes\bin\index.js:367:14
    at Array.forEach (native)
    at getAllReachableCommits (C:\Users\myname\AppData\Roaming\npm\node_modules\github-changes\bin\index.js:364:31)
    at C:\Users\myname\AppData\Roaming\npm\node_modules\github-changes\bin\index.js:367:14
    at Array.forEach (native)
    at getAllReachableCommits (C:\Users\myname\AppData\Roaming\npm\node_modules\github-changes\bin\index.js:364:31)
    at C:\Users\myname\AppData\Roaming\npm\node_modules\github-changes\bin\index.js:367:14
    at Array.forEach (native)
stack RangeError: Maximum call stack size exceeded
    at Array.forEach (native)
    at getAllReachableCommits (C:\Users\myname\AppData\Roaming\npm\node_modules\github-changes\bin\index.js:364:31)
    at C:\Users\myname\AppData\Roaming\npm\node_modules\github-changes\bin\index.js:367:14
    at Array.forEach (native)
    at getAllReachableCommits (C:\Users\myname\AppData\Roaming\npm\node_modules\github-changes\bin\index.js:364:31)
    at C:\Users\myname\AppData\Roaming\npm\node_modules\github-changes\bin\index.js:367:14
    at Array.forEach (native)
    at getAllReachableCommits (C:\Users\myname\AppData\Roaming\npm\node_modules\github-changes\bin\index.js:364:31)
    at C:\Users\myname\AppData\Roaming\npm\node_modules\github-changes\bin\index.js:367:14
    at Array.forEach (native)

then, I like to suggest stackover flow workaround like this PR.

[lalitkapoor]

@haradayoshitsuguis I don't believe this actually fixes the problem, there is code that depends on this method executing synchronously. How about giving the code below a go and letting me know if it works. If it works for you I'll explore it further and update the method. The following code makes this logic iterative vs recursive:

var getAllReachableCommits = function(startingSha, store) {
  var shasToExplore = [startingSha];

  while(shasToExplore.length > 0) {
    sha = shasToExplore.shift();
    if (!commitsBySha[sha]) continue;
    store[sha] = true;

    commitsBySha[sha].parents.forEach(function(parent) {
      if (directDescendents[parent.sha]) return;
      if (store[parent.sha]) return; // don't revist commits we've explored
      shasToExplore.push(parent.sha);
    })
  }
}

[ghost]

@lalitkapoor thank you for your reply ! I tryed your code. but is's not working. a processing don't exit while loop Infinitely. I think shasToExplore.push(parent.sha) is shasToExplore.pop(parent.sha)
is it correct ? when I did that, working successfully.

[lalitkapoor]

@haradayoshitsuguis no, that’s not correct. It needs to be push. Can you add a console.log below the while loop that prints out shasToExplore and can you share that output? It should eventually terminate...it shouldn’t be looping through things it has already explored.

[lalitkapoor]

Also log to see how many times getAllReachableCommits is being called, maybe it is just taking Long on your repo for some reason, would be good to understand why

[ghost]

@lalitkapoor hello, I tried again with log output. (as below)

  var getAllReachableCommits = function(startingSha, store) {

    console.log('start getAllReachableCommits--------------');
    var shasToExplore = [startingSha];

    while(shasToExplore.length > 0) {

      console.log('shasToExplore.length: ');
      console.log(shasToExplore.length);
      console.log('shasToExplore: ');
      console.log(shasToExplore);

      sha = shasToExplore.shift();
      if (!commitsBySha[sha]) continue;
      store[sha] = true;

      commitsBySha[sha].parents.forEach(function(parent) {
        if (directDescendents[parent.sha]) return;
        if (store[parent.sha]) return; // don't revist commits we've explored
        shasToExplore.push(parent.sha);
      })
    }
  };

paste log on my gist. https://gist.github.com/haradayoshitsuguis/b7c963080a16f02263adeaf650a3fbaf#file-getallreachablecommits_failed

Then I found while loop is not Infinitely, but It did not end even after 20 minutes passed.

I thought it's my repo fault. (I tryed on another repo. the code you gave this time worked fine.)

this is not your problem. do not need to fix. sorry...

[lalitkapoor]

great! Good luck! I'll close this PR. Feel free to reach back out if you find a bug in github-changes. Thanks!

[ghost]

Thanks @lalitkapoor ! I will continue to use github-changes. keep in touch.