diff --git a/.changeset/happy-tests-grow.md b/.changeset/happy-tests-grow.md new file mode 100644 index 0000000..e992259 --- /dev/null +++ b/.changeset/happy-tests-grow.md @@ -0,0 +1,5 @@ +--- +'styled-map-package-api': patch +--- + +Fix uncaught error in `download()` when the style URL is unreachable — errors now propagate through the returned stream instead of becoming unhandled rejections. Accept `Readonly` in public API type signatures. diff --git a/packages/api/lib/download.js b/packages/api/lib/download.js index 12f240b..58f6232 100644 --- a/packages/api/lib/download.js +++ b/packages/api/lib/download.js @@ -18,7 +18,7 @@ import { Writer } from './writer.js' * containing all the resources needed to serve the style offline. * * @param {object} opts - * @param {import("./utils/geo.js").BBox} opts.bbox Bounding box to download tiles for + * @param {Readonly} opts.bbox Bounding box to download tiles for * @param {number} opts.maxzoom Max zoom level to download tiles for * @param {string} opts.styleUrl URL of the style to download * @param { (progress: DownloadProgress) => void } [opts.onprogress] Optional callback for reporting progress @@ -74,14 +74,16 @@ export function download({ }) ;(async () => { - const style = await downloader.getStyle() - const writer = new Writer(style, { dedupe: !!dedupe }) - handleProgress({ style: { done: true } }) - // Pipe the output stream through the size counter (fire-and-forget; - // errors propagate via writer.abort()) - writer.outputStream.pipeTo(sizeCounter.writable).catch(() => {}) - + /** @type {Writer | undefined} */ + let writer try { + const style = await downloader.getStyle() + writer = new Writer(style, { dedupe: !!dedupe }) + handleProgress({ style: { done: true } }) + // Pipe the output stream through the size counter (fire-and-forget; + // errors propagate via writer.abort()) + writer.outputStream.pipeTo(sizeCounter.writable).catch(() => {}) + for await (const spriteInfo of downloader.getSprites()) { await writer.addSprite(spriteInfo) handleProgress({ @@ -111,7 +113,11 @@ export function download({ writer.finish() } catch (err) { - writer.abort(/** @type {Error} */ (err)) + if (writer) { + writer.abort(/** @type {Error} */ (err)) + } else { + sizeCounter.writable.abort(/** @type {Error} */ (err)) + } } })() diff --git a/packages/api/lib/style-downloader.js b/packages/api/lib/style-downloader.js index 7b7082d..5a3106f 100644 --- a/packages/api/lib/style-downloader.js +++ b/packages/api/lib/style-downloader.js @@ -279,7 +279,7 @@ export class StyleDownloader { * bytes downloaded. * * @param {object} opts - * @param {import('./utils/geo.js').BBox} opts.bounds + * @param {Readonly} opts.bounds * @param {number} opts.maxzoom * @param {(progress: TileDownloadStats) => void} [opts.onprogress] * @param {boolean} [opts.trackErrors=false] Include errors in the returned array of skipped tiles - this has memory overhead so should only be used for debugging. diff --git a/packages/api/lib/tile-downloader.js b/packages/api/lib/tile-downloader.js index e939c8d..7bfd426 100644 --- a/packages/api/lib/tile-downloader.js +++ b/packages/api/lib/tile-downloader.js @@ -24,11 +24,11 @@ import { noop } from './utils/misc.js' * * @param {object} opts * @param {string[]} opts.tileUrls Array of tile URL templates. Use `{x}`, `{y}`, `{z}` placeholders, and optional `{scheme}` placeholder which can be `xyz` or `tms`. - * @param {import('./utils/geo.js').BBox} opts.bounds Bounding box of the area to download + * @param {Readonly} opts.bounds Bounding box of the area to download * @param {number} opts.maxzoom Maximum zoom level to download * @param {(progress: TileDownloadStats) => void} [opts.onprogress] Callback to report download progress * @param {boolean} [opts.trackErrors=false] Include errors in the returned array of skipped tiles - this has memory overhead so should only be used for debugging. - * @param {import('./utils/geo.js').BBox} [opts.sourceBounds=MAX_BOUNDS] Bounding box of source data. + * @param {Readonly} [opts.sourceBounds=MAX_BOUNDS] Bounding box of source data. * @param {boolean} [opts.boundsBuffer=false] Buffer the bounds by one tile at each zoom level to ensure no tiles are missed at the edges. With this set to false, in most instances the map will appear incomplete when viewed because the downloaded tiles at lower zoom levels will not cover the map view area. * @param {number} [opts.minzoom=0] Minimum zoom level to download (for most cases this should be left as `0` - the size overhead is minimal, because each zoom level has 4x as many tiles) * @param {number} [opts.concurrency=8] Number of concurrent downloads (ignored if `fetchQueue` is provided) @@ -155,8 +155,8 @@ export function downloadTiles({ /** * * @param {object} opts - * @param {import('./utils/geo.js').BBox} [opts.bounds] - * @param {import('./utils/geo.js').BBox} [opts.sourceBounds] + * @param {Readonly} [opts.bounds] + * @param {Readonly} [opts.sourceBounds] * @param {boolean} [opts.boundsBuffer] * @param {number} [opts.minzoom] * @param {number} opts.maxzoom diff --git a/packages/api/lib/utils/geo.js b/packages/api/lib/utils/geo.js index ade4b2f..43bb29e 100644 --- a/packages/api/lib/utils/geo.js +++ b/packages/api/lib/utils/geo.js @@ -60,7 +60,7 @@ export function getTileUrl(urls, { x, y, z, scheme = 'xyz' }) { /** * Returns a bbox that is the smallest bounding box that contains all the input bboxes. * - * @param {[BBox, ...BBox[]]} bboxes + * @param {[Readonly, ...Readonly[]]} bboxes * @returns {BBox} Bounding Box [w, s, e, n] */ export function unionBBox(bboxes) { diff --git a/packages/api/test/download.js b/packages/api/test/download.js new file mode 100644 index 0000000..c00a9ce --- /dev/null +++ b/packages/api/test/download.js @@ -0,0 +1,220 @@ +import { ZipReader } from '@gmaclennan/zip-reader' +import { BufferSource } from '@gmaclennan/zip-reader/buffer-source' +import { afterAll, assert, beforeAll, describe, expect, test } from 'vitest' + +import { fileURLToPath } from 'node:url' + +import { download } from '../lib/download.js' +import { Reader } from '../lib/reader.js' +import { startSMPServer } from './utils/smp-server.js' +import { streamToBuffer } from './utils/stream-consumers.js' + +describe('download with demotiles-z2 (glyphs, no sprites)', () => { + /** @type {{ baseUrl: string, close: () => Promise }} */ + let server + + beforeAll(async () => { + const fixturePath = fileURLToPath( + new URL('./fixtures/demotiles-z2.smp', import.meta.url), + ) + server = await startSMPServer(fixturePath) + }) + + afterAll(async () => { + if (server) await server.close() + }) + + test('download produces a valid SMP file', async () => { + const smpStream = download({ + styleUrl: server.baseUrl + 'style.json', + bbox: [-180, -85, 180, 85], + maxzoom: 1, + }) + + const smp = await streamToBuffer(smpStream) + assert(smp.length > 0, 'output is non-empty') + + const reader = new Reader(await ZipReader.from(new BufferSource(smp))) + const style = await reader.getStyle() + + assert.equal(style.version, 8) + assert(Array.isArray(style.layers), 'has layers') + assert(style.metadata, 'has metadata') + assert(style.metadata['smp:bounds'], 'has smp:bounds') + assert(typeof style.metadata['smp:maxzoom'] === 'number', 'has smp:maxzoom') + assert(Object.keys(style.sources).length > 0, 'has sources') + + await reader.close() + }) + + test('download output contains readable tiles', async () => { + const smpStream = download({ + styleUrl: server.baseUrl + 'style.json', + bbox: [-180, -85, 180, 85], + maxzoom: 1, + }) + + const smp = await streamToBuffer(smpStream) + const reader = new Reader(await ZipReader.from(new BufferSource(smp))) + const style = await reader.getStyle() + + // Find the vector source and its tile path pattern + const vectorSource = /** @type {any} */ ( + Object.values(style.sources).find( + (/** @type {any} */ s) => s.type === 'vector', + ) + ) + assert(vectorSource, 'has vector source') + assert(vectorSource.tiles, 'vector source has tiles') + + // Read a z0 tile via the SMP URI pattern + const tilePath = vectorSource.tiles[0] + .replace('smp://maps.v1/', '') + .replace('{z}', '0') + .replace('{x}', '0') + .replace('{y}', '0') + const resource = await reader.getResource(tilePath) + assert(resource.contentLength > 0, 'tile has content') + await streamToBuffer(resource.stream) + + await reader.close() + }) + + test('download output contains readable glyphs', async () => { + const smpStream = download({ + styleUrl: server.baseUrl + 'style.json', + bbox: [-180, -85, 180, 85], + maxzoom: 0, + }) + + const smp = await streamToBuffer(smpStream) + const reader = new Reader(await ZipReader.from(new BufferSource(smp))) + const style = await reader.getStyle() + + assert(typeof style.glyphs === 'string', 'has glyphs URI') + + // Read a glyph resource + const resource = await reader.getResource( + 'fonts/Open Sans Semibold/0-255.pbf.gz', + ) + assert(resource.contentLength > 0, 'glyph has content') + await streamToBuffer(resource.stream) + + await reader.close() + }) + + test('download calls onprogress with expected fields', async () => { + /** @type {import('../lib/download.js').DownloadProgress[]} */ + const progressUpdates = [] + const smpStream = download({ + styleUrl: server.baseUrl + 'style.json', + bbox: [-180, -85, 180, 85], + maxzoom: 0, + onprogress: (p) => progressUpdates.push(structuredClone(p)), + }) + + await streamToBuffer(smpStream) + + assert(progressUpdates.length > 0, 'onprogress was called') + + const last = progressUpdates[progressUpdates.length - 1] + assert.equal(last.style.done, true, 'style done') + assert.equal(last.tiles.done, true, 'tiles done') + assert.equal(last.glyphs.done, true, 'glyphs done') + assert.equal(last.sprites.done, true, 'sprites done') + assert.equal(last.output.done, true, 'output done') + assert(last.output.totalBytes > 0, 'output has bytes') + assert(last.elapsedMs > 0, 'elapsedMs > 0') + }) + + test( + 'download stream emits error for unreachable URL', + { timeout: 15_000 }, + async () => { + const smpStream = download({ + styleUrl: 'http://127.0.0.1:1/nonexistent/style.json', + bbox: [-1, -1, 1, 1], + maxzoom: 0, + }) + + await expect(() => streamToBuffer(smpStream)).rejects.toThrow() + }, + ) +}) + +describe('download with osm-bright-z6 (sprites)', () => { + /** @type {{ baseUrl: string, close: () => Promise }} */ + let server + + beforeAll(async () => { + const fixturePath = fileURLToPath( + new URL('./fixtures/osm-bright-z6.smp', import.meta.url), + ) + server = await startSMPServer(fixturePath) + }) + + afterAll(async () => { + if (server) await server.close() + }) + + test('download with sprites produces valid SMP with sprite resources', async () => { + // Use a tight bbox and low maxzoom to keep this fast + const smpStream = download({ + styleUrl: server.baseUrl + 'style.json', + bbox: [10, 47, 11, 48], + maxzoom: 0, + }) + + const smp = await streamToBuffer(smpStream) + const reader = new Reader(await ZipReader.from(new BufferSource(smp))) + const style = await reader.getStyle() + + // Verify sprites are present in the output + assert(style.sprite, 'output style has sprite') + + // Read sprite resources + const spriteJsonResource = await reader.getResource( + 'sprites/default/sprite.json', + ) + assert(spriteJsonResource.contentLength > 0, 'sprite json has content') + await streamToBuffer(spriteJsonResource.stream) + + const spritePngResource = await reader.getResource( + 'sprites/default/sprite.png', + ) + assert(spritePngResource.contentLength > 0, 'sprite png has content') + await streamToBuffer(spritePngResource.stream) + + // Verify @2x sprites too + const sprite2xJsonResource = await reader.getResource( + 'sprites/default/sprite@2x.json', + ) + assert( + sprite2xJsonResource.contentLength > 0, + 'sprite @2x json has content', + ) + await streamToBuffer(sprite2xJsonResource.stream) + + await reader.close() + }) + + test('download with sprites tracks sprite progress', async () => { + /** @type {import('../lib/download.js').DownloadProgress[]} */ + const progressUpdates = [] + const smpStream = download({ + styleUrl: server.baseUrl + 'style.json', + bbox: [10, 47, 11, 48], + maxzoom: 0, + onprogress: (p) => progressUpdates.push(structuredClone(p)), + }) + + await streamToBuffer(smpStream) + + const last = progressUpdates[progressUpdates.length - 1] + assert.equal(last.sprites.done, true, 'sprites done') + assert( + last.sprites.downloaded > 0, + `sprites downloaded: ${last.sprites.downloaded}`, + ) + }) +}) diff --git a/packages/api/test/tile-downloader.js b/packages/api/test/tile-downloader.js new file mode 100644 index 0000000..b369403 --- /dev/null +++ b/packages/api/test/tile-downloader.js @@ -0,0 +1,291 @@ +import { afterAll, assert, beforeAll, describe, test } from 'vitest' + +import { createServer as createHTTPServer } from 'node:http' +import { fileURLToPath } from 'node:url' +import { promisify } from 'node:util' +import zlib from 'node:zlib' + +import { downloadTiles, tileIterator } from '../lib/tile-downloader.js' +import { startSMPServer } from './utils/smp-server.js' +import { streamToBuffer } from './utils/stream-consumers.js' + +const gzip = promisify(zlib.gzip) + +describe('tileIterator', () => { + test('generates correct tiles for global bounds z0-1', () => { + const tiles = [...tileIterator({ maxzoom: 1 })] + // z0: 1 tile (0,0,0), z1: 4 tiles (0,0,1), (0,1,1), (1,0,1), (1,1,1) + assert.equal(tiles.length, 5) + assert.deepEqual(tiles[0], { x: 0, y: 0, z: 0 }) + const z1Tiles = tiles.filter((t) => t.z === 1) + assert.equal(z1Tiles.length, 4) + }) + + test('minzoom skips lower zoom levels', () => { + const tiles = [...tileIterator({ minzoom: 1, maxzoom: 1 })] + assert.equal(tiles.filter((t) => t.z === 0).length, 0, 'no z0 tiles') + assert.equal(tiles.length, 4, 'only z1 tiles') + }) + + test('sourceBounds constrains tile output', () => { + const allTiles = [...tileIterator({ maxzoom: 2 })] + const constrained = [ + ...tileIterator({ + maxzoom: 2, + sourceBounds: [0, 0, 90, 45], + }), + ] + assert( + constrained.length < allTiles.length, + 'sourceBounds reduces tile count', + ) + // At z0, the single tile should still be yielded since the bounds overlap + assert(constrained.some((t) => t.z === 0)) + }) + + test('boundsBuffer adds extra tiles at edges', () => { + // boundsBuffer only has effect when sourceBounds is larger than bounds + const bounds = /** @type {const} */ ([10, 10, 20, 20]) + const sourceBounds = /** @type {const} */ ([-180, -85, 180, 85]) + const withoutBuffer = [ + ...tileIterator({ + bounds, + maxzoom: 3, + boundsBuffer: false, + sourceBounds, + }), + ] + const withBuffer = [ + ...tileIterator({ + bounds, + maxzoom: 3, + boundsBuffer: true, + sourceBounds, + }), + ] + assert( + withBuffer.length > withoutBuffer.length, + `buffer (${withBuffer.length}) > no buffer (${withoutBuffer.length})`, + ) + }) + + test('small bounds yields few tiles per zoom', () => { + // A very small area should yield ~1 tile per zoom + const bounds = /** @type {const} */ ([10, 10, 10.001, 10.001]) + const tiles = [...tileIterator({ bounds, maxzoom: 2 })] + // Should yield 1 tile per zoom level for this tiny area + for (let z = 0; z <= 2; z++) { + assert( + tiles.filter((t) => t.z === z).length >= 1, + `at least 1 tile at z${z}`, + ) + } + }) +}) + +describe('downloadTiles', () => { + /** @type {{ baseUrl: string, close: () => Promise }} */ + let server + /** @type {string[]} */ + let tileUrls + + beforeAll(async () => { + const fixturePath = fileURLToPath( + new URL('./fixtures/demotiles-z2.smp', import.meta.url), + ) + server = await startSMPServer(fixturePath) + const res = await fetch(server.baseUrl + 'style.json') + const style = await res.json() + const vectorSource = Object.values(style.sources).find( + (/** @type {any} */ s) => s.type === 'vector', + ) + tileUrls = /** @type {any} */ (vectorSource).tiles + }) + + afterAll(async () => { + if (server) await server.close() + }) + + test('downloads tiles and yields [stream, tileInfo] tuples', async () => { + const tiles = downloadTiles({ + tileUrls, + bounds: /** @type {const} */ ([-180, -85, 180, 85]), + maxzoom: 1, + }) + + let count = 0 + for await (const [stream, tileInfo] of tiles) { + const buf = await streamToBuffer(stream) + assert(buf.length > 0, 'tile buffer is non-empty') + assert(typeof tileInfo.z === 'number') + assert(typeof tileInfo.x === 'number') + assert(typeof tileInfo.y === 'number') + assert(typeof tileInfo.format === 'string') + count++ + } + assert(count > 0, 'at least one tile was downloaded') + }) + + test('MVT tiles are gzipped', async () => { + const tiles = downloadTiles({ + tileUrls, + bounds: /** @type {const} */ ([-180, -85, 180, 85]), + maxzoom: 0, + }) + + for await (const [stream, tileInfo] of tiles) { + const buf = await streamToBuffer(stream) + assert.equal(tileInfo.format, 'mvt') + // Gzip magic bytes + assert.equal(buf[0], 0x1f, 'first byte is gzip magic') + assert.equal(buf[1], 0x8b, 'second byte is gzip magic') + } + }) + + test('stats and skipped properties', async () => { + const tiles = downloadTiles({ + tileUrls, + bounds: /** @type {const} */ ([-180, -85, 180, 85]), + maxzoom: 1, + }) + + for await (const [stream] of tiles) { + await streamToBuffer(stream) + } + + assert(tiles.stats.total > 0, 'total > 0') + assert(tiles.stats.downloaded > 0, 'some tiles downloaded') + assert(tiles.stats.totalBytes > 0, 'totalBytes > 0') + }) + + test('onprogress callback is called', async () => { + /** @type {import('../lib/tile-downloader.js').TileDownloadStats[]} */ + const progressUpdates = [] + const tiles = downloadTiles({ + tileUrls, + bounds: /** @type {const} */ ([-180, -85, 180, 85]), + maxzoom: 1, + onprogress: (stats) => progressUpdates.push({ ...stats }), + }) + + for await (const [stream] of tiles) { + await streamToBuffer(stream) + } + + assert(progressUpdates.length > 0, 'onprogress was called') + const last = progressUpdates[progressUpdates.length - 1] + assert(last.total > 0) + assert(last.downloaded > 0) + }) + + test('handles 404 tiles gracefully', async () => { + const tiles = downloadTiles({ + tileUrls: [server.baseUrl + 's/nonexistent/{z}/{x}/{y}.mvt.gz'], + bounds: /** @type {const} */ ([-180, -85, 180, 85]), + maxzoom: 0, + }) + + let count = 0 + for await (const [stream] of tiles) { + await streamToBuffer(stream) + count++ + } + + assert.equal(count, 0, 'no tiles yielded') + assert(tiles.skipped.length > 0, 'skipped has entries') + }) + + test('trackErrors includes error objects in skipped', async () => { + const tiles = downloadTiles({ + tileUrls: [server.baseUrl + 's/nonexistent/{z}/{x}/{y}.mvt.gz'], + bounds: /** @type {const} */ ([-180, -85, 180, 85]), + maxzoom: 0, + trackErrors: true, + }) + + for await (const [stream] of tiles) { + await streamToBuffer(stream) + } + + assert(tiles.skipped.length > 0) + assert(tiles.skipped[0].error instanceof Error, 'error is an Error') + }) +}) + +describe('downloadTiles without Content-Type header', () => { + /** @type {{ baseUrl: string, close: () => Promise }} */ + let smpServer + /** @type {import('node:http').Server} */ + let noCtServer + /** @type {string} */ + let noCtBaseUrl + /** @type {Buffer} */ + let tileBuffer + + beforeAll(async () => { + // Start SMP server to get a real tile + const fixturePath = fileURLToPath( + new URL('./fixtures/demotiles-z2.smp', import.meta.url), + ) + smpServer = await startSMPServer(fixturePath) + + // Fetch a real tile and re-gzip it (fetch auto-decompresses) + const res = await fetch(smpServer.baseUrl + 'style.json') + const style = await res.json() + const vectorSource = /** @type {any} */ ( + Object.values(style.sources).find( + (/** @type {any} */ s) => s.type === 'vector', + ) + ) + const tileUrl = vectorSource.tiles[0] + .replace('{z}', '0') + .replace('{x}', '0') + .replace('{y}', '0') + const tileRes = await fetch(tileUrl) + const rawTile = Buffer.from(await tileRes.arrayBuffer()) + // Re-gzip so magic bytes (0x1f, 0x8b) are present for detection + tileBuffer = await gzip(rawTile) + + // Start a server that serves tiles without Content-Type + noCtServer = createHTTPServer((req, res) => { + const match = req.url?.match(/\/(\d+)\/(\d+)\/(\d+)\.tile/) + if (match) { + // Deliberately omit Content-Type header + res.writeHead(200, { 'Content-Length': String(tileBuffer.length) }) + res.end(tileBuffer) + } else { + res.writeHead(404) + res.end() + } + }) + await /** @type {Promise} */ ( + new Promise((resolve) => noCtServer.listen(0, resolve)) + ) + const { port } = /** @type {import('node:net').AddressInfo} */ ( + noCtServer.address() + ) + noCtBaseUrl = `http://localhost:${port}/` + }) + + afterAll(async () => { + if (smpServer) await smpServer.close() + if (noCtServer) await new Promise((resolve) => noCtServer.close(resolve)) + }) + + test('falls back to magic byte detection when no Content-Type', async () => { + const tiles = downloadTiles({ + tileUrls: [noCtBaseUrl + '{z}/{x}/{y}.tile'], + bounds: /** @type {const} */ ([-180, -85, 180, 85]), + maxzoom: 0, + }) + + let count = 0 + for await (const [stream, tileInfo] of tiles) { + const buf = await streamToBuffer(stream) + assert(buf.length > 0, 'tile has content') + assert.equal(tileInfo.format, 'mvt', 'detected as mvt from magic bytes') + count++ + } + assert(count > 0, 'at least one tile downloaded') + }) +})