Colorfulstan / LeagueJS

A Javascript Wrapper for the League of Legends API
MIT License
100 stars 7 forks source link

Second call to league.Match.gettingById invisbly fails #20

Open bencooper222 opened 5 years ago

bencooper222 commented 5 years ago

I'm writing a basic tool to grab a list of matches from the past 24 hours and just console.log them right now. I'm successfully getting the matchlist with the encrypted summoner ID. Then, I'm getting the detailed data by getting with the matchID. I'm trying to do this for the entire matchlist but it only works for the first one in the matchlist. Running through a debugger, it appears that the code just mysteriously (and without an error) crashes at MatchEndpoint.js:48.

I'm sure that getting a summoner's list of matches and the details about them is a fairly solved challenge so I'm guessing this issue doesn't actually require a fix. That said, it seems like something wonky is going on with my use of your library - not my use of Promises and async/await more broadly.

My project is will be open so I have the luxury of sharing my code with you:

const league = new (require('leaguejs'))(process.env.LEAGUE_KEY);
const { season, queue } = require('./constants');

exports.getSummonerDataSince = async beginTime => {
  const matchList = (await league.Match.gettingListByAccount(
    process.env.LEAGUE_ACCOUNT_ID,
    process.env.LEAGUE_API_PLATFORM_ID,
    { beginTime },
  )).matches;

  const data = [];

  for (let match of matchList) data.push(await getGameData(match.gameId));

  return data;
};

const getGameData = async matchId => {
  console.log('start fetch');
  const fullData = await league.Match.gettingById(matchId);
  console.log('data fetched');
  return {
    season: season(fullData.seasonId),
    queue: queue(fullData.queueId),
    duration: fullData.gameDuration,
  };
};

// this code only printed out "start fetch", "data fetched", "start fetch"
Colorfulstan commented 5 years ago

Hi, thanks for reporting this.

Can you check if it works with 2.0.x (instead of 2.1.x)?

I changed something in the Api request, but it would be weird if that's the reason.

I'll check my own usage next week or sunday as Im using it for this use case too, although I think I use Bluebird.all/.map to resolve the matches instead of pushing awaits into an array. Shouldn't make a difference in theory.

bencooper222 commented 5 years ago

I don't think it's working any differently

Colorfulstan commented 5 years ago

Please try anyways, as it's no effort.

bencooper222 commented 5 years ago

I did try but I've since reworked my codebase to just make the RESTful calls directly to the service so I was trying these in a sample file with the newer and older version. It's possible there were differences between the sample file and my actual codebase. That said, I'm pretty sure both versions fail.

Colorfulstan commented 5 years ago

Ok, thanks. Ill check if I can reproduce with your sample code.

bencooper222 commented 5 years ago

Might be helpful; constants file imported in code: https://github.com/bencooper222/write-league-games-spreadsheet/blob/master/src/constants.js

Colorfulstan commented 5 years ago

The Issue is somehow related to the ratelimiter usign SPREAD Strategy. Im not sure if I get to look into that right away (I would like to rework that anyways...)

If you use burst (by passing into the leagueJS constructor const league = new (require('leaguejs'))(process.env.LEAGUE_KEY, {limits: {allowBursts:true}}); It works fine.

Thanks again for finding and reporting this issue.

Colorfulstan commented 5 years ago

For my own reference: https://github.com/Colorfulstan/RiotRateLimiter-node/issues/10