From 74f8c246605411a802ae788f2ddbae0c2cbb1821 Mon Sep 17 00:00:00 2001 From: Nicklas Ronge Date: Sat, 9 May 2020 17:57:42 +0200 Subject: [PATCH] - fixed a rate limit issue which was caused by requests not getting marked as finished (#695) --- CHANGELOG.md | 4 ++ .../poe/service/trade-rate-limit.service.ts | 46 ++++++++++++------- .../evaluate-search.component.ts | 16 +++++-- 3 files changed, 46 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19e44e3a..424fbc2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 0.6.28 (2020-05-09) + +- fixed a rate limit issue which was caused by requests not getting marked as finished (#695) + ## 0.6.27 (2020-05-08) - updated rate limiting behavior (#678) diff --git a/src/app/data/poe/service/trade-rate-limit.service.ts b/src/app/data/poe/service/trade-rate-limit.service.ts index 6f94e363..83aa5186 100644 --- a/src/app/data/poe/service/trade-rate-limit.service.ts +++ b/src/app/data/poe/service/trade-rate-limit.service.ts @@ -1,7 +1,7 @@ import { HttpResponse } from '@angular/common/http'; import { Injectable } from '@angular/core'; import { Observable, of, throwError } from 'rxjs'; -import { catchError, delay, flatMap, map, retryWhen } from 'rxjs/operators'; +import { catchError, delay, finalize, flatMap, map, retryWhen } from 'rxjs/operators'; // 1 request // x-rate-limit-ip: 12:4:10,16:12:300 @@ -62,9 +62,10 @@ interface TradeRateLimit { enum TradeRateThrottle { None = 1, - Stale = 2, - Reached = 3, - Limited = 4 + NoRules = 2, + Stale = 3, + Reached = 4, + Limited = 5 } @Injectable({ @@ -82,20 +83,22 @@ export class TradeRateLimitService { switch (reason) { case TradeRateThrottle.Limited: return throwError('limited'); + case TradeRateThrottle.NoRules: case TradeRateThrottle.Reached: case TradeRateThrottle.Stale: return throwError('waiting'); default: const request = this.createRequest(resource); - return getRequest().pipe( + return of(null).pipe( + flatMap(() => getRequest().pipe( + finalize(() => request.finished = Date.now()) + )), map(response => { - request.finished = Date.now(); this.updateRules(resource, response); this.filterRequests(resource); return response.body }), catchError(response => { - request.finished = Date.now(); if (response.status === 429) { this.updateRules(resource, response); } @@ -129,15 +132,20 @@ export class TradeRateLimitService { const { rules, requests, update } = this.getLimit(resource); const inflight = requests.some(request => !request.finished); - if (!inflight) { - return TradeRateThrottle.None; + if (!rules) { + if (!inflight) { + // allow a new request to gather rules + return TradeRateThrottle.None; + } + // request made for gathering rules + // do not allow any further requests + return TradeRateThrottle.NoRules; } - // only allow 1 request until - // > rules are filled - // > rules are fresh again const now = Date.now(); - if (!rules || (now - update) > RULE_FRESH_DURATION) { + if (inflight && (now - update) > RULE_FRESH_DURATION) { + // a request was made and data is stale + // wait for request to be finished before allowing further requests return TradeRateThrottle.Stale; } @@ -173,16 +181,22 @@ export class TradeRateLimitService { private filterRequests(resource: string): void { const limit = this.getLimit(resource); + + const { rules, requests } = limit; + if (!rules) { + return; + } + const now = Date.now(); - limit.requests = limit.requests.filter(request => { + limit.requests = requests.filter(request => { if (!request.finished) { return true; } - return limit.rules.some(rule => { + return rules.some(rule => { const min = now - rule.period * 1000; return request.finished >= min; }); - }) + }); } private updateRules(resource: string, response: HttpResponse): void { diff --git a/src/app/modules/evaluate/component/evaluate-search/evaluate-search.component.ts b/src/app/modules/evaluate/component/evaluate-search/evaluate-search.component.ts index 001dd3b4..f8dfe8cb 100644 --- a/src/app/modules/evaluate/component/evaluate-search/evaluate-search.component.ts +++ b/src/app/modules/evaluate/component/evaluate-search/evaluate-search.component.ts @@ -1,4 +1,4 @@ -import { ChangeDetectionStrategy, Component, EventEmitter, Input, OnInit, Output } from '@angular/core'; +import { ChangeDetectionStrategy, Component, EventEmitter, Input, OnDestroy, OnInit, Output } from '@angular/core'; import { BrowserService, LoggerService } from '@app/service'; import { EvaluateResult } from '@modules/evaluate/type/evaluate.type'; import { ItemSearchAnalyzeResult, ItemSearchAnalyzeService, ItemSearchListing, ItemSearchResult, ItemSearchService } from '@shared/module/poe/service'; @@ -15,8 +15,10 @@ import { EvaluateResultView, EvaluateUserSettings, EVALUATE_QUERY_DEBOUNCE_TIME_ styleUrls: ['./evaluate-search.component.scss'], changeDetection: ChangeDetectionStrategy.OnPush }) -export class EvaluateSearchComponent implements OnInit { +export class EvaluateSearchComponent implements OnInit, OnDestroy { + private searchSubscription: Subscription; private listSubscription: Subscription; + private analyzeSubscription: Subscription; public graph: boolean; @@ -60,6 +62,12 @@ export class EvaluateSearchComponent implements OnInit { } } + public ngOnDestroy(): void { + this.searchSubscription?.unsubscribe(); + this.listSubscription?.unsubscribe(); + this.analyzeSubscription?.unsubscribe(); + } + public onSearchClick(): void { this.initSearch(); } @@ -145,7 +153,7 @@ export class EvaluateSearchComponent implements OnInit { const options: ItemSearchOptions = { ...this.options }; - this.itemSearchService.search(item, options).pipe( + this.searchSubscription = this.itemSearchService.search(item, options).pipe( takeUntil(this.queryItemChange) ).subscribe(search => { this.search$.next(search); @@ -170,7 +178,7 @@ export class EvaluateSearchComponent implements OnInit { private analyze(listings: ItemSearchListing[], currency?: Currency): void { const currencies = currency ? [currency] : this.currencies; - this.itemSearchAnalyzeService.analyze(listings, currencies).pipe( + this.analyzeSubscription = this.itemSearchAnalyzeService.analyze(listings, currencies).pipe( takeUntil(this.queryItemChange) ).subscribe( result => this.result$.next(result),