fix(client-js): send page/perPage in logs queries when they are 0#18632
fix(client-js): send page/perPage in logs queries when they are 0#18632abhay-codes07 wants to merge 1 commit into
Conversation
listLogs and getLogForRun guarded page/perPage with a truthy check, so page: 0 (the first page) and perPage: 0 were dropped from the query string and the server fell back to its own defaults. Every other paginated method in the client guards with !== undefined; these two were the only sites using a truthy check. Change the four guards to !== undefined. Adds unit tests. Closes mastra-ai#18631
🦋 Changeset detectedLatest commit: 919580f The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@abhay-codes07 is attempting to deploy a commit to the Mastra Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughIn Fix zero pagination params in client-js log methods
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
PR triageLinked issue check passed (#18631). Mastra uses CodeRabbit for automated code reviews. Please address all feedback from CodeRabbit by either making changes to your PR or leaving a comment explaining why you disagree with the feedback. Since CodeRabbit is an AI, it may occasionally provide incorrect feedback. PR complexity score
Applied label: Changed test gateChanged Test Gate is pending. The |
|
@abhiaiyer91 kindly approve it since I found few good quality issues and because of limit reset I can't proceed with the fix |
What
client.listLogsandclient.getLogForRundrop thepageandperPagequery parameters when they are0, because they guard with a truthy check:page/perPageare optionalnumbers, soif (page)is falsy for0. Requesting the first page withpage: 0(orperPage: 0) never sends those values and the server falls back to its own defaults, returning the wrong page.Every other paginated method in the client guards with
!== undefined(listMemoryThreads,getMcpServers,listScoresByRunId, theresources/methods, etc.). These two log methods are the only sites using a truthy check.Change
Change the four guards in
listLogsandgetLogForRunto!== undefined.Tests
Added unit tests asserting
listLogsandgetLogForRunincludepage=0andperPage=0in the request URL. Both fail on the current code and pass after the fix. The existingclient.test.tssuite still passes.Closes #18631
cc @abhiaiyer91 who last touched this file. One-line-per-site change, no behavior change for non-zero values.
ELI5
This change fixes a bug where asking for the first page of logs could accidentally be ignored. Now, when
pageorperPageis set to0, the client still sends it properly instead of skipping it and using the server’s default.What changed
client.listLogsandclient.getLogForRunto keeppage=0andperPage=0in query strings by checking for!== undefinedinstead of truthy values.@mastra/client-js.Notes