mirror of
https://github.com/AmanTahiliani/box-box.git
synced 2026-08-07 19:56:18 -04:00
fix(#76): prevent false-fresh aggregate responses
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
@@ -26,8 +27,12 @@ type requestPacer struct {
|
||||
|
||||
// wait blocks until this caller's reserved slot arrives.
|
||||
func (p *requestPacer) wait() {
|
||||
_ = p.waitContext(context.Background())
|
||||
}
|
||||
|
||||
func (p *requestPacer) waitContext(ctx context.Context) error {
|
||||
if p == nil || p.interval <= 0 {
|
||||
return
|
||||
return nil
|
||||
}
|
||||
p.mu.Lock()
|
||||
now := time.Now()
|
||||
@@ -38,8 +43,20 @@ func (p *requestPacer) wait() {
|
||||
p.next = p.next.Add(p.interval)
|
||||
p.mu.Unlock()
|
||||
if sleep > 0 {
|
||||
time.Sleep(sleep)
|
||||
timer := time.NewTimer(sleep)
|
||||
defer timer.Stop()
|
||||
select {
|
||||
case <-timer.C:
|
||||
case <-ctx.Done():
|
||||
// Return the unused reservation so repeated bounded enrichment
|
||||
// cancellations do not leave pacing debt for later real requests.
|
||||
p.mu.Lock()
|
||||
p.next = p.next.Add(-p.interval)
|
||||
p.mu.Unlock()
|
||||
return ctx.Err()
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
type OpenF1Client struct {
|
||||
|
||||
@@ -2,6 +2,7 @@ package api
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
@@ -59,8 +60,14 @@ func retryAfter429(resp *http.Response) time.Duration {
|
||||
// Without this, concurrent fan-outs (championship hub, track prefetch) burst
|
||||
// past the free-tier limit and callers silently treat 429s as missing data.
|
||||
func (c *OpenF1Client) doPaced(req *http.Request) (*http.Response, error) {
|
||||
return c.doPacedContext(req.Context(), req)
|
||||
}
|
||||
|
||||
func (c *OpenF1Client) doPacedContext(ctx context.Context, req *http.Request) (*http.Response, error) {
|
||||
for attempt := 0; ; attempt++ {
|
||||
c.pacer.wait()
|
||||
if err := c.pacer.waitContext(ctx); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
resp, err := c.httpClient.Do(req)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
@@ -70,7 +77,14 @@ func (c *OpenF1Client) doPaced(req *http.Request) (*http.Response, error) {
|
||||
}
|
||||
delay := retryAfter429(resp)
|
||||
resp.Body.Close()
|
||||
time.Sleep(delay)
|
||||
timer := time.NewTimer(delay)
|
||||
select {
|
||||
case <-timer.C:
|
||||
case <-ctx.Done():
|
||||
timer.Stop()
|
||||
return nil, ctx.Err()
|
||||
}
|
||||
timer.Stop()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -83,13 +97,23 @@ func (c *OpenF1Client) doPaced(req *http.Request) (*http.Response, error) {
|
||||
// entry for this URL, that stale entry is returned instead of propagating the
|
||||
// error. The client's staleFlag is set so the UI can show a disclaimer.
|
||||
func (c *OpenF1Client) get(url string) (io.ReadCloser, error) {
|
||||
return c.getContext(context.Background(), url)
|
||||
}
|
||||
|
||||
// getContext is the cancellable form used by bounded optional web enrichment.
|
||||
// A caller cancellation never falls back to stale data: the work is no longer
|
||||
// relevant to that response and must stop instead of continuing in background.
|
||||
func (c *OpenF1Client) getContext(ctx context.Context, url string) (io.ReadCloser, error) {
|
||||
if err := ctx.Err(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
// 1. Check the cache for a fresh (non-expired) entry.
|
||||
if cachedData, ok := c.cache.Get(url); ok {
|
||||
return io.NopCloser(bytes.NewReader(cachedData)), nil
|
||||
}
|
||||
|
||||
// 2. Attempt a live network request.
|
||||
req, err := http.NewRequest("GET", url, nil)
|
||||
req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
|
||||
if err != nil {
|
||||
// Even a request-construction failure warrants a stale fallback.
|
||||
return c.tryStale(url, err)
|
||||
@@ -98,8 +122,11 @@ func (c *OpenF1Client) get(url string) (io.ReadCloser, error) {
|
||||
req.Header.Set("Authorization", "Bearer "+c.apiKey)
|
||||
}
|
||||
|
||||
resp, err := c.doPaced(req)
|
||||
resp, err := c.doPacedContext(ctx, req)
|
||||
if err != nil {
|
||||
if ctx.Err() != nil {
|
||||
return nil, ctx.Err()
|
||||
}
|
||||
return c.tryStale(url, err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
@@ -128,6 +155,9 @@ func (c *OpenF1Client) get(url string) (io.ReadCloser, error) {
|
||||
// 3. Success — read the body, store in cache, return.
|
||||
data, err := io.ReadAll(resp.Body)
|
||||
if err != nil {
|
||||
if ctx.Err() != nil {
|
||||
return nil, ctx.Err()
|
||||
}
|
||||
return c.tryStale(url, err)
|
||||
}
|
||||
|
||||
@@ -245,7 +275,13 @@ func (c *OpenF1Client) GetDriversForSession(sessionKey int) ([]models.Driver, er
|
||||
}
|
||||
|
||||
func (c *OpenF1Client) GetDriver(sessionKey, driverNumber int) (*models.Driver, error) {
|
||||
body, err := c.get(fmt.Sprintf("%s/v1/drivers?session_key=%d&driver_number=%d", c.url, sessionKey, driverNumber))
|
||||
return c.GetDriverContext(context.Background(), sessionKey, driverNumber)
|
||||
}
|
||||
|
||||
// GetDriverContext is a cancellable single-driver lookup for optional bounded
|
||||
// enrichment. Other public methods retain their existing background semantics.
|
||||
func (c *OpenF1Client) GetDriverContext(ctx context.Context, sessionKey, driverNumber int) (*models.Driver, error) {
|
||||
body, err := c.getContext(ctx, fmt.Sprintf("%s/v1/drivers?session_key=%d&driver_number=%d", c.url, sessionKey, driverNumber))
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"io"
|
||||
"net/http"
|
||||
@@ -49,6 +50,28 @@ func TestRequestPacerNilSafe(t *testing.T) {
|
||||
p.wait() // must not panic
|
||||
}
|
||||
|
||||
func TestRequestPacerCancellationReturnsUnusedReservation(t *testing.T) {
|
||||
p := &requestPacer{interval: 100 * time.Millisecond}
|
||||
if err := p.waitContext(context.Background()); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
p.mu.Lock()
|
||||
wantNext := p.next
|
||||
p.mu.Unlock()
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Millisecond)
|
||||
defer cancel()
|
||||
if err := p.waitContext(ctx); err == nil {
|
||||
t.Fatal("expected paced wait cancellation")
|
||||
}
|
||||
p.mu.Lock()
|
||||
gotNext := p.next
|
||||
p.mu.Unlock()
|
||||
if !gotNext.Equal(wantNext) {
|
||||
t.Fatalf("cancelled reservation left pacing debt: next %v, want %v", gotNext, wantNext)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetRetriesOn429(t *testing.T) {
|
||||
var calls atomic.Int32
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
Reference in New Issue
Block a user