Only cache successful or NotFound responses #3479

kkaefer · 2016-01-08T15:32:00Z

Right now, we're storing all sorts of responses into the cache, potentially overwriting existing successful responses. This means that a bad connection could slowly remove all cache items by simply requesting them.

We don't want other types of error end up in our cache, since it'll likely evict perfectly good content.

friedbunny · 2016-01-08T16:44:10Z

Nice! This fixes #2012, where 401s for bad access tokens were being cached.

Mapbox GL[DEBUG] {OnlineFileSource}[Setup]: response error: 6; https://api.mapbox.com/styles/v1/mapbox/streets-v8/sprite@2x.json?access_token=flaskjdflasdkjfalskdfjasd
Mapbox GL[DEBUG] {OnlineFileSource}[Setup]: NOT caching resource
Mapbox GL[ERROR] {Map}[Style]: Failed to load sprite: HTTP status code 401

jfirebaugh · 2016-01-08T17:12:40Z

test/storage/cache_response.cpp

@@ -50,3 +50,175 @@ TEST_F(Storage, CacheResponse) {

    loop.run();
 }
+
+// Make sure we don't store a bad server response into the cache


This comment should be "Make sure we do store 404 responses in the cache".

Fixed in 13fbf8b

[core] only cache successful or NotFound responses

a06f28d

We don't want other types of error end up in our cache, since it'll likely evict perfectly good content.

kkaefer self-assigned this Jan 8, 2016

kkaefer added the in progress label Jan 8, 2016

kkaefer merged commit a06f28d into master Jan 8, 2016

kkaefer deleted the 3479-only-cache-successful branch January 8, 2016 16:18

kkaefer removed the in progress label Jan 8, 2016

friedbunny mentioned this pull request Jan 8, 2016

{Map}[Style]: Failed to load [mapbox://mapbox.mapbox-streets-v6]: #2012

Closed

jfirebaugh reviewed Jan 8, 2016
View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Only cache successful or NotFound responses #3479

Only cache successful or NotFound responses #3479

kkaefer commented Jan 8, 2016

friedbunny commented Jan 8, 2016

jfirebaugh Jan 8, 2016

kkaefer Jan 11, 2016

Only cache successful or NotFound responses #3479

Only cache successful or NotFound responses #3479

Conversation

kkaefer commented Jan 8, 2016

friedbunny commented Jan 8, 2016

jfirebaugh Jan 8, 2016

Choose a reason for hiding this comment

kkaefer Jan 11, 2016

Choose a reason for hiding this comment