-
Notifications
You must be signed in to change notification settings - Fork 1.6k
MINOR: Handle empty cursor / offset in pagination #25533
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🔍 CI failure analysis for 031d595: CRITICAL: 10 of 11 CI jobs failed (91% failure rate) - pagination bug affects all test suites plus unrelated AWS test configuration issues.Issue SummaryCRITICAL: 10 of 11 CI jobs failed for this PR (91% failure rate) Integration Tests (Backend - Confirmed Related to PR):
End-to-End Tests (Frontend - Confirmed Related to Backend Slowness): Python Ingestion Tests (Backend - Potentially Related):
Maven Build Tests (Mixed - Pagination + Environment Issues):
Total Failures: 59 E2E timeouts + 3 integration tests + 4 Python tests + 1 app test + 3 AWS tests = 70+ test failures Root Cause (Pagination Bug)The integration test failures are directly caused by the PR's changes: The PR added Before PR:
After PR:
Impact: When empty strings reach
Location: The fix should be: // Line 1008 - Change "" to null
parseCursorMap(after == null || after.isEmpty() ? null : RestUtil.decodeCursor(after));Maven Build Test AnalysisAppsResourceTest.post_trigger_app_200:
AwsCredentialsUtilTest (3 errors):
Overall Assessment91% CI failure rate (10 of 11 jobs) - This PR is catastrophically failing:
All pagination-related failures trace to the same root cause: Passing Recommendation:
Code Review ✅ ApprovedClean, focused bug fix that properly handles empty cursor/offset strings in pagination. The defensive programming pattern is applied consistently across all affected code paths. Tip Comment OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
Describe your changes:
Fixes
I worked on ... because ...
Type of change:
Checklist:
Fixes <issue-number>: <short explanation>Summary by Gitar
RestUtil.decodeCursor()to preventIllegalArgumentExceptionwhen Base64 decoding empty cursorsEntityRepository.listAfter()to check both null and emptyafterparameters before processingJdbiUtils.getOffset()andWebAnalyticEventRepository.getOffset()before integer parsingThis will update automatically on new commits.