🐛 bug: preserve idempotency replay protection for oversized responses#4287
🐛 bug: preserve idempotency replay protection for oversized responses#4287gaby wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
WalkthroughThe idempotency middleware no longer gates caching decisions on the application BodyLimit configuration. A prior guard that skipped storing responses when body size exceeded the limit is removed, and the corresponding test is updated to verify that caching now succeeds for oversized bodies. ChangesIdempotency Caching for Large Bodies
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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 |
There was a problem hiding this comment.
Pull request overview
Fixes an idempotency middleware regression where successful responses exceeding the app's BodyLimit were not cached, allowing duplicate requests with the same idempotency key to re-execute handler side effects. With the early return removed, all successful responses (regardless of size) are marshaled and stored so replays are served from the cache.
Changes:
- Remove the
BodyLimitsize check that skipped caching of oversized successful responses inmiddleware/idempotency/idempotency.go. - Rename and invert the regression test (
Test_New_SkipCache_WhenBodyTooLarge→Test_New_Cache_WhenBodyTooLarge) to assert oversized responses are cached and replayed.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| middleware/idempotency/idempotency.go | Removes the BodyLimit early return so oversized successful responses are still persisted for replay. |
| middleware/idempotency/idempotency_test.go | Updates the regression test to assert that the oversized response is cached once and replayed on retry. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4287 +/- ##
==========================================
+ Coverage 91.21% 91.26% +0.05%
==========================================
Files 130 130
Lines 12760 12793 +33
==========================================
+ Hits 11639 11676 +37
+ Misses 709 705 -4
Partials 412 412
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request removes the logic in the idempotency middleware that previously skipped caching for response bodies exceeding the application's configured body limit. The associated unit tests have been updated to verify that oversized responses are now correctly cached. I have no feedback to provide.
Motivation
BodyLimitafter the unsafe handler ran, allowing retries with the same idempotency key to re-execute side effects.Description
BodyLimit-based early return inmiddleware/idempotency/idempotency.goso successful handler responses are always marshaled and stored for idempotency replay protection.Test_New_SkipCache_WhenBodyTooLarge→Test_New_Cache_WhenBodyTooLargeinmiddleware/idempotency/idempotency_test.goto assert oversized responses are cached and replayed (assertscount == 1,s.setCount == 1, andWasPutToCacheis true for the initial request).