-
Notifications
You must be signed in to change notification settings - Fork 86
Conversation
Size Change: 0 B Total Size: 712 kB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we would cache anything that is ok == true
but then you would have to cache the status too.
Thats a better approach, will use that |
Something to consider is that there is a single cache across the whole system, so any changes to the cache file have to be backwards compatible. Which may make it impossible to cache status. |
@code-forger are you making this change? |
On second look, I think caching the status code is to much for this simple cache. This is a CDN cache, the CDN should always be responding with 200's, I don't see any other 2XX code it would be useful to cache. |
Agreed. Also it's not worth changing the cache format given the lack of benefit. |
Description
Prevent the caching and re-playing of non 200 reponses.
Motivation and Context
if there is a non-200 reponse, the cache will re-play the text content as a '200'. This means errors are stored in the cache and re-played incorrectly.
Simply do not cache the non 200
How Has This Been Tested?
TBT
Types of Changes
Checklist:
What is the Impact to Developers Using One App?