Skip to content

Commit 1605e53

Browse files
committed
address comments
Signed-off-by: Eric Pickard <piceri@github.com>
1 parent eca959e commit 1605e53

2 files changed

Lines changed: 17 additions & 10 deletions

File tree

pkg/deploymentrecord/client.go

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -184,11 +184,6 @@ func (c *Client) PostOne(ctx context.Context, record *DeploymentRecord) error {
184184
return errors.New("record cannot be nil")
185185
}
186186

187-
// Wait for request throttler
188-
if err := c.requestThrottler.Wait(ctx); err != nil {
189-
return fmt.Errorf("request throttler wait failed: %w", err)
190-
}
191-
192187
url := fmt.Sprintf("%s/orgs/%s/artifacts/metadata/deployment-record", c.baseURL, c.org)
193188

194189
body, err := json.Marshal(record)
@@ -209,6 +204,10 @@ func (c *Client) PostOne(ctx context.Context, record *DeploymentRecord) error {
209204
return err
210205
}
211206

207+
if err = c.requestThrottler.Wait(ctx); err != nil {
208+
return fmt.Errorf("request throttler wait failed: %w", err)
209+
}
210+
212211
// Reset reader position for retries
213212
bodyReader.Reset(body)
214213

@@ -334,8 +333,11 @@ func (c *Client) waitForServerRateLimit(ctx context.Context) error {
334333
"delay", delay.Round(time.Millisecond),
335334
)
336335

336+
timer := time.NewTimer(delay)
337+
defer timer.Stop()
338+
337339
select {
338-
case <-time.After(delay):
340+
case <-timer.C:
339341
return nil
340342
case <-ctx.Done():
341343
return fmt.Errorf("context cancelled during server rate limit wait: %w", ctx.Err())
@@ -408,8 +410,11 @@ func waitForBackoff(ctx context.Context, attempt int) error {
408410
}
409411

410412
// Wait with context cancellation support
413+
timer := time.NewTimer(delay)
414+
defer timer.Stop()
415+
411416
select {
412-
case <-time.After(delay):
417+
case <-timer.C:
413418
case <-ctx.Done():
414419
return fmt.Errorf("context cancelled during retry backoff: %w", ctx.Err())
415420
}

pkg/deploymentrecord/client_test.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -703,17 +703,19 @@ func TestPostOneRespectsRetryAfterAcrossGoroutines(t *testing.T) {
703703
time.Sleep(50 * time.Millisecond)
704704

705705
// Goroutine 2: must observe the shared backoff
706+
secondReqDone := make(chan struct{})
706707
start := time.Now()
707708
wg.Go(func() {
709+
defer close(secondReqDone)
708710
if err := client.PostOne(ctx, testRecord()); err != nil {
709711
t.Errorf("goroutine 2 error: %v", err)
710712
}
711713
})
712-
713-
wg.Wait()
714-
714+
// Measure only goroutine 2's duration
715+
<-secondReqDone
715716
elapsed := time.Since(start)
716717
if elapsed < 1500*time.Millisecond {
717718
t.Errorf("goroutine 2 should have waited for retry-after, but only waited %v", elapsed)
718719
}
720+
wg.Wait()
719721
}

0 commit comments

Comments
 (0)