mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-03-29 11:57:20 +02:00
📸 fix: Snapshot Options to Prevent Mid-Await Client Disposal Crash (#12398)
Some checks failed
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Has been cancelled
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Has been cancelled
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Has been cancelled
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Has been cancelled
Some checks failed
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Has been cancelled
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Has been cancelled
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Has been cancelled
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Has been cancelled
* 🐛 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.
This commit is contained in:
parent
abaf9b3e13
commit
f277b32030
2 changed files with 71 additions and 11 deletions
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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(() => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue