Skip to content

Commit dd39fd4

Browse files
committed
fix(client): bound the list window at the call time
`List` never set `ReceivedBeforeMs`, and the gateway rejects any list where `received_at_or_after_ms >= received_before_ms` (`gateway/controller/list.go:85`). With `Since` unset the client sent `(0, 0)` and with `-since 1h` it sent `(now-1h, 0)` — both invalid, so every `list` call, and every `watch` that did not name its requests with `-sqid`, failed with InvalidArgument against a real gateway. Both bounds are now taken from a single `now` before the first page. Fixing them up front is required rather than tidy: the continuation token pins both bounds (`list.go:111`), so a bound recomputed per page would be rejected from the second page on.
1 parent d7fb1cf commit dd39fd4

2 files changed

Lines changed: 51 additions & 4 deletions

File tree

submitqueue/client/query.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ type ListQuery struct {
2929
// own queue.
3030
Queue string
3131

32-
// Since bounds the window to requests received within it. Zero reads from
33-
// the beginning of retained history.
32+
// Since bounds the window to requests received within it, ending at the
33+
// time of the call. Zero reads from the beginning of retained history.
3434
Since time.Duration
3535

3636
// Limit caps how many requests are returned across all pages. Zero means
@@ -52,9 +52,18 @@ func (c *Client) List(ctx context.Context, q ListQuery) ([]*pb.RequestSummary, e
5252
return nil, fmt.Errorf("queue must not be empty")
5353
}
5454

55-
req := &pb.ListRequest{Queue: q.Queue, PageSize: int32(q.PageSize)}
55+
// Both bounds are fixed before the first page. The gateway requires
56+
// received_at_or_after_ms < received_before_ms, so an unset upper bound
57+
// rejects every call, and its continuation token pins both bounds, so one
58+
// recomputed per page would be rejected from the second page on.
59+
now := time.Now()
60+
req := &pb.ListRequest{
61+
Queue: q.Queue,
62+
PageSize: int32(q.PageSize),
63+
ReceivedBeforeMs: now.UnixMilli(),
64+
}
5665
if q.Since > 0 {
57-
req.ReceivedAtOrAfterMs = time.Now().Add(-q.Since).UnixMilli()
66+
req.ReceivedAtOrAfterMs = now.Add(-q.Since).UnixMilli()
5867
}
5968

6069
var out []*pb.RequestSummary

submitqueue/client/query_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,42 @@ func TestListWithoutSinceLeavesTheWindowOpen(t *testing.T) {
108108
"no window means all retained history, not a bound of zero-time")
109109
}
110110

111+
func TestListClosesTheWindowAtTheCallTime(t *testing.T) {
112+
// The gateway rejects a list whose lower bound is not strictly below its
113+
// upper one, so leaving the upper bound unset fails every call.
114+
gw := &pagingGateway{pages: [][]string{{"q/1"}}}
115+
sq, stop := dial(t, gw)
116+
defer stop()
117+
118+
before := time.Now().UnixMilli()
119+
_, err := sq.List(context.Background(), ListQuery{Queue: "q"})
120+
require.NoError(t, err)
121+
after := time.Now().UnixMilli()
122+
123+
got := gw.lastRequest.GetReceivedBeforeMs()
124+
assert.GreaterOrEqual(t, got, before)
125+
assert.LessOrEqual(t, got, after)
126+
assert.Less(t, gw.lastRequest.GetReceivedAtOrAfterMs(), got,
127+
"the gateway requires received_at_or_after_ms < received_before_ms")
128+
}
129+
130+
func TestListKeepsTheWindowFixedAcrossPages(t *testing.T) {
131+
// The continuation token pins both bounds, so a bound recomputed per page
132+
// would be rejected from the second page on.
133+
gw := &pagingGateway{pages: [][]string{{"q/1"}, {"q/2"}, {"q/3"}}}
134+
sq, stop := dial(t, gw)
135+
defer stop()
136+
137+
_, err := sq.List(context.Background(), ListQuery{Queue: "q", Since: time.Hour})
138+
require.NoError(t, err)
139+
140+
require.Len(t, gw.requests, 3)
141+
for _, req := range gw.requests[1:] {
142+
assert.Equal(t, gw.requests[0].GetReceivedAtOrAfterMs(), req.GetReceivedAtOrAfterMs())
143+
assert.Equal(t, gw.requests[0].GetReceivedBeforeMs(), req.GetReceivedBeforeMs())
144+
}
145+
}
146+
111147
func TestRowsFromSummaries(t *testing.T) {
112148
received := time.Now().Add(-5 * time.Minute)
113149
rows := RowsFromSummaries([]*pb.RequestSummary{
@@ -143,10 +179,12 @@ type pagingGateway struct {
143179
alwaysToken bool
144180

145181
calls int
182+
requests []*pb.ListRequest
146183
lastRequest *pb.ListRequest
147184
}
148185

149186
func (g *pagingGateway) List(_ context.Context, req *pb.ListRequest) (*pb.ListResponse, error) {
187+
g.requests = append(g.requests, req)
150188
g.lastRequest = req
151189
page := g.calls
152190
g.calls++

0 commit comments

Comments
 (0)