Skip to content

Commit 8cc5752

Browse files
http/longpoll: Fix bug and simplify implementation
This fixes a bug where if duration was set to 0 the context would expire instantly causing any context-aware function to stop.
1 parent a33e3a9 commit 8cc5752

File tree

2 files changed

+19
-35
lines changed

2 files changed

+19
-35
lines changed

http/longpoll/longpoll.go

+10-32
Original file line numberDiff line numberDiff line change
@@ -42,51 +42,29 @@ func Until(ctx context.Context, d time.Duration, fn LongPollFunc) (ok bool, err
4242
// budget is communicated via the provided context. This is a defence measure to not have accidental
4343
// long running routines. If no duration is given (0) the long poll will have exactly one execution.
4444
func (c Config) LongPollUntil(ctx context.Context, d time.Duration, fn LongPollFunc) (ok bool, err error) {
45-
until := time.Now()
45+
var cancel context.CancelFunc
4646

4747
if d != 0 {
48-
if d < c.MinWaitTime { // guard lower bound
49-
until = until.Add(c.MinWaitTime)
50-
} else if d > c.MaxWaitTime { // guard upper bound
51-
until = until.Add(c.MaxWaitTime)
52-
} else {
53-
until = until.Add(d)
48+
if d < c.MinWaitTime {
49+
d = c.MinWaitTime
50+
} else if d > c.MaxWaitTime {
51+
d = c.MaxWaitTime
5452
}
55-
}
5653

57-
fnCtx, cancel := context.WithDeadline(ctx, until)
58-
defer cancel()
54+
ctx, cancel = context.WithTimeout(ctx, d)
55+
defer cancel()
56+
}
5957

60-
loop:
6158
for {
62-
ok, err = fn(fnCtx)
63-
if err != nil {
59+
ok, err = fn(ctx)
60+
if ok || err != nil || d == 0 {
6461
return
6562
}
6663

67-
// fn returns true, break the loop
68-
if ok {
69-
break
70-
}
71-
72-
// no long polling
73-
if d <= 0 {
74-
break
75-
}
76-
77-
// long pooling desired?
78-
if !time.Now().Add(c.RetryTime).Before(until) {
79-
break
80-
}
81-
8264
select {
83-
// handle context cancelation
8465
case <-ctx.Done():
8566
return false, ctx.Err()
8667
case <-time.After(c.RetryTime):
87-
continue loop
8868
}
8969
}
90-
91-
return
9270
}

http/longpoll/longpoll_test.go

+9-3
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,13 @@ func TestLongPollUntilBounds(t *testing.T) {
3737

3838
func TestLongPollUntilNoTimeout(t *testing.T) {
3939
called := 0
40-
ok, err := Until(context.Background(), 0, func(context.Context) (bool, error) {
40+
ok, err := Until(context.Background(), 0, func(ctx context.Context) (bool, error) {
41+
f := func(ctx context.Context) {
42+
assert.NoError(t, ctx.Err())
43+
}
44+
45+
f(ctx)
46+
4147
called++
4248
return false, nil
4349
})
@@ -73,8 +79,8 @@ func TestLongPollUntilTimeout(t *testing.T) {
7379
return false, nil
7480
})
7581
assert.False(t, ok)
76-
assert.NoError(t, err)
77-
assert.Equal(t, 2, called)
82+
assert.Error(t, err)
83+
assert.GreaterOrEqual(t, called, 2)
7884
}
7985

8086
func TestLongPollUntilTimeoutWithContext(t *testing.T) {

0 commit comments

Comments
 (0)