fix(networking): skip ETag cache for responses exceeding 4 MB to prevent OOM on large /offerings payloads#3658
Open
tsushanth wants to merge 1 commit into
Open
Conversation
…ent OOM HTTPResultWithETag.serialize() calls JSONObject.toString() which builds the entire response body into a single contiguous String. For large /offerings responses (e.g. those containing multiple V2 paywalls), this single allocation can reach tens of MB and throw OutOfMemoryError on memory-constrained devices (Java heap limit 128 MB, observed crash at ~37 MB allocation). Guard storeResult() with a payload-length check against a 4 MB ceiling. Responses larger than that are served to the caller normally but not written to SharedPreferences, so the problematic allocation never occurs. The limit is generous for typical /offerings payloads while well below the crash threshold seen in production. A WARNING log is emitted when a response is skipped so operators can detect abnormally large payloads. Fixes RevenueCat#3628
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ETagManager.storeResultcallsHTTPResultWithETag.serialize(), which internally callsJSONObject.toString()to build the entire response body as a single in-memoryString. For large/offeringsresponses — for example those containing multiple published V2 paywalls — this single contiguous allocation can reach tens of MB and throwOutOfMemoryErroron devices with a small Java heap cap (commonly 128 MB on low-end Android 10–12 hardware).The crash site from production:
The reporter observed 600+ fatal events over 8 days, spiking sharply after publishing 4 offerings with V2 paywalls. Removing those paywalls stopped the crashes immediately.
Fix
Add a payload-size guard at the top of
storeResult. Ifresult.payloadText.lengthexceedsMAX_CACHEABLE_PAYLOAD_BYTES(4 MB), the response is returned to the caller normally but skipped for caching, preventing the large allocation. AWARNINGlog is emitted so operators can detect abnormally large payloads. Normal-sized responses are unaffected.The 4 MB threshold is generous for typical
/offeringsresponses while well below the observed crash floor (~37 MB). It can be adjusted upward later if needed.MAX_CACHEABLE_PAYLOAD_BYTESisinternaland therefore directly accessible from the existingETagManagerTestfor unit-test coverage.Trade-offs
When a response is larger than the threshold the ETag short-circuit is unavailable for that URL: the next request will always go to the network and re-download the full response instead of receiving a
304 Not Modified. This is the correct safety/correctness trade-off — a cache miss is always recoverable, an OOM crash is not. The underlying/offeringsresponse size issue is separate and can be addressed by the backend team independently.Closes #3628
Note
Medium Risk
Touches core networking cache behavior for large responses; behavior change is intentional (skip cache vs crash) with limited blast radius for typical payloads under 4 MB.
Overview
ETagManager.storeResultnow checkspayloadTextlength before persisting. Responses overMAX_CACHEABLE_PAYLOAD_BYTES(4 MB) are still returned to callers but are not written to SharedPreferences, avoiding the largeJSONObject.toString()allocation duringHTTPResultWithETag.serializethat was causing production OOMs on huge/offeringspayloads.Oversized skips emit a
WARNINGvia the newNetworkStrings.ETAG_RESPONSE_TOO_LARGE_TO_CACHEmessage. Trade-off: those URLs lose ETag/304 caching until responses shrink, but avoid fatal crashes.Reviewed by Cursor Bugbot for commit 0755dc3. Bugbot is set up for automated code reviews on this repo. Configure here.