- fixed a rate limit issue which was caused by requests not getting m…

…arked as finished (#695)

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)

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<any>): void {

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),