From 92a96fd1a644a3d667cde01d04a6bcb920f1d9ab Mon Sep 17 00:00:00 2001 From: Sandesh Pandey Date: Tue, 18 Aug 2026 09:39:27 -0500 Subject: [PATCH 1/2] Isolate event listeners so one handler cannot abort an emit --- src/essence/mmgisAPI/mmgisAPI.js | 16 +++++++++++-- tests/unit/mmgisAPIBus.spec.js | 39 ++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 tests/unit/mmgisAPIBus.spec.js diff --git a/src/essence/mmgisAPI/mmgisAPI.js b/src/essence/mmgisAPI/mmgisAPI.js index 9a5a84042..e7e2ba3a7 100644 --- a/src/essence/mmgisAPI/mmgisAPI.js +++ b/src/essence/mmgisAPI/mmgisAPI.js @@ -1072,13 +1072,25 @@ var mmgisAPI = { off: events.off, /** - * Emit an event to all subscribers + * Emit an event to all subscribers. A handler that throws is reported and + * skipped, so one bad listener cannot abort the emit. * @param {string} event - Event name * @param {*} data - Event data to pass to subscribers * @example * mmgisAPI.emit('layer:toggle', { layerName: 'Terrain', visible: true }); */ - emit: events.emit, + emit: (event, data) => { + const run = (fn, args, label) => { + try { + fn(...args) + } catch (err) { + console.error(`[mmgisAPI] ${label} for "${event}" threw:`, err) + } + } + // Copy before iterating: a handler may subscribe or unsubscribe mid-emit. + ;[...(events.all.get(event) || [])].forEach((fn) => run(fn, [data], 'listener')) + ;[...(events.all.get('*') || [])].forEach((fn) => run(fn, [event, data], 'wildcard listener')) + }, // ============ REQUEST/RESPONSE API ============ diff --git a/tests/unit/mmgisAPIBus.spec.js b/tests/unit/mmgisAPIBus.spec.js new file mode 100644 index 000000000..da141110f --- /dev/null +++ b/tests/unit/mmgisAPIBus.spec.js @@ -0,0 +1,39 @@ +import { test, expect, vi, afterEach } from 'vitest' + +// Viewer_ pulls in Photosphere/ModelViewer/PDFViewer, which are JSX written in +// .js files that vite's import-analysis can't parse. Nothing here needs the +// real viewers, so stub the aggregator to keep the import chain parseable. +vi.mock('../../src/essence/Basics/Viewer_/Viewer_', () => ({ default: {} })) + +import { mmgisAPI } from '../../src/essence/mmgisAPI/mmgisAPI' + +afterEach(() => vi.restoreAllMocks()) + +test('a throwing listener does not reach the emitter', () => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + const off = mmgisAPI.on('test:isolation', () => { throw new Error('boom') }) + expect(() => mmgisAPI.emit('test:isolation', { a: 1 })).not.toThrow() + expect(errorSpy).toHaveBeenCalledWith( + expect.stringContaining('test:isolation'), + expect.any(Error) + ) + off() +}) + +test('a throwing listener does not stop later listeners', () => { + vi.spyOn(console, 'error').mockImplementation(() => {}) + const seen = [] + const off1 = mmgisAPI.on('test:order', () => { throw new Error('boom') }) + const off2 = mmgisAPI.on('test:order', () => { seen.push('second') }) + mmgisAPI.emit('test:order', {}) + expect(seen).toEqual(['second']) + off1(); off2() +}) + +test('wildcard listeners still receive event name and payload', () => { + const seen = [] + const off = mmgisAPI.on('*', (type, data) => { seen.push([type, data]) }) + mmgisAPI.emit('test:wildcard', { v: 7 }) + expect(seen).toEqual([['test:wildcard', { v: 7 }]]) + off() +}) From c51bb8fbceb8e903aac8ae7cb2a996649b4467f5 Mon Sep 17 00:00:00 2001 From: Sandesh Pandey Date: Thu, 20 Aug 2026 14:55:22 -0500 Subject: [PATCH 2/2] Cover the emit rules the isolation rewrite depends on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dispatching from events.all rather than through mitt's own emit is what lets a throwing listener be caught and skipped, and it puts three of mitt's rules in this file's hands: the subscriber list is copied before iteration, specific listeners run before wildcards, and wildcards are called with (type, data). Only the wildcard signature was tested. Removing the copy, reordering the two passes, or handing every listener the wrong arguments all left the suite green, and so did dropping the try/catch around the wildcard pass. Tests for each of those, plus the plainest promise of the lot — that an ordinary listener receives the payload it was emitted with. Cleanups move to afterEach. Run after the assertions, an unsubscribe is skipped by the first failure, and the next test in the file inherits a live listener on a bus that outlives it. --- src/essence/mmgisAPI/mmgisAPI.js | 6 +++ tests/unit/mmgisAPIBus.spec.js | 90 +++++++++++++++++++++++++++++--- 2 files changed, 88 insertions(+), 8 deletions(-) diff --git a/src/essence/mmgisAPI/mmgisAPI.js b/src/essence/mmgisAPI/mmgisAPI.js index e7e2ba3a7..636bbef76 100644 --- a/src/essence/mmgisAPI/mmgisAPI.js +++ b/src/essence/mmgisAPI/mmgisAPI.js @@ -1087,6 +1087,12 @@ var mmgisAPI = { console.error(`[mmgisAPI] ${label} for "${event}" threw:`, err) } } + // Dispatching from events.all — mitt's published subscriber map — + // rather than through events.emit, so each listener can be isolated. + // It restates two of mitt's rules: specific listeners before + // wildcards, and wildcards called with (type, data). A mitt upgrade is + // a place to re-check both. + // // Copy before iterating: a handler may subscribe or unsubscribe mid-emit. ;[...(events.all.get(event) || [])].forEach((fn) => run(fn, [data], 'listener')) ;[...(events.all.get('*') || [])].forEach((fn) => run(fn, [event, data], 'wildcard listener')) diff --git a/tests/unit/mmgisAPIBus.spec.js b/tests/unit/mmgisAPIBus.spec.js index da141110f..5528638ba 100644 --- a/tests/unit/mmgisAPIBus.spec.js +++ b/tests/unit/mmgisAPIBus.spec.js @@ -7,33 +7,107 @@ vi.mock('../../src/essence/Basics/Viewer_/Viewer_', () => ({ default: {} })) import { mmgisAPI } from '../../src/essence/mmgisAPI/mmgisAPI' -afterEach(() => vi.restoreAllMocks()) +// The bus is a module-level singleton, so a subscription outliving its test is +// heard by the next one. Collected here so a test that fails partway through +// still unsubscribes. +const cleanups = [] +const listen = (event, fn) => { + cleanups.push(mmgisAPI.on(event, fn)) +} + +afterEach(() => { + cleanups.splice(0).forEach((off) => off()) + vi.restoreAllMocks() +}) + +test('a listener receives the emitted payload', () => { + const seen = [] + listen('test:payload', (data) => seen.push(data)) + mmgisAPI.emit('test:payload', { a: 1 }) + expect(seen).toEqual([{ a: 1 }]) +}) test('a throwing listener does not reach the emitter', () => { const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) - const off = mmgisAPI.on('test:isolation', () => { throw new Error('boom') }) + listen('test:isolation', () => { throw new Error('boom') }) expect(() => mmgisAPI.emit('test:isolation', { a: 1 })).not.toThrow() expect(errorSpy).toHaveBeenCalledWith( expect.stringContaining('test:isolation'), expect.any(Error) ) - off() }) test('a throwing listener does not stop later listeners', () => { vi.spyOn(console, 'error').mockImplementation(() => {}) const seen = [] - const off1 = mmgisAPI.on('test:order', () => { throw new Error('boom') }) - const off2 = mmgisAPI.on('test:order', () => { seen.push('second') }) + listen('test:order', () => { throw new Error('boom') }) + listen('test:order', () => { seen.push('second') }) mmgisAPI.emit('test:order', {}) expect(seen).toEqual(['second']) - off1(); off2() }) test('wildcard listeners still receive event name and payload', () => { const seen = [] - const off = mmgisAPI.on('*', (type, data) => { seen.push([type, data]) }) + listen('*', (type, data) => { seen.push([type, data]) }) mmgisAPI.emit('test:wildcard', { v: 7 }) expect(seen).toEqual([['test:wildcard', { v: 7 }]]) - off() +}) + +test('a throwing wildcard listener is isolated too', () => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + const seen = [] + listen('*', () => { throw new Error('boom') }) + listen('test:wildcard-isolation', () => seen.push('specific')) + + expect(() => mmgisAPI.emit('test:wildcard-isolation', {})).not.toThrow() + + expect(seen).toEqual(['specific']) + expect(errorSpy).toHaveBeenCalledWith( + expect.stringContaining('wildcard listener'), + expect.any(Error) + ) +}) + +test('a listener that unsubscribes mid-emit does not disturb the rest', () => { + // Every listener registered when the emit began still receives it. + const seen = [] + let offSecond + listen('test:mutation', () => seen.push('first')) + listen('test:mutation', () => { + seen.push('second') + offSecond() + }) + offSecond = cleanups[cleanups.length - 1] + listen('test:mutation', () => seen.push('third')) + + mmgisAPI.emit('test:mutation', {}) + expect(seen).toEqual(['first', 'second', 'third']) + + seen.length = 0 + mmgisAPI.emit('test:mutation', {}) + expect(seen).toEqual(['first', 'third']) +}) + +test('a listener that subscribes mid-emit is not called by that same emit', () => { + const seen = [] + listen('test:late-subscribe', () => { + seen.push('first') + listen('test:late-subscribe', () => seen.push('added-during-emit')) + }) + + mmgisAPI.emit('test:late-subscribe', {}) + expect(seen).toEqual(['first']) + + seen.length = 0 + mmgisAPI.emit('test:late-subscribe', {}) + expect(seen).toEqual(['first', 'added-during-emit']) +}) + +test('specific listeners run before wildcards', () => { + const order = [] + listen('*', () => order.push('wildcard')) + listen('test:precedence', () => order.push('specific')) + + mmgisAPI.emit('test:precedence', {}) + expect(order).toEqual(['specific', 'wildcard']) })