fix: AbortSignal Cleanup Logic for New Chats (#9177)

* chore: import paths for isEnabled and logger in title.js

*  fix: `AbortSignal` Cleanup Logic for New Chats

* test: Add `isNewConvo` parameter to onStart expectation in BaseClient tests
This commit is contained in:
Danny Avila 2025-08-20 14:56:07 -04:00 committed by GitHub
parent 9a79635012
commit e0ebb7097e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 42 additions and 40 deletions

View file

@ -187,7 +187,8 @@ class BaseClient {
this.user = user; this.user = user;
const saveOptions = this.getSaveOptions(); const saveOptions = this.getSaveOptions();
this.abortController = opts.abortController ?? new AbortController(); this.abortController = opts.abortController ?? new AbortController();
const conversationId = overrideConvoId ?? opts.conversationId ?? crypto.randomUUID(); const requestConvoId = overrideConvoId ?? opts.conversationId;
const conversationId = requestConvoId ?? crypto.randomUUID();
const parentMessageId = opts.parentMessageId ?? Constants.NO_PARENT; const parentMessageId = opts.parentMessageId ?? Constants.NO_PARENT;
const userMessageId = const userMessageId =
overrideUserMessageId ?? opts.overrideParentMessageId ?? crypto.randomUUID(); overrideUserMessageId ?? opts.overrideParentMessageId ?? crypto.randomUUID();
@ -212,11 +213,12 @@ class BaseClient {
...opts, ...opts,
user, user,
head, head,
saveOptions,
userMessageId,
requestConvoId,
conversationId, conversationId,
parentMessageId, parentMessageId,
userMessageId,
responseMessageId, responseMessageId,
saveOptions,
}; };
} }
@ -235,11 +237,12 @@ class BaseClient {
const { const {
user, user,
head, head,
saveOptions,
userMessageId,
requestConvoId,
conversationId, conversationId,
parentMessageId, parentMessageId,
userMessageId,
responseMessageId, responseMessageId,
saveOptions,
} = await this.setMessageOptions(opts); } = await this.setMessageOptions(opts);
const userMessage = opts.isEdited const userMessage = opts.isEdited
@ -261,7 +264,8 @@ class BaseClient {
} }
if (typeof opts?.onStart === 'function') { if (typeof opts?.onStart === 'function') {
opts.onStart(userMessage, responseMessageId); const isNewConvo = !requestConvoId && parentMessageId === Constants.NO_PARENT;
opts.onStart(userMessage, responseMessageId, isNewConvo);
} }
return { return {

View file

@ -579,6 +579,8 @@ describe('BaseClient', () => {
expect(onStart).toHaveBeenCalledWith( expect(onStart).toHaveBeenCalledWith(
expect.objectContaining({ text: 'Hello, world!' }), expect.objectContaining({ text: 'Hello, world!' }),
expect.any(String), expect.any(String),
/** `isNewConvo` */
true,
); );
}); });

View file

@ -1,6 +1,6 @@
const { logger } = require('@librechat/data-schemas'); const { logger } = require('@librechat/data-schemas');
const { countTokens, isEnabled, sendEvent } = require('@librechat/api'); const { countTokens, isEnabled, sendEvent } = require('@librechat/api');
const { isAssistantsEndpoint, ErrorTypes } = require('librechat-data-provider'); const { isAssistantsEndpoint, ErrorTypes, Constants } = require('librechat-data-provider');
const { truncateText, smartTruncateText } = require('~/app/clients/prompts'); const { truncateText, smartTruncateText } = require('~/app/clients/prompts');
const clearPendingReq = require('~/cache/clearPendingReq'); const clearPendingReq = require('~/cache/clearPendingReq');
const { sendError } = require('~/server/middleware/error'); const { sendError } = require('~/server/middleware/error');
@ -11,6 +11,10 @@ const { abortRun } = require('./abortRun');
const abortDataMap = new WeakMap(); const abortDataMap = new WeakMap();
/**
* @param {string} abortKey
* @returns {boolean}
*/
function cleanupAbortController(abortKey) { function cleanupAbortController(abortKey) {
if (!abortControllers.has(abortKey)) { if (!abortControllers.has(abortKey)) {
return false; return false;
@ -71,6 +75,20 @@ function cleanupAbortController(abortKey) {
return true; return true;
} }
/**
* @param {string} abortKey
* @returns {function(): void}
*/
function createCleanUpHandler(abortKey) {
return function () {
try {
cleanupAbortController(abortKey);
} catch {
// Ignore cleanup errors
}
};
}
async function abortMessage(req, res) { async function abortMessage(req, res) {
let { abortKey, endpoint } = req.body; let { abortKey, endpoint } = req.body;
@ -172,11 +190,15 @@ const createAbortController = (req, res, getAbortData, getReqData) => {
/** /**
* @param {TMessage} userMessage * @param {TMessage} userMessage
* @param {string} responseMessageId * @param {string} responseMessageId
* @param {boolean} [isNewConvo]
*/ */
const onStart = (userMessage, responseMessageId) => { const onStart = (userMessage, responseMessageId, isNewConvo) => {
sendEvent(res, { message: userMessage, created: true }); sendEvent(res, { message: userMessage, created: true });
const abortKey = userMessage?.conversationId ?? req.user.id; const prelimAbortKey = userMessage?.conversationId ?? req.user.id;
const abortKey = isNewConvo
? `${prelimAbortKey}${Constants.COMMON_DIVIDER}${Constants.NEW_CONVO}`
: prelimAbortKey;
getReqData({ abortKey }); getReqData({ abortKey });
const prevRequest = abortControllers.get(abortKey); const prevRequest = abortControllers.get(abortKey);
const { overrideUserMessageId } = req?.body ?? {}; const { overrideUserMessageId } = req?.body ?? {};
@ -194,16 +216,7 @@ const createAbortController = (req, res, getAbortData, getReqData) => {
}; };
abortControllers.set(addedAbortKey, { abortController, ...minimalOptions }); abortControllers.set(addedAbortKey, { abortController, ...minimalOptions });
const cleanupHandler = createCleanUpHandler(addedAbortKey);
// Use a simple function for cleanup to avoid capturing context
const cleanupHandler = () => {
try {
cleanupAbortController(addedAbortKey);
} catch (e) {
// Ignore cleanup errors
}
};
res.on('finish', cleanupHandler); res.on('finish', cleanupHandler);
return; return;
} }
@ -216,16 +229,7 @@ const createAbortController = (req, res, getAbortData, getReqData) => {
}; };
abortControllers.set(abortKey, { abortController, ...minimalOptions }); abortControllers.set(abortKey, { abortController, ...minimalOptions });
const cleanupHandler = createCleanUpHandler(abortKey);
// Use a simple function for cleanup to avoid capturing context
const cleanupHandler = () => {
try {
cleanupAbortController(abortKey);
} catch (e) {
// Ignore cleanup errors
}
};
res.on('finish', cleanupHandler); res.on('finish', cleanupHandler);
}; };
@ -364,15 +368,7 @@ const handleAbortError = async (res, req, error, data) => {
}; };
} }
// Create a simple callback without capturing parent scope const callback = createCleanUpHandler(conversationId);
const callback = async () => {
try {
cleanupAbortController(conversationId);
} catch (e) {
// Ignore cleanup errors
}
};
await sendError(req, res, options, callback); await sendError(req, res, options, callback);
}; };

View file

@ -1,8 +1,8 @@
const { isEnabled } = require('@librechat/api');
const { logger } = require('@librechat/data-schemas');
const { CacheKeys } = require('librechat-data-provider'); const { CacheKeys } = require('librechat-data-provider');
const getLogStores = require('~/cache/getLogStores'); const getLogStores = require('~/cache/getLogStores');
const { isEnabled } = require('~/server/utils');
const { saveConvo } = require('~/models'); const { saveConvo } = require('~/models');
const { logger } = require('~/config');
/** /**
* Add title to conversation in a way that avoids memory retention * Add title to conversation in a way that avoids memory retention