From f277b3203040020579b687f9656b45d35813d8f4 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 25 Mar 2026 14:18:32 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=B8=20fix:=20Snapshot=20Options=20to?= =?UTF-8?q?=20Prevent=20Mid-Await=20Client=20Disposal=20Crash=20(#12398)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🐛 fix: Prevent crash when token balance exhaustion races with client disposal Add null guard in saveMessageToDatabase to handle the case where disposeClient nullifies this.options while a userMessagePromise is still pending from a prior async save operation. * 🐛 fix: Snapshot this.options to prevent mid-await disposal crash The original guard at function entry was insufficient — this.options is always valid at entry. The crash occurs after the first await (db.saveMessage) when disposeClient nullifies client.options while the promise is suspended. Fix: capture this.options into a local const before any await. The local reference is immune to client.options = null set by disposeClient. Also add .catch on userMessagePromise in sendMessage to prevent unhandled rejections when checkBalance throws before the promise is awaited, and add two regression tests. --- api/app/clients/BaseClient.js | 34 ++++++++++++----- api/app/clients/specs/BaseClient.test.js | 48 +++++++++++++++++++++++- 2 files changed, 71 insertions(+), 11 deletions(-) diff --git a/api/app/clients/BaseClient.js b/api/app/clients/BaseClient.js index ae2d362773..ec5ccfb5f4 100644 --- a/api/app/clients/BaseClient.js +++ b/api/app/clients/BaseClient.js @@ -487,7 +487,12 @@ class BaseClient { } delete userMessage.image_urls; } - userMessagePromise = this.saveMessageToDatabase(userMessage, saveOptions, user); + userMessagePromise = this.saveMessageToDatabase(userMessage, saveOptions, user).catch( + (err) => { + logger.error('[BaseClient] Failed to save user message:', err); + return {}; + }, + ); this.savedMessageIds.add(userMessage.messageId); if (typeof opts?.getReqData === 'function') { opts.getReqData({ @@ -727,21 +732,30 @@ class BaseClient { * @param {string | null} user */ async saveMessageToDatabase(message, endpointOptions, user = null) { + // Snapshot options before any await; disposeClient may set client.options = null + // while this method is suspended at an I/O boundary, but the local reference + // remains valid (disposeClient nulls the property, not the object itself). + const options = this.options; + if (!options) { + logger.error('[BaseClient] saveMessageToDatabase: client disposed before save, skipping'); + return {}; + } + if (this.user && user !== this.user) { throw new Error('User mismatch.'); } - const hasAddedConvo = this.options?.req?.body?.addedConvo != null; + const hasAddedConvo = options?.req?.body?.addedConvo != null; const reqCtx = { - userId: this.options?.req?.user?.id, - isTemporary: this.options?.req?.body?.isTemporary, - interfaceConfig: this.options?.req?.config?.interfaceConfig, + userId: options?.req?.user?.id, + isTemporary: options?.req?.body?.isTemporary, + interfaceConfig: options?.req?.config?.interfaceConfig, }; const savedMessage = await db.saveMessage( reqCtx, { ...message, - endpoint: this.options.endpoint, + endpoint: options.endpoint, unfinished: false, user, ...(hasAddedConvo && { addedConvo: true }), @@ -755,20 +769,20 @@ class BaseClient { const fieldsToKeep = { conversationId: message.conversationId, - endpoint: this.options.endpoint, - endpointType: this.options.endpointType, + endpoint: options.endpoint, + endpointType: options.endpointType, ...endpointOptions, }; const existingConvo = this.fetchedConvo === true ? null - : await db.getConvo(this.options?.req?.user?.id, message.conversationId); + : await db.getConvo(options?.req?.user?.id, message.conversationId); const unsetFields = {}; const exceptions = new Set(['spec', 'iconURL']); const hasNonEphemeralAgent = - isAgentsEndpoint(this.options.endpoint) && + isAgentsEndpoint(options.endpoint) && endpointOptions?.agent_id && !isEphemeralAgentId(endpointOptions.agent_id); if (hasNonEphemeralAgent) { diff --git a/api/app/clients/specs/BaseClient.test.js b/api/app/clients/specs/BaseClient.test.js index edbbcaa87b..3ce910948c 100644 --- a/api/app/clients/specs/BaseClient.test.js +++ b/api/app/clients/specs/BaseClient.test.js @@ -38,7 +38,7 @@ jest.mock('~/models', () => ({ updateFileUsage: jest.fn(), })); -const { getConvo, saveConvo } = require('~/models'); +const { getConvo, saveConvo, saveMessage } = require('~/models'); jest.mock('@librechat/agents', () => { const actual = jest.requireActual('@librechat/agents'); @@ -906,6 +906,52 @@ describe('BaseClient', () => { ); }); + test('saveMessageToDatabase returns early when this.options is null (client disposed)', async () => { + const savedOptions = TestClient.options; + TestClient.options = null; + saveMessage.mockClear(); + + const result = await TestClient.saveMessageToDatabase( + { messageId: 'msg-1', conversationId: 'conv-1', isCreatedByUser: true, text: 'hi' }, + {}, + null, + ); + + expect(result).toEqual({}); + expect(saveMessage).not.toHaveBeenCalled(); + + TestClient.options = savedOptions; + }); + + test('saveMessageToDatabase uses snapshot of options, immune to mid-await disposal', async () => { + const savedOptions = TestClient.options; + saveMessage.mockClear(); + saveConvo.mockClear(); + + // Make db.saveMessage yield, simulating I/O suspension during which disposal occurs + saveMessage.mockImplementation(async (_reqCtx, msgData) => { + // Simulate disposeClient nullifying client.options while awaiting + TestClient.options = null; + return msgData; + }); + saveConvo.mockResolvedValue({ conversationId: 'conv-1' }); + + const result = await TestClient.saveMessageToDatabase( + { messageId: 'msg-1', conversationId: 'conv-1', isCreatedByUser: true, text: 'hi' }, + { endpoint: 'openAI' }, + null, + ); + + // Should complete without TypeError, using the snapshotted options + expect(result).toHaveProperty('message'); + expect(result).toHaveProperty('conversation'); + expect(saveMessage).toHaveBeenCalled(); + + TestClient.options = savedOptions; + saveMessage.mockReset(); + saveConvo.mockReset(); + }); + test('userMessagePromise is awaited before saving response message', async () => { // Mock the saveMessageToDatabase method TestClient.saveMessageToDatabase = jest.fn().mockImplementation(() => {