From f8e26d4e11b9aa286c231170b0266c715eb1f7db Mon Sep 17 00:00:00 2001 From: JoshuaVSherman Date: Sun, 12 Jul 2026 05:38:43 -0400 Subject: [PATCH 1/2] Fix jamPics/gig deletes silently no-oping (root cause of JaMmusic#1199) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: mongoose 9.x removed findByIdAndRemove entirely (Model and Query), but lib/facade.ts still called it — every deleteById threw synchronously. handleImage() swallowed that (and any other) rejection and returned the message string instead of rethrowing, so removeImage's own catch never fired: no socketError was ever sent, and a bogus imageDeleted got published with the error text as its payload. Delete looked like a no-op from the client. - facade.ts: findByIdAndRemove() now calls the model's findByIdAndDelete (mongoose 9.x-compatible), keeping its own external method name so Controller/GigController/JamPicsController are untouched. - AgController.handleImage(): rethrows on failure instead of swallowing, so newImage/removeImage's existing catch blocks (which already transmit socketError) actually get exercised, and a fake imageCreated/imageDeleted no longer gets published on failure. See WebJamApps/JaMmusic#1199. Bumps 3.0.7 -> 3.0.8. Co-Authored-By: Claude Sonnet 5 --- package-lock.json | 4 ++-- package.json | 2 +- src/AgController/index.ts | 11 ++++++--- src/lib/facade.ts | 7 +++++- test/AgController/index.spec.ts | 40 ++++++++++++++++++++++++++++----- test/lib/facade.test.ts | 11 +++++++-- 6 files changed, 61 insertions(+), 14 deletions(-) diff --git a/package-lock.json b/package-lock.json index 27e4fd7..648cc44 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "webjamsocketserver", - "version": "3.0.7", + "version": "3.0.8", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "webjamsocketserver", - "version": "3.0.7", + "version": "3.0.8", "hasInstallScript": true, "license": "MIT", "dependencies": { diff --git a/package.json b/package.json index 2712246..ead784f 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "webjamsocketserver", "description": "Uses latest version of socketcluster-server", - "version": "3.0.7", + "version": "3.0.8", "license": "MIT", "type": "module", "main": "build/src/index.js", diff --git a/src/AgController/index.ts b/src/AgController/index.ts index 8949242..87ebdf0 100644 --- a/src/AgController/index.ts +++ b/src/AgController/index.ts @@ -172,9 +172,14 @@ class AgController { let r: any; // eslint-disable-next-line security/detect-object-injection try { r = await (this.jamPicsController as any)[func](data); } catch (e) { - const eMessage = (e as Error).message; - debug(eMessage); - return eMessage; + // Rethrow (JaMmusic#1199): swallowing this and returning the error + // message meant callers (newImage/removeImage) never saw the failure, + // so no socketError was ever sent and a bogus imageCreated/imageDeleted + // was still published below with the error string as its payload. Let + // the caller's own try/catch (which already transmits socketError) + // handle it. + debug((e as Error).message); + throw e; } this.server.exchange.transmitPublish(message, r); return message; diff --git a/src/lib/facade.ts b/src/lib/facade.ts index 899fdcc..48c4c4d 100644 --- a/src/lib/facade.ts +++ b/src/lib/facade.ts @@ -48,8 +48,13 @@ class Facade { // return this.Schema.findById(id).lean().exec(); // } // + // Mongoose 9.x removed `findByIdAndRemove` entirely (both on Model and + // Query) in favor of `findByIdAndDelete` (JaMmusic#1199) — calling the old + // name here threw synchronously, which the caller silently swallowed, so + // deletes never took effect. Keep this method's own name (`findByIdAndRemove`) + // unchanged since Controller/GigController/JamPicsController call it. findByIdAndRemove(id: any): any { - return this.Schema.findByIdAndRemove(id).lean().exec(); + return this.Schema.findByIdAndDelete(id).lean().exec(); } } diff --git a/test/AgController/index.spec.ts b/test/AgController/index.spec.ts index cd4704b..f6a7b0d 100644 --- a/test/AgController/index.spec.ts +++ b/test/AgController/index.spec.ts @@ -648,6 +648,37 @@ describe('AgControler', () => { expect(agController.verifyAdminWrite).toHaveBeenCalledWith('token'); expect(agController.handleImage).toHaveBeenCalled(); }); + it('surfaces a genuine deleteById failure as socketError instead of a silent no-op (#1199)', async () => { + const agController = new AgController(aStub); + agController.clients = ['123']; + agController.jamPicsController.deleteById = vi.fn(() => Promise.reject(new Error('Delete id not found'))); + agController.verifyAdminWrite = vi.fn(() => Promise.resolve()); + const transmit = vi.fn(); + const cStub:any = { + socket: { + id: '123', + listener: () => ({ createConsumer: () => ({ next: () => Promise.resolve({ done: true, value: '1000' }) }) }), + transmit, + receiver: () => ({ + createConsumer: () => ({ + next: () => Promise.resolve({ + value: { + token: 'token', + data: 'id', + }, + done: true, + }), + }), + }), + }, + }; + const setIntervalMock:any = vi.fn((cb:any) => cb()); + global.setInterval = setIntervalMock; + agController.removeImage(cStub); + await delay(2000); + expect(transmit).toHaveBeenCalledWith('socketError', { deleteImage: 'Delete id not found' }); + expect(aStub.exchange.transmitPublish).not.toHaveBeenCalledWith('imageDeleted', expect.anything()); + }); it('rejects deleteImage when the token is missing/invalid (#94)', async () => { const agController = new AgController(aStub); agController.handleImage = vi.fn(); @@ -964,14 +995,13 @@ describe('AgControler', () => { }, 'imageCreated'); expect(r).toBe('imageCreated'); }); - it('returns error message when creates a book (image)', async () => { + it('rethrows the error when creating a book (image) fails (#1199)', async () => { const agController = new AgController(aStub); agController.jamPicsController.createDocs = vi.fn(() => Promise.reject(new Error('bad'))); - r = await agController.handleImage('createDocs', { + await expect(agController.handleImage('createDocs', { url: 'url', title: 'title', type: 'JaMmusic', - }, 'imageCreated'); - expect(r).toBe('bad'); - await delay(1000); + }, 'imageCreated')).rejects.toThrow('bad'); + expect(aStub.exchange.transmitPublish).not.toHaveBeenCalledWith('imageCreated', expect.anything()); }); it('updateImage when id not found', async () => { clientStub = { diff --git a/test/lib/facade.test.ts b/test/lib/facade.test.ts index 0177f88..b53ac89 100644 --- a/test/lib/facade.test.ts +++ b/test/lib/facade.test.ts @@ -31,14 +31,21 @@ describe('Facade', () => { const result = await facade.find(); expect(result.test).toBe(true); }); - it('findByIdAndRemove', async () => { + it('findByIdAndRemove calls the model\'s findByIdAndDelete (mongoose 9.x removed findByIdAndRemove, JaMmusic#1199)', async () => { const schema = { - findByIdAndRemove: () => ({ lean: () => ({ exec: () => Promise.resolve(true) }) }), + findByIdAndDelete: () => ({ lean: () => ({ exec: () => Promise.resolve(true) }) }), }; const facade:any = new Facade(schema); const result = await facade.findByIdAndRemove(); expect(result).toBe(true); }); + it('findByIdAndRemove propagates a rejection from findByIdAndDelete', async () => { + const schema = { + findByIdAndDelete: () => ({ lean: () => ({ exec: () => Promise.reject(new Error('bad')) }) }), + }; + const facade:any = new Facade(schema); + await expect(facade.findByIdAndRemove()).rejects.toThrow('bad'); + }); it('findByIdAndUpdate', async () => { const schema = { findByIdAndUpdate: () => ({ lean: () => ({ exec: () => Promise.resolve(true) }) }), From c7c99a3a8affff86f801b67e330c169f7934825b Mon Sep 17 00:00:00 2001 From: JoshuaVSherman Date: Sun, 12 Jul 2026 06:12:25 -0400 Subject: [PATCH 2/2] Bump mongoose to 9.7.4 Update mongoose dependency from ^9.6.1 to ^9.7.4 and refresh lockfile. Co-Authored-By: Claude Haiku 4.5 --- package-lock.json | 10 +++++----- package.json | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index 648cc44..57fe906 100644 --- a/package-lock.json +++ b/package-lock.json @@ -25,7 +25,7 @@ "express": "^5.2.1", "express-sslify": "^1.2.0", "jsonwebtoken": "^9.0.1", - "mongoose": "^9.6.1", + "mongoose": "^9.7.4", "morgan": "^1.10.0", "rimraf": "^6.1.3", "sc-errors": "^3.0.0", @@ -782,7 +782,6 @@ "version": "1.1.0", "resolved": "https://registry.npmjs.org/@standard-schema/spec/-/spec-1.1.0.tgz", "integrity": "sha512-l2aFy5jALhniG5HgqrD6jXLi/rUWrKvqN/qJx6yoJsgKhblVd+iqqU4RCXavm/jPityDo5TCvKMnpjKnOriy0w==", - "dev": true, "license": "MIT" }, "node_modules/@tybys/wasm-util": { @@ -4056,11 +4055,12 @@ } }, "node_modules/mongoose": { - "version": "9.7.2", - "resolved": "https://registry.npmjs.org/mongoose/-/mongoose-9.7.2.tgz", - "integrity": "sha512-VxxS3tjOGRIvhBMd84RR7L1WbDdm08Qq6V88VMq6jP6aE1Q+NlbTZDBtye2NAyO48i5kBFxM2oxfX0CPKNwCMg==", + "version": "9.7.4", + "resolved": "https://registry.npmjs.org/mongoose/-/mongoose-9.7.4.tgz", + "integrity": "sha512-nuSYGUWWzNd4EAbGYxE469wPTL+kmxb5+91YvCvMkJ08rvNRht/usZUU3LuFuk7rDutF2QWBZHPHuzM8TxXApA==", "license": "MIT", "dependencies": { + "@standard-schema/spec": "^1.1.0", "kareem": "3.3.0", "mongodb": "~7.2", "mpath": "0.9.0", diff --git a/package.json b/package.json index ead784f..fcfbff3 100644 --- a/package.json +++ b/package.json @@ -64,7 +64,7 @@ "express": "^5.2.1", "express-sslify": "^1.2.0", "jsonwebtoken": "^9.0.1", - "mongoose": "^9.6.1", + "mongoose": "^9.7.4", "morgan": "^1.10.0", "rimraf": "^6.1.3", "sc-errors": "^3.0.0",