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:
- loop through an array of three (known) summoner names,
- rAPI.summoner.getBySummonerName for retrieving "puuid",
- rAPI.matchV5.getIdsbyPuuid for retrieving latest 15 matches,
- rAPI.matchV5.getMatchById for retrieving match participants and data
- 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.