Skip to content

Commit 7754ae4

Browse files
committed
chore(search-sync-worker): address CodeRabbit nits on collection + drain helper
- spotlight.go / user_room.go: move the HistorySharedSince short-circuit ahead of the Accounts validation. Previously a restricted-room event with an empty Accounts slice returned an "empty accounts" error instead of being silently skipped like every other restricted event. Matches the event-level contract documented on InboxMemberEvent. - inbox_integration_test.go: surface batch.Error() after draining each fetch so mid-batch server errors (consumer deleted, leader change) fail the test with their real cause instead of a misleading "drained N of M" mismatch. https://claude.ai/code/session_01XTmSpmv5dT6UXX7NpRdYqN
1 parent 1e49129 commit 7754ae4

3 files changed

Lines changed: 18 additions & 6 deletions

File tree

search-sync-worker/inbox_integration_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,10 @@ func drainConsumer(
110110
handler.Add(msg)
111111
received++
112112
}
113+
// Surface any mid-batch error (consumer deleted, leader change,
114+
// transient connection). Without this, batch.Messages() just closes
115+
// silently and only the outer Equal would fail — losing the root cause.
116+
require.NoError(t, batch.Error(), "batch error after draining (attempt %d, received %d of %d)", attempts, received, expected)
113117
}
114118
require.Equal(t, expected, received, "drained %d of %d expected messages", received, expected)
115119
handler.Flush(ctx)

search-sync-worker/spotlight.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,16 @@ func (c *spotlightCollection) BuildAction(data []byte) ([]searchengine.BulkActio
5555
if payload.RoomID == "" {
5656
return nil, fmt.Errorf("build spotlight action: missing roomId")
5757
}
58-
if len(payload.Accounts) == 0 {
59-
return nil, fmt.Errorf("build spotlight action: empty accounts")
60-
}
58+
// Event-level restricted-room short-circuit runs BEFORE account
59+
// validation so restricted events with any payload shape (including an
60+
// empty Accounts slice) are uniformly skipped per the InboxMemberEvent
61+
// contract — no surprise error on an otherwise-valid "skip me" event.
6162
if payload.HistorySharedSince != 0 {
6263
return nil, nil
6364
}
65+
if len(payload.Accounts) == 0 {
66+
return nil, fmt.Errorf("build spotlight action: empty accounts")
67+
}
6468

6569
actions := make([]searchengine.BulkAction, 0, len(payload.Accounts))
6670
for i, account := range payload.Accounts {

search-sync-worker/user_room.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,16 @@ func (c *userRoomCollection) BuildAction(data []byte) ([]searchengine.BulkAction
8787
if payload.RoomID == "" {
8888
return nil, fmt.Errorf("build user-room action: missing roomId")
8989
}
90-
if len(payload.Accounts) == 0 {
91-
return nil, fmt.Errorf("build user-room action: empty accounts")
92-
}
90+
// Event-level restricted-room short-circuit runs BEFORE account
91+
// validation so restricted events with any payload shape (including an
92+
// empty Accounts slice) are uniformly skipped per the InboxMemberEvent
93+
// contract — no surprise error on an otherwise-valid "skip me" event.
9394
if payload.HistorySharedSince != 0 {
9495
return nil, nil
9596
}
97+
if len(payload.Accounts) == 0 {
98+
return nil, fmt.Errorf("build user-room action: empty accounts")
99+
}
96100

97101
ts := evt.Timestamp
98102
roomID := payload.RoomID

0 commit comments

Comments
 (0)