fix(api): bound v1 list endpoints with a page size (EN-1230) - #193
fix(api): bound v1 list endpoints with a page size (EN-1230)#193flemzord wants to merge 1 commit into
Conversation
The v1 listInstances and listWorkflows handlers passed a zero-value query, and bunpaginate applies no LIMIT when PageSize == 0 -- so each request loaded the entire workflow_instances / workflows table into memory. Read the page size via bunpaginate.GetPageSize (default 15, max 100, overridable with ?pageSize=), as the v2 handlers already do, while keeping the v1 flat-array response shape. Behavioural note: v1 list responses are now bounded to one page; clients needing more pass ?pageSize=. Adds TestListInstancesIsBounded.
|
Warning Review limit reached
More reviews will be available in 41 minutes and 58 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
flemzord
changed the title
| return func(w http.ResponseWriter, r *http.Request) { | ||
|
|
||
| workflows, err := backend.ListWorkflows(r.Context(), bunpaginate.OffsetPaginatedQuery[any]{}) | ||
| // Bound the query: without a page size, bunpaginate applies no LIMIT, |
There was a problem hiding this comment.
This PR applies the v2 pagination pattern to /v1/workflows and /v1/instances, but /v1/triggers/{triggerID}/occurrences still builds ListTriggersOccurrencesQuery without PageSize, while the v2 handler already calls bunpaginate.GetPageSize. Since this fix is meant to bound the v1 list endpoints, please include the occurrences handler too; otherwise that endpoint still runs with PageSize == 0 and no LIMIT.
|
Superseded by #199, which consolidates this change with the related reliability and safety fixes on top of the current main branch. |
1 participant
Problem (H11 — HIGH)
v1listInstancesandlistWorkflowspassed a zero-value paginated query. In go-libsbunpaginate,PageSize == 0applies no LIMIT, so each request loaded the entireworkflow_instances/workflowstable into memory and serialized it — an unbounded query on tables that grow without limit in production.Fix
Read the page size with
bunpaginate.GetPageSize(default 15, max 100, overridable via?pageSize=), exactly as the v2 handlers do, while keeping the v1 flat-array response shape.Behavioural note: v1 list responses are now bounded to one page (default 15); clients needing more pass
?pageSize=(up to 100). v1 has no cursor envelope, so this is the pragmatic bound for the legacy surface.Test
TestListInstancesIsBounded: 20 instances ⇒ default returns 15,?pageSize=5returns 5.Severity: HIGH.