From 9cb82b3198bc12430c90f6d3eb75e223b91ab311 Mon Sep 17 00:00:00 2001 From: JoshuaVSherman Date: Sat, 25 Jul 2026 05:54:39 -0400 Subject: [PATCH 1/2] Rethrow updateGig errors instead of swallowing them (#253) updateGig caught its own errors and returned the message as a string, so it resolved normally and editDoc's catch (which transmits socketError) never saw the failure. A failed gig update emitted neither socketError nor gigUpdated. Rethrow so editDoc's existing catch is the single place the error is handled and transmitted, matching the pattern already used by handleGig/handleImage. Co-Authored-By: Claude Sonnet 5 --- package.json | 2 +- src/AgController/index.ts | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index d05c2c2..6db0440 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "webjamsocketserver", "description": "Uses latest version of socketcluster-server", - "version": "3.0.10", + "version": "3.0.11", "license": "MIT", "type": "module", "main": "build/src/index.js", diff --git a/src/AgController/index.ts b/src/AgController/index.ts index 690d893..05a1bea 100644 --- a/src/AgController/index.ts +++ b/src/AgController/index.ts @@ -308,9 +308,13 @@ class AgController { if (!gig.venue || !gig.datetime || !gig.city || !gig.usState) throw new Error('Invalid gig data'); r = await this.gigController.findByIdAndUpdate(id, gig); } catch (e) { - const eMessage = (e as Error).message; - debug(eMessage); - return eMessage; + // Rethrow (#253, same pattern as handleImage/JaMmusic#1199): swallowing + // this and returning the error message meant editDoc's catch (which + // already transmits socketError) never saw the failure, so neither + // socketError nor gigUpdated was ever sent. Let editDoc's own try/catch + // handle it. + debug((e as Error).message); + throw e; } this.server.exchange.transmitPublish('gigUpdated', r); return 'Gig updated'; From 7b3bfc05229a13528ac19b41330646bc89cf1192 Mon Sep 17 00:00:00 2001 From: JoshuaVSherman Date: Sat, 25 Jul 2026 05:56:04 -0400 Subject: [PATCH 2/2] Add unit tests for updateGig rethrow behavior (#253) Covers validation-failure, database-failure, and success paths for updateGig directly, plus editDoc-level integration tests asserting socketError transmission (or none, on success) and that editTour behaves identically to editGig. Co-Authored-By: Claude Sonnet 5 --- test/AgController/index.spec.ts | 140 ++++++++++++++++++++++++++++++-- 1 file changed, 134 insertions(+), 6 deletions(-) diff --git a/test/AgController/index.spec.ts b/test/AgController/index.spec.ts index 33c4510..7da1385 100644 --- a/test/AgController/index.spec.ts +++ b/test/AgController/index.spec.ts @@ -263,16 +263,22 @@ describe('AgControler', () => { }); expect(r).toBe('Gig updated'); }); - it('handles error from updates a tours', async () => { + it('rethrows a database failure from updateGig instead of swallowing it (#253)', async () => { const agController = new AgController(aStub); agController.gigController.findByIdAndUpdate = vi.fn(() => Promise.reject(new Error('bad'))); - r = await agController.updateGig({ + await expect(agController.updateGig({ tourId: testId, tour: { - venue: 'venue', datetime: new Date(), city: 'city', usState: 'state', - }, - }); - expect(r).toBe('bad'); + venue: 'venue', datetime: new Date(), city: 'city', usState: 'state', + }, + })).rejects.toThrow('bad'); + }); + it('rethrows a validation failure from updateGig instead of swallowing it (#253)', async () => { + const agController = new AgController(aStub); + await expect(agController.updateGig({ + gigId: testId, + gig: {}, + })).rejects.toThrow('Invalid gig data'); }); it('does not process the newTour message from client when token is not valid', async () => { const agController = new AgController(aStub); @@ -1012,6 +1018,128 @@ describe('AgControler', () => { await delay(1000); expect(agController.server.exchange.transmitPublish).not.toHaveBeenCalled(); }); + it('transmits socketError with Invalid gig data when editGig fails validation (#253)', async () => { + const agController = new AgController(aStub); + agController.clients = ['123']; + agController.verifyAdminWrite = vi.fn(() => Promise.resolve()); + const sStub:any = { + socket: { + id: '123', + transmit: vi.fn(), + receiver: () => ({ + createConsumer: () => ({ + next: () => Promise.resolve({ + value: { + gigId: '123', + token: 'token', + gig: {}, + }, + done: true, + }), + }), + }), + }, + }; + agController.server.exchange.transmitPublish = vi.fn(); + agController.editDoc(sStub, 'editGig'); + await delay(1000); + expect(sStub.socket.transmit).toHaveBeenCalledWith('socketError', { editGig: 'Invalid gig data' }); + expect(agController.server.exchange.transmitPublish).not.toHaveBeenCalled(); + }); + it('transmits socketError with the db message when editGig fails at the database layer (#253)', async () => { + const agController = new AgController(aStub); + agController.clients = ['123']; + agController.verifyAdminWrite = vi.fn(() => Promise.resolve()); + agController.gigController.findByIdAndUpdate = vi.fn(() => Promise.reject(new Error('db exploded'))); + const sStub:any = { + socket: { + id: '123', + transmit: vi.fn(), + receiver: () => ({ + createConsumer: () => ({ + next: () => Promise.resolve({ + value: { + gigId: '123', + token: 'token', + gig: { + venue: 'venue', datetime: new Date(), city: 'city', usState: 'state', + }, + }, + done: true, + }), + }), + }), + }, + }; + agController.server.exchange.transmitPublish = vi.fn(); + agController.editDoc(sStub, 'editGig'); + await delay(1000); + expect(sStub.socket.transmit).toHaveBeenCalledWith('socketError', { editGig: 'db exploded' }); + expect(agController.server.exchange.transmitPublish).not.toHaveBeenCalled(); + }); + it('publishes gigUpdated exactly once and transmits no socketError on a successful editGig (#253)', async () => { + const agController = new AgController(aStub); + agController.clients = ['123']; + agController.verifyAdminWrite = vi.fn(() => Promise.resolve()); + agController.gigController.findByIdAndUpdate = vi.fn(() => Promise.resolve({ _id: '123' })); + const sStub:any = { + socket: { + id: '123', + transmit: vi.fn(), + receiver: () => ({ + createConsumer: () => ({ + next: () => Promise.resolve({ + value: { + gigId: '123', + token: 'token', + gig: { + venue: 'venue', datetime: new Date(), city: 'city', usState: 'state', + }, + }, + done: true, + }), + }), + }), + }, + }; + agController.server.exchange.transmitPublish = vi.fn(); + agController.editDoc(sStub, 'editGig'); + await delay(1000); + expect(agController.server.exchange.transmitPublish).toHaveBeenCalledTimes(1); + expect(agController.server.exchange.transmitPublish).toHaveBeenCalledWith('gigUpdated', { _id: '123' }); + expect(sStub.socket.transmit).not.toHaveBeenCalledWith('socketError', expect.anything()); + }); + it('behaves identically for the legacy editTour alias on a database failure (#253)', async () => { + const agController = new AgController(aStub); + agController.clients = ['123']; + agController.verifyAdminWrite = vi.fn(() => Promise.resolve()); + agController.gigController.findByIdAndUpdate = vi.fn(() => Promise.reject(new Error('bad'))); + const sStub:any = { + socket: { + id: '123', + transmit: vi.fn(), + receiver: () => ({ + createConsumer: () => ({ + next: () => Promise.resolve({ + value: { + tourId: '123', + token: 'token', + tour: { + venue: 'venue', datetime: new Date(), city: 'city', usState: 'state', + }, + }, + done: true, + }), + }), + }), + }, + }; + agController.server.exchange.transmitPublish = vi.fn(); + agController.editDoc(sStub, 'editTour'); + await delay(1000); + expect(sStub.socket.transmit).toHaveBeenCalledWith('socketError', { editTour: 'bad' }); + expect(agController.server.exchange.transmitPublish).not.toHaveBeenCalled(); + }); it('handles missing token when the deleteTour message from client', async () => { const agController = new AgController(aStub); agController.clients = ['123'];