Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

IOS: Offline pack is not working properly on next day when iPad is in offline. #9705

Closed
ThiyagarajanShivSankaran opened this issue Aug 4, 2017 · 16 comments
Labels
bug Core The cross-platform C++ core, aka mbgl offline
Milestone

Comments

@ThiyagarajanShivSankaran
Copy link

ThiyagarajanShivSankaran commented Aug 4, 2017

Platform: IOS
Mapbox SDK version: 3.6.0

Steps to trigger behavior

  1. Download all the offline packs.
  2. Leave the device offline until tomorrow, or change the date time in device to tomorrow.
  3. Then the offline pack is not loading properly.

Expected behavior

  1. The offline pack's should work even after the device is in offline for more than 3 to 4 days.

Actual behavior

  1. But it's not working every time we need to come online.

We have client's who will work only in offline for more than 2 days. In that case the offline packs should work , but it's not working. I think the tiles are expiring every day.

@Guardiola31337 Guardiola31337 added the iOS Mapbox Maps SDK for iOS label Aug 4, 2017
@boundsj boundsj added the offline label Aug 7, 2017
@ThiyagarajanShivSankaran
Copy link
Author

ThiyagarajanShivSankaran commented Aug 8, 2017

Please download the video file from dropbox for further reference of this issue,

https://www.dropbox.com/s/sdfs2y05ivrz9q3/mapbox%20offline%20pack%20issue.mov?dl=0

But, I have checked the same scenario using the older SDK v3.5.4, it's working there.

@kkaefer
Copy link
Contributor

kkaefer commented Aug 10, 2017

@ThiyagarajanShivSankaran thanks for reporting this bug

@kkaefer kkaefer added Core The cross-platform C++ core, aka mbgl and removed iOS Mapbox Maps SDK for iOS labels Aug 10, 2017
@kkaefer
Copy link
Contributor

kkaefer commented Aug 10, 2017

@ThiyagarajanShivSankaran it's hard to see this from the video; are you using raster tiles or vector tiles?

@ThiyagarajanShivSankaran
Copy link
Author

@kkaefer we are using vector tiles. Sorry for the video clarity can you download and check. Thanks.

@arnekaiser
Copy link
Contributor

This looks like #9211

I think the expires timestamps in the database are expired after some days.

@andrewljohnson
Copy link

I think we fixed the bulk of this problem with a simple change, in src/mbgl/tile/tile_loader_impl.hpp

template <typename T>
  void TileLoader<T>::loadedData(const Response& res) {
      if (res.error && res.error->reason != Response::Error::Reason::NotFound) {
 -        tile.setError(std::make_exception_ptr(std::runtime_error(res.error->message)));
 +        if (!(tile.isComplete() && tile.isRenderable())) {
 +          tile.setError(std::make_exception_ptr(std::runtime_error(res.error->message)));
 +        }
      } else if (res.notModified) {
          resource.priorExpires = res.expires;
          // Do not notify the tile; when we get this message, it already has the current

@ThiyagarajanShivSankaran
Copy link
Author

ThiyagarajanShivSankaran commented Aug 21, 2017

Can I know what is the status of this issue. We need this to be fixed immediately. Thanks.

@ThiyagarajanShivSankaran
Copy link
Author

ThiyagarajanShivSankaran commented Aug 29, 2017

@kkaefer @friedbunny @boundsj Status please ???

@friedbunny
Copy link
Contributor

@ThiyagarajanShivSankaran There’s been no change in the status of this issue — when someone has the opportunity to investigate further, they will post an update here. In the meantime, any additional information (or testing of proposed fixes) is much appreciated.

@arethasamuel
Copy link

@andrewljohnson is this fix in the latest release of mapbox sdk? 3.6.2? we will test this and get back to you

@andrewljohnson
Copy link

@arethasamuel not AFAIK

@arethasamuel
Copy link

@andrewljohnson @kkaefer @friedbunny @boundsj

this issue still exists in mapbox 3.6.2

  1. Turn on wifi --> Download all the offline packs.
  2. change the date time in device to tomorrow
  3. Open the device in airplane mode --> open the app again and the tiles are no longer showing

So i opened up the cache.db sqlite database and found out that the table: tiles has a column called expires and the value of the expires column has a date 12 hours After the offline pack was installed.

Is there a way to increase this time? (like at least by a year or something?)

@ThiyagarajanShivSankaran
Copy link
Author

ThiyagarajanShivSankaran commented Sep 5, 2017

Hi @friedbunny yeah the above fix is working.

@jmkiley
Copy link
Contributor

jmkiley commented Sep 12, 2017

@kkaefer do you think that the fix @andrewljohnson proposed in #9705 (comment) should be pulled into the SDK?

@kkaefer
Copy link
Contributor

kkaefer commented Sep 13, 2017

There are several underlying issues:

  • We're treating a 404 response/not found as a valid response meaning "there is no data here" instead of using a correct HTTP 204 No content response. E.g. here:
    } else if (responseCode == 204 || (responseCode == 404 && resource.kind == Resource::Kind::Tile)) {
  • When we have a tile successfully parsed/loaded (e.g. an expired tile from cache), subsequent attempts to refresh the tile fail, and overwrite the existing successfully parsed tile with empty data (this is what IOS: Offline pack is not working properly on next day when iPad is in offline. #9705 (comment) attempts to fix, although this fix doesn't work in situations where we're still parsing data (so the tile isn't renderable yet), then receive a revalidation error)

I attempted a refactor of our tile state system in https://github.com/mapbox/mapbox-gl-native/tree/make-stale-resources-renderable, but it's incomplete.

@friedbunny
Copy link
Contributor

This should be fixed by #10012 and 3.6.4, which will be available imminently.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Core The cross-platform C++ core, aka mbgl offline
Projects
None yet
Development

No branches or pull requests

9 participants