fightmegg/riot-api

Unhandled Rejection

nilzgithub opened this issue · 5 comments

Hi @francois-blanchard @OllieJennings,

I started to use your library and faced a problem with an unhandled rejection after (as it seems) hitting the rate limit of the riot api:

  1. loop through an array of three (known) summoner names,
  2. rAPI.summoner.getBySummonerName for retrieving "puuid",
  3. rAPI.matchV5.getIdsbyPuuid for retrieving latest 15 matches,
  4. rAPI.matchV5.getMatchById for retrieving match participants and data
  5. rAPI.league.getEntriesBySummonerId for retrieving current ladder rankings (MMR/ELO).

Everthing seems to run fine, until the rate limit is kicking in. Ill get an unhandled promise, Ill catch it with the following listener:

process.on('unhandledRejection', (error, promise) => {
	console.log(error.stack);
	console.log(promise);
});

The rejected message:

Promise {
  <rejected> {
    status: 429,
    statusText: 'Too Many Requests',
    appLimits: '20:1,100:120',
    appCounts: '22:1,20:120',
    methodLimits: '250:10',
    methodCounts: '20:10',
    retryAfter: 2000,
    limitType: 'application'
  }
}

Do you know whats wrong here? It seems that the rate-limiter is not working properly.

@nilzgithub Hey, thanks for raising an issue, let me check it.

Apologies for not getting back soon, COVID got me for a 2nd time.

@nilzgithub just a quick question, can you see if you are hitting this: https://github.com/fightmegg/riot-rate-limiter/wiki/Max-Retries

Basically there is a retry limit when a 429 is encountered, explained here: https://github.com/fightmegg/riot-rate-limiter/wiki/429-Reponses

Hi @OllieJennings! No problem :) Get well soon - I had it once, it was horrible!

I'm pretty sure im not hitting the max retries or at least it is not waiting for the retryAfter-Header as it seems, if I count correctly considering two summoners who NEVER played in a match with each other:

  • 2 requests to the summoner endpoint for retrieving "puuids" (summoner.getBySummonerName)
  • 2 requests to the matchV5 endpoint to get the latest 15 matches by puuid (matchV5.getIdsbyPuuid)
  • 2 * 15 requests (calculating the worst case here, e.g. if the summoners are not having played any matches together) to the matchV5 endpoint (matchV5.getMatchById)
  • 2 (summoners) * 15 (matches) * 10 (participants (theoretically i could skip the known summoner(s) here. I already do a check if a summoner is already known and I got the MMR)) but since I'm talking about the worst case lets have 150 requests in total) to the league endpoint to retrieve MMR/ELO stats (league.getEntriesBySummonerId)

= 2 + 2 + 30 + 150 requests (correct me if Im wrong). All requests are made in around 2-5secs. I'm using the development key and rates are as follows:

20 requests in 1 sec.
100 requests in 2 minutes.

The rate limit should be hit very quickly, but it looks like if all requests are made and retryAfter is skipped, since after few seconds I'll get those unhandled rejections.

I noticed yesterday that at some point if the queue with requests adds up I get this message :

BottleneckError: A job with the same id already exists (id=euw1.league.getEntriesBySummonerId.<summonerId_here>) at Bottleneck._receive (/var/www/clients/client1/web85/web/backend/node_modules/bottleneck/lib/Bottleneck.js:420:21) at Bottleneck.schedule (/var/www/clients/client1/web85/web/backend/node_modules/bottleneck/lib/Bottleneck.js:488:12) at RiotRateLimiter.execute (/var/www/clients/client1/web85/web/backend/node_modules/@fightmegg/riot-rate-limiter/dist/cjs/index.js:97:29) at RiotAPI.request (/var/www/clients/client1/web85/web/backend/node_modules/@fightmegg/riot-api/dist/cjs/index.js:85:49) at runMicrotasks (<anonymous>) at processTicksAndRejections (node:internal/process/task_queues:96:5)

I guess thats because I need to find a way to keep track of which summonerIds are already in the queue and if so dont do the request again (do you have any idea how to do so?):

rAPI.league.getEntriesBySummonerId({
      region: region,
      summonerId: participant.summonerId,
}).then(laddersMMR =>{																            
     Object.entries(laddersMMR).forEach(([key, value]) => {																	 
           insertDTOObjectIntoTable('summoner_mmr', value, null, participant.puuid);
     });
).catch(err => {
     console.log(err);
}); 

Also I'm not pretty sure how to inject the config to the rate-limiter, as I'm working with the fightmegg/riot-api. Would it be enough to just set
const limiter = new RiotRateLimiter({ retryLimit: 4 });
once?

@nilzgithub From initial read through, i think you have found a potential issue, i think this library should handle duplicate requests, so that you don't get the error above.

Basically, i think it should check to see if a job with the identical ID already exists (in queue), if so, ignore it, as you are already requesting it.

@OllieJennings sounds good, but I would leave it open to what happens to the duplicate ID/request (or at least configurable):

  • ignore it,
  • execute anyway

One additional thing I would like to see is to list which jobs are currently on the stack (async) (or in queue), if not possible already.