Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
- Show a grab cursor over draggable markers, including on non-interactive maps ([#8019](https://github.com/maplibre/maplibre-gl-js/issues/8019)) (by [@hugosmoreira](https://github.com/hugosmoreira))

### 🐞 Bug fixes

- Surface worker script load failures through the map `error` event with the resolved worker URL and bundler migration guidance ([#8018](https://github.com/maplibre/maplibre-gl-js/issues/8018)) (by [@hugosmoreira](https://github.com/hugosmoreira))
- Use `role=img` for non-interactive default markers and `role=button` when they become interactive ([#7790](https://github.com/maplibre/maplibre-gl-js/issues/7790)) (by [@cat0825](https://github.com/cat0825))
- Fix an error thrown when a paint property transitions between arrays of different length ([#6606](https://github.com/maplibre/maplibre-gl-js/issues/6606)) (by [@HarelM](https://github.com/HarelM))
- Fix renderer crash when `RasterTileSource.setTiles`/`setUrl` is called while the source contains errored tiles ([#7911](https://github.com/maplibre/maplibre-gl-js/pull/7911)) (by [@lazerg](https://github.com/lazerg))
Expand Down
4 changes: 3 additions & 1 deletion src/style/style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,9 @@ export class Style extends Evented<MapEventType> {
super();

this.map = map;
this.dispatcher = new Dispatcher(getGlobalWorkerPool(), map._getMapId());
this.dispatcher = new Dispatcher(getGlobalWorkerPool(), map._getMapId(), (error) => {
this.fire(new ErrorEvent(error));
});
this.dispatcher.registerMessageHandler(MessageType.getGlyphs, (mapId, params) => {
return this.getGlyphs(mapId, params);
});
Expand Down
35 changes: 35 additions & 0 deletions src/style/style_worker_error.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import {afterEach, describe, expect, test, vi} from 'vitest';
import {Style} from './style.ts';
import {WorkerPool} from '../util/worker_pool.ts';
import {StubMap} from '../util/test/util.ts';

describe('Style worker errors', () => {
afterEach(() => {
vi.restoreAllMocks();
});

test('forwards worker script load failures to the map error event', async () => {
WorkerPool.workerCount = 1;
const worker = new EventTarget() as Worker;
worker.postMessage = vi.fn();
worker.terminate = vi.fn();
vi.spyOn(globalThis, 'Worker').mockImplementation(function() {
return worker;
});
const map = new StubMap();
const style = new Style(map as any);
style.setEventedParent(map);
const errorPromise = map.once('error');
await style.dispatcher.actorsPromise;

worker.dispatchEvent(new ErrorEvent('error'));
const event = await errorPromise;

expect(event.error.message).toBe(
`Failed to load the MapLibre worker script: ${globalThis.location.href}\n` +
'If you use a bundler, set the worker URL explicitly; see ' +
'https://maplibre.org/maplibre-gl-js/docs/guides/v5-to-v6-migration-guide/'
);
style._remove();
});
});
4 changes: 3 additions & 1 deletion src/util/actor.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import {describe, test, expect, vi} from 'vitest';
import {Actor, type ActorTarget} from './actor.ts';
import {type WorkerGlobalScopeInterface, workerFactory} from './web_worker.ts';
import {type WorkerGlobalScopeInterface, workerFactory as createWorker} from './web_worker.ts';
import {sleep} from './test/util.ts';
import {ABORT_ERROR, AbortError} from './abort_error.ts';
import {MessageType} from './actor_messages.ts';

const workerFactory = () => createWorker(vi.fn());

describe('Actor', () => {
test('removes "abort" event listener from signal on reject', async () => {
const worker = await workerFactory() as any as WorkerGlobalScopeInterface & ActorTarget;
Expand Down
19 changes: 17 additions & 2 deletions src/util/dispatcher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {WorkerPool} from './worker_pool.ts';

describe('Dispatcher', () => {
test('requests and releases workers from pool', async () => {
const workers = [await workerFactory(), await workerFactory()];
const workers = [await workerFactory(vi.fn()), await workerFactory(vi.fn())];
const mapId = 1;
const releaseCalled = [];
const workerPool = {
Expand All @@ -32,7 +32,7 @@ describe('Dispatcher', () => {
const releaseCalled = [];
const workerPool = {
async acquire () {
workers ||= [await workerFactory(), await workerFactory()];
workers ||= [await workerFactory(vi.fn()), await workerFactory(vi.fn())];
return workers;
},
release (id) {
Expand Down Expand Up @@ -71,4 +71,19 @@ describe('Dispatcher', () => {
dispatcher.remove();
expect(actorsRemoved).toHaveLength(4);
});

test('reports worker construction failures', async () => {
const error = new Error('worker construction failed');
const workerPool = {
acquire () {
return Promise.reject(error);
}
} as any as WorkerPool;
const onWorkerError = vi.fn();

const dispatcher = new Dispatcher(workerPool, 1, onWorkerError);

await expect(dispatcher.actorsPromise).rejects.toBe(error);
expect(onWorkerError).toHaveBeenCalledWith(error);
});
});
13 changes: 11 additions & 2 deletions src/util/dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {getGlobalWorkerPool} from './global_worker_pool.ts';
import {GLOBAL_DISPATCHER_ID, makeRequest} from './ajax.ts';

import type {WorkerPool} from './worker_pool.ts';
import type {WorkerErrorHandler} from './web_worker.ts';
import type {RequestResponseMessageMap} from './actor_messages.ts';
import {MessageType} from './actor_messages.ts';

Expand All @@ -16,18 +17,26 @@ export class Dispatcher {
currentActor: number;
id: string | number;
private removed: boolean;
private onWorkerError?: WorkerErrorHandler;

constructor(workerPool: WorkerPool, mapId: string | number) {
constructor(workerPool: WorkerPool, mapId: string | number, onWorkerError?: WorkerErrorHandler) {
this.workerPool = workerPool;
this.actors = [];
this.currentActor = 0;
this.id = mapId;
this.removed = false;
this.onWorkerError = onWorkerError;
this.actorsPromise = this.initActors(mapId);
}

private async initActors(mapId: string | number): Promise<Actor[]> {
const workers = await this.workerPool.acquire(mapId);
let workers;
try {
workers = await this.workerPool.acquire(mapId, this.onWorkerError);
} catch (error) {
this.onWorkerError?.(error instanceof Error ? error : new Error(String(error)));
throw error;
}
if (this.removed) return [];
this.actors = workers.map((worker: ActorTarget, i: number) => {
const actor = new Actor(worker, mapId);
Expand Down
123 changes: 97 additions & 26 deletions src/util/web_worker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,61 +2,74 @@ import {describe, test, expect, beforeEach, afterEach, vi} from 'vitest';
import {workerFactory} from './web_worker.ts';
import {config} from './config.ts';

const WORKER_MIGRATION_GUIDE = 'https://maplibre.org/maplibre-gl-js/docs/guides/v5-to-v6-migration-guide/';

function createMockWorker(): Worker {
const worker = new EventTarget() as Worker;
worker.postMessage = vi.fn();
worker.terminate = vi.fn();
return worker;
}

function spyOnWorker(worker = createMockWorker()) {
return vi.spyOn(globalThis, 'Worker').mockImplementation(function() {
return worker;
});
}

describe('workerFactory', () => {
const originalWorker = (globalThis as any).Worker;
const originalWorkerUrl = config.WORKER_URL;

beforeEach(() => {
config.WORKER_URL = '';
});

afterEach(() => {
(globalThis as any).Worker = originalWorker;
config.WORKER_URL = originalWorkerUrl;
vi.restoreAllMocks();
});

test('creates a module worker when WORKER_URL is empty', async () => {
const WorkerSpy = vi.fn();
(globalThis as any).Worker = WorkerSpy;
const WorkerSpy = spyOnWorker();

await workerFactory();
await workerFactory(vi.fn());

expect(WorkerSpy).toHaveBeenCalledTimes(1);
expect(WorkerSpy.mock.calls[0]).toEqual(['', {type: 'module'}]);
});

test('creates a classic worker when WORKER_URL ends with .cjs', async () => {
const WorkerSpy = vi.fn();
(globalThis as any).Worker = WorkerSpy;
const WorkerSpy = spyOnWorker();
config.WORKER_URL = '/path/to/worker.cjs';

await workerFactory();
await workerFactory(vi.fn());

expect(WorkerSpy).toHaveBeenCalledTimes(1);
expect(WorkerSpy.mock.calls[0]).toEqual(['/path/to/worker.cjs']);
});

test('creates a module worker when WORKER_URL ends with .mjs', async () => {
const WorkerSpy = vi.fn();
(globalThis as any).Worker = WorkerSpy;
const WorkerSpy = spyOnWorker();
config.WORKER_URL = '/path/to/worker.mjs';

await workerFactory();
await workerFactory(vi.fn());

expect(WorkerSpy).toHaveBeenCalledTimes(1);
expect(WorkerSpy.mock.calls[0]).toEqual(['/path/to/worker.mjs', {type: 'module'}]);
});

test('falls back to classic worker if module worker construction throws', async () => {
const WorkerSpy = vi.fn()
.mockImplementationOnce(() => { throw new Error('module workers not supported'); });
(globalThis as any).Worker = WorkerSpy;
const worker = createMockWorker();
const WorkerSpy = vi.spyOn(globalThis, 'Worker')
.mockImplementationOnce(() => { throw new Error('module workers not supported'); })
.mockImplementation(function() {
return worker;
});
config.WORKER_URL = '/path/to/worker.mjs';

const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});

await workerFactory();
await workerFactory(vi.fn());

expect(WorkerSpy).toHaveBeenCalledTimes(2);
expect(WorkerSpy.mock.calls[0]).toEqual(['/path/to/worker.mjs', {type: 'module'}]);
Expand All @@ -68,10 +81,7 @@ describe('workerFactory', () => {
});

test('cross-origin module worker URL is converted to an import script and the worker is constructed from a Blob URL', async () => {
const WorkerSpy = vi.fn(function() {
return {postMessage: vi.fn(), addEventListener: vi.fn(), removeEventListener: vi.fn(), terminate: vi.fn()};
});
(globalThis as any).Worker = WorkerSpy;
const WorkerSpy = spyOnWorker();

const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue({
ok: true,
Expand All @@ -83,7 +93,7 @@ describe('workerFactory', () => {

config.WORKER_URL = 'https://unpkg.com/maplibre-gl/dist/maplibre-gl-worker.mjs';

await workerFactory();
await workerFactory(vi.fn());

expect(fetchSpy).toHaveBeenCalledTimes(0);
expect(BlobSpy).toHaveBeenCalledWith(['import "https://unpkg.com/maplibre-gl/dist/maplibre-gl-worker.mjs"'], {type: 'text/javascript'});
Expand All @@ -94,10 +104,7 @@ describe('workerFactory', () => {
});

test('cross-origin classic worker URL is fetched and the worker is constructed from a Blob URL', async () => {
const WorkerSpy = vi.fn(function() {
return {postMessage: vi.fn(), addEventListener: vi.fn(), removeEventListener: vi.fn(), terminate: vi.fn()};
});
(globalThis as any).Worker = WorkerSpy;
const WorkerSpy = spyOnWorker();

const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue({
ok: true,
Expand All @@ -108,7 +115,7 @@ describe('workerFactory', () => {

config.WORKER_URL = 'https://unpkg.com/maplibre-gl/dist/maplibre-gl-worker.cjs';

await workerFactory();
await workerFactory(vi.fn());

expect(fetchSpy).toHaveBeenCalledWith('https://unpkg.com/maplibre-gl/dist/maplibre-gl-worker.cjs');
expect(createObjectURLSpy).toHaveBeenCalled();
Expand All @@ -121,6 +128,70 @@ describe('workerFactory', () => {
vi.spyOn(globalThis, 'fetch').mockResolvedValue({ok: false, status: 404} as any);
config.WORKER_URL = 'https://unpkg.com/maplibre-gl/dist/maplibre-gl-worker.cjs';

await expect(workerFactory()).rejects.toThrow('Failed to fetch worker script (404)');
await expect(workerFactory(vi.fn())).rejects.toThrow('Failed to fetch worker script (404)');
});

test('reports worker script load failures with the resolved URL and migration guidance', async () => {
const worker = createMockWorker();
spyOnWorker(worker);
config.WORKER_URL = '/missing-worker.mjs';
const onError = vi.fn();

await workerFactory(onError);
worker.dispatchEvent(new ErrorEvent('error'));

expect(onError).toHaveBeenCalledTimes(1);
expect(onError.mock.calls[0][0]).toBeInstanceOf(Error);
expect(onError.mock.calls[0][0].message).toBe(
`Failed to load the MapLibre worker script: ${new URL('/missing-worker.mjs', globalThis.location.href).href}\n` +
`If you use a bundler, set the worker URL explicitly; see ${WORKER_MIGRATION_GUIDE}`
);
});

test('reports worker runtime failures without bundler guidance after receiving a message', async () => {
const worker = createMockWorker();
spyOnWorker(worker);
config.WORKER_URL = '/worker.mjs';
const onError = vi.fn();

await workerFactory(onError);
worker.dispatchEvent(new MessageEvent('message'));
worker.dispatchEvent(new ErrorEvent('error', {message: 'Unexpected failure'}));

expect(onError).toHaveBeenCalledTimes(1);
expect(onError.mock.calls[0][0].message).toBe(
`The MapLibre worker script failed while running: ${new URL('/worker.mjs', globalThis.location.href).href} (Unexpected failure)`
);
});

test('reports worker message deserialization failures', async () => {
const worker = createMockWorker();
spyOnWorker(worker);
config.WORKER_URL = '/worker.mjs';
const onError = vi.fn();

await workerFactory(onError);
worker.dispatchEvent(new MessageEvent('messageerror'));

expect(onError).toHaveBeenCalledTimes(1);
expect(onError.mock.calls[0][0].message).toBe(
`Failed to communicate with the MapLibre worker script: ${new URL('/worker.mjs', globalThis.location.href).href}`
);
});

test('preserves a malformed worker URL in the load failure message', async () => {
const worker = createMockWorker();
spyOnWorker(worker);
config.WORKER_URL = 'http://[';
const onError = vi.fn();

await workerFactory(onError);
worker.dispatchEvent(new ErrorEvent('error'));

expect(onError).toHaveBeenCalledTimes(1);
expect(onError.mock.calls[0][0].message).toBe(
'Failed to load the MapLibre worker script: http://[\n' +
`If you use a bundler, set the worker URL explicitly; see ${WORKER_MIGRATION_GUIDE}`
);
});
});
Loading