From 53f0a32a1b646ecc692d8be08121e6f300aa6103 Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Thu, 12 Nov 2026 15:57:02 -0800 Subject: [PATCH] Keep a Data Library entry whose layer fails to tear down Remove dropped an entry from the selection and reported success even when the manager could not disable or destroy its layer, leaving a registered, possibly visible layer that Browse, share links and saved state no longer knew. Remove now keeps the entry and reports failure, so its row and Remove all still reach it and a later Remove can retry. --- src/library/controller.js | 9 ++++++-- src/library/controller.test.mjs | 41 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/library/controller.js b/src/library/controller.js index a2e6ecf9f..22cfb68f9 100644 --- a/src/library/controller.js +++ b/src/library/controller.js @@ -325,8 +325,13 @@ export function createDataLibrary({ async function remove(id) { if (destroyed || pending.has(id)) return false; const work = (async () => { - if (isRegistered(id)) - await dataManager.unregisterLibraryLayer(libraryLayerId(id)); + // A layer that would not tear down is still there: keep it in the + // library so its row, Remove and Remove all still reach it. + if ( + isRegistered(id) && + !(await dataManager.unregisterLibraryLayer(libraryLayerId(id))) + ) + return false; selection = { added: selection.added.filter((other) => other !== id), on: selection.on.filter((other) => other !== id), diff --git a/src/library/controller.test.mjs b/src/library/controller.test.mjs index 4ca490dd6..fa4305a5d 100644 --- a/src/library/controller.test.mjs +++ b/src/library/controller.test.mjs @@ -403,6 +403,47 @@ test('a full library refuses another entry before its layer exists', async (t) = opened.destroy(); }); +test('a layer that fails to tear down stays in the library', async (t) => { + const manager = sealedManager(t); + const storage = memoryStorage(); + const library = createDataLibrary({ + entries, + ledger, + dataManager: manager, + services: {}, + storage, + shareLink: fakeShareLink(null), + source: source({}), + }); + await library.start(); + await library.add('night-lights'); + const { module } = manager.layers.get('lib-night-lights'); + const disable = module.disable; + module.disable = async () => { + throw new Error('stuck'); + }; + t.mock.method(console, 'warn', () => {}); + try { + assert.equal(await library.remove('night-lights'), false); + assert.equal(await library.reset(), 0); + assert.equal(manager.layers.has('lib-night-lights'), true); + assert.deepEqual( + JSON.parse(storage.store.get(LIBRARY_STORAGE_KEY)).added, + ['night-lights'], + ); + assert.equal( + library.list().find((row) => row.entry.id === 'night-lights').added, + true, + ); + } finally { + // Once it tears down, Remove works (and the manager can shut down). + module.disable = disable; + } + assert.equal(await library.remove('night-lights'), true); + assert.equal(manager.layers.has('lib-night-lights'), false); + library.destroy(); +}); + test('zoom flies to an added layer through the caller, reset removes everything', async (t) => { const manager = sealedManager(t); const storage = memoryStorage();