Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If-Range header causes error #415

Closed
tongwang opened this issue Jun 20, 2024 · 7 comments
Closed

If-Range header causes error #415

tongwang opened this issue Jun 20, 2024 · 7 comments

Comments

@tongwang
Copy link

tongwang commented Jun 20, 2024

Somehow If-Range in the request header by pmtiles.js is set on second or subsequent page load against a .pmtiles file in Leaflet. If the ETag of this file has changed, the range request condition is not fulfilled and the HTTP server returns the full content with 200 status, resulting in error:

Error: Server returned no content-length header or content-length exceeding request. Check that your storage backend supports HTTP Byte Serving.
    at FetchSource.<anonymous> (pmtiles.js:1188:17)
    at Generator.next (<anonymous>)
    at fulfilled (pmtiles.js:25:26)


Screenshot 2024-06-20 at 5 07 04 PM

pmtiles.js version is 3.0.6
browsers tested: Edge, Chrome
Firefox and Safari do not set If-Range and thus do not have this issue.

@bdon
Copy link
Member

bdon commented Jun 21, 2024

Can you upload a running page reproducing this? What storage system or HTTP backend is being used?

@tongwang
Copy link
Author

tongwang commented Jun 24, 2024

We can just use this page: https://pmtiles.io/examples/leaflet.html

We can see the "If-Range" header in the request. You don't get an error from this page, because the content doesn't change. But if the content changes, "If_Range" condition would be false, then the full resource is sent back with a 200 OK status [1]. When this happens, pmtiles.js complains about content length exceeding request.

[1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-Range
Screenshot 2024-06-24 at 2 39 25 PM

@bdon
Copy link
Member

bdon commented Jun 24, 2024

OK, I can see Chrome is the only browser attaching the If-Range header which we don't seem to have any control over.

The ETags implementation on the client intentionally does not use If-Match, etc, and checks the response ETag for comparison to detect changes. #272

As is, we don't want to accept the whole file because that won't be a valid tile, so we may need to build another retry after that situation is detected. Then it becomes difficult to distinguish between this case for Chrome and an actual server without byte serving support.

@tongwang
Copy link
Author

In addition to Chrome, my Edge browser also has this issue.

Retry sounds like a good idea. If the server returns full content and the ETag changes, then retry. Otherwise (i.e. identical ETag) then it indicates server doesn't support byte range?

@bdon
Copy link
Member

bdon commented Jun 26, 2024

Just so I can understand the impact here - are you updating the PMTiles archive in-place often enough that this presents a common issue? It ought to be that once it fails as described, a hard-refresh is needed for the map to work again; but at least it doesn't accidentally consume the entire file, which could be many gigabytes.

Let me know if that matches how you observe this bug working; it should be a high priority to fix

@tongwang
Copy link
Author

My case is pretty unique as we generate pmtiles files upon request and cache them for a certain period of time. Subsequent requests will be served from the cache. Meanwhile the cached pmtiles files is touched as a way of cache refreshing. Nginx calculate the ETag based on file size and last modification time. As a result, ETag of my pmtiles keeps changing even their content remain the same.

I need to rethink my strategy as obviously it isn't going to work. I currently disable ETag in Nginx as a workaround.

As I said before, my case is unique. Maybe a hard-refresh is all we need to pass this error in rare occasion when pmtiles content does change. I am okay with closing this issue without fixing it. Thanks!

@bdon
Copy link
Member

bdon commented Aug 1, 2024

I'm going to close this for now as it isn't actionable, unless someone finds a way to change Chrome's If-Range behavior; thanks.

@bdon bdon closed this as completed Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants