mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-03-02 14:20:18 +01:00
🚦 fix: 404 JSON Responses for Unmatched API Routes (#11976)
* feat: Implement 404 JSON response for unmatched API routes - Added middleware to return a 404 JSON response with a message for undefined API routes. - Updated SPA fallback to serve index.html for non-API unmatched routes. - Ensured the error handler is positioned correctly as the last middleware in the stack. * fix: Enhance logging in BaseClient for better token usage tracking - Updated `getTokenCountForResponse` to log the messageId of the response for improved debugging. - Enhanced userMessage logging to include messageId, tokenCount, and conversationId for clearer context during token count mapping. * chore: Improve logging in processAddedConvo for better debugging - Updated the logging structure in the processAddedConvo function to provide clearer context when processing added conversations. - Removed redundant logging and enhanced the output to include model, agent ID, and endpoint details for improved traceability. * chore: Enhance logging in BaseClient for improved token usage tracking - Added debug logging in the BaseClient to track response token usage, including messageId, model, promptTokens, and completionTokens for better debugging and traceability. * chore: Enhance logging in MemoryAgent for improved context - Updated logging in the MemoryAgent to include userId, conversationId, messageId, and provider details for better traceability during memory processing. - Adjusted log messages to provide clearer context when content is returned or not, aiding in debugging efforts. * chore: Refactor logging in initializeClient for improved clarity - Consolidated multiple debug log statements into a single message that provides a comprehensive overview of the tool context being stored for the primary agent, including the number of tools and the size of the tool registry. This enhances traceability and debugging efficiency. * feat: Implement centralized 404 handling for unmatched API routes - Introduced a new middleware function `apiNotFound` to standardize 404 JSON responses for undefined API routes. - Updated the server configuration to utilize the new middleware, enhancing code clarity and maintainability. - Added tests to ensure correct 404 responses for various non-GET methods and the `/api` root path. * fix: Enhance logging in apiNotFound middleware for improved safety - Updated the `apiNotFound` function to sanitize the request path by replacing problematic characters and limiting its length, ensuring safer logging of 404 errors. * refactor: Move apiNotFound middleware to a separate file for better organization - Extracted the `apiNotFound` function from the error middleware into its own file, enhancing code organization and maintainability. - Updated the index file to export the new `notFound` middleware, ensuring it is included in the middleware stack. * docs: Add comment to clarify usage of unsafeChars regex in notFound middleware - Included a comment in the notFound middleware file to explain that the unsafeChars regex is safe to reuse with .replace() at the module scope, as it does not retain lastIndex state.
This commit is contained in:
parent
a17a38b8ed
commit
6169d4f70b
9 changed files with 94 additions and 23 deletions
|
|
@ -124,7 +124,9 @@ class BaseClient {
|
|||
* @returns {number}
|
||||
*/
|
||||
getTokenCountForResponse(responseMessage) {
|
||||
logger.debug('[BaseClient] `recordTokenUsage` not implemented.', responseMessage);
|
||||
logger.debug('[BaseClient] `recordTokenUsage` not implemented.', {
|
||||
messageId: responseMessage?.messageId,
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -661,10 +663,13 @@ class BaseClient {
|
|||
);
|
||||
|
||||
if (tokenCountMap) {
|
||||
logger.debug('[BaseClient] tokenCountMap', tokenCountMap);
|
||||
if (tokenCountMap[userMessage.messageId]) {
|
||||
userMessage.tokenCount = tokenCountMap[userMessage.messageId];
|
||||
logger.debug('[BaseClient] userMessage', userMessage);
|
||||
logger.debug('[BaseClient] userMessage', {
|
||||
messageId: userMessage.messageId,
|
||||
tokenCount: userMessage.tokenCount,
|
||||
conversationId: userMessage.conversationId,
|
||||
});
|
||||
}
|
||||
|
||||
this.handleTokenCountMap(tokenCountMap);
|
||||
|
|
@ -793,6 +798,13 @@ class BaseClient {
|
|||
model: responseMessage.model,
|
||||
});
|
||||
}
|
||||
|
||||
logger.debug('[BaseClient] Response token usage', {
|
||||
messageId: responseMessage.messageId,
|
||||
model: responseMessage.model,
|
||||
promptTokens,
|
||||
completionTokens,
|
||||
});
|
||||
}
|
||||
|
||||
if (userMessagePromise) {
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ const { logger } = require('@librechat/data-schemas');
|
|||
const mongoSanitize = require('express-mongo-sanitize');
|
||||
const {
|
||||
isEnabled,
|
||||
apiNotFound,
|
||||
ErrorController,
|
||||
performStartupChecks,
|
||||
handleJsonParseError,
|
||||
|
|
@ -297,6 +298,7 @@ if (cluster.isMaster) {
|
|||
/** Routes */
|
||||
app.use('/oauth', routes.oauth);
|
||||
app.use('/api/auth', routes.auth);
|
||||
app.use('/api/admin', routes.adminAuth);
|
||||
app.use('/api/actions', routes.actions);
|
||||
app.use('/api/keys', routes.keys);
|
||||
app.use('/api/api-keys', routes.apiKeys);
|
||||
|
|
@ -310,7 +312,6 @@ if (cluster.isMaster) {
|
|||
app.use('/api/endpoints', routes.endpoints);
|
||||
app.use('/api/balance', routes.balance);
|
||||
app.use('/api/models', routes.models);
|
||||
app.use('/api/plugins', routes.plugins);
|
||||
app.use('/api/config', routes.config);
|
||||
app.use('/api/assistants', routes.assistants);
|
||||
app.use('/api/files', await routes.files.initialize());
|
||||
|
|
@ -324,8 +325,8 @@ if (cluster.isMaster) {
|
|||
app.use('/api/tags', routes.tags);
|
||||
app.use('/api/mcp', routes.mcp);
|
||||
|
||||
/** Error handler */
|
||||
app.use(ErrorController);
|
||||
/** 404 for unmatched API routes */
|
||||
app.use('/api', apiNotFound);
|
||||
|
||||
/** SPA fallback - serve index.html for all unmatched routes */
|
||||
app.use((req, res) => {
|
||||
|
|
@ -343,6 +344,9 @@ if (cluster.isMaster) {
|
|||
res.send(updatedIndexHtml);
|
||||
});
|
||||
|
||||
/** Error handler (must be last - Express identifies error middleware by its 4-arg signature) */
|
||||
app.use(ErrorController);
|
||||
|
||||
/** Start listening on shared port (cluster will distribute connections) */
|
||||
app.listen(port, host, async (err) => {
|
||||
if (err) {
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ const { logger } = require('@librechat/data-schemas');
|
|||
const mongoSanitize = require('express-mongo-sanitize');
|
||||
const {
|
||||
isEnabled,
|
||||
apiNotFound,
|
||||
ErrorController,
|
||||
memoryDiagnostics,
|
||||
performStartupChecks,
|
||||
|
|
@ -163,8 +164,10 @@ const startServer = async () => {
|
|||
app.use('/api/tags', routes.tags);
|
||||
app.use('/api/mcp', routes.mcp);
|
||||
|
||||
app.use(ErrorController);
|
||||
/** 404 for unmatched API routes */
|
||||
app.use('/api', apiNotFound);
|
||||
|
||||
/** SPA fallback - serve index.html for all unmatched routes */
|
||||
app.use((req, res) => {
|
||||
res.set({
|
||||
'Cache-Control': process.env.INDEX_CACHE_CONTROL || 'no-cache, no-store, must-revalidate',
|
||||
|
|
@ -180,6 +183,9 @@ const startServer = async () => {
|
|||
res.send(updatedIndexHtml);
|
||||
});
|
||||
|
||||
/** Error handler (must be last - Express identifies error middleware by its 4-arg signature) */
|
||||
app.use(ErrorController);
|
||||
|
||||
app.listen(port, host, async (err) => {
|
||||
if (err) {
|
||||
logger.error('Failed to start server:', err);
|
||||
|
|
|
|||
|
|
@ -100,6 +100,40 @@ describe('Server Configuration', () => {
|
|||
expect(response.headers['expires']).toBe('0');
|
||||
});
|
||||
|
||||
it('should return 404 JSON for undefined API routes', async () => {
|
||||
const response = await request(app).get('/api/nonexistent');
|
||||
expect(response.status).toBe(404);
|
||||
expect(response.body).toEqual({ message: 'Endpoint not found' });
|
||||
});
|
||||
|
||||
it('should return 404 JSON for nested undefined API routes', async () => {
|
||||
const response = await request(app).get('/api/nonexistent/nested/path');
|
||||
expect(response.status).toBe(404);
|
||||
expect(response.body).toEqual({ message: 'Endpoint not found' });
|
||||
});
|
||||
|
||||
it('should return 404 JSON for non-GET methods on undefined API routes', async () => {
|
||||
const post = await request(app).post('/api/nonexistent');
|
||||
expect(post.status).toBe(404);
|
||||
expect(post.body).toEqual({ message: 'Endpoint not found' });
|
||||
|
||||
const del = await request(app).delete('/api/nonexistent');
|
||||
expect(del.status).toBe(404);
|
||||
expect(del.body).toEqual({ message: 'Endpoint not found' });
|
||||
});
|
||||
|
||||
it('should return 404 JSON for the /api root path', async () => {
|
||||
const response = await request(app).get('/api');
|
||||
expect(response.status).toBe(404);
|
||||
expect(response.body).toEqual({ message: 'Endpoint not found' });
|
||||
});
|
||||
|
||||
it('should serve SPA HTML for non-API unmatched routes', async () => {
|
||||
const response = await request(app).get('/this/does/not/exist');
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.headers['content-type']).toMatch(/html/);
|
||||
});
|
||||
|
||||
it('should return 500 for unknown errors via ErrorController', async () => {
|
||||
// Testing the error handling here on top of unit tests to ensure the middleware is correctly integrated
|
||||
|
||||
|
|
|
|||
|
|
@ -55,16 +55,16 @@ const processAddedConvo = async ({
|
|||
userMCPAuthMap,
|
||||
}) => {
|
||||
const addedConvo = endpointOption.addedConvo;
|
||||
logger.debug('[processAddedConvo] Called with addedConvo:', {
|
||||
hasAddedConvo: addedConvo != null,
|
||||
addedConvoEndpoint: addedConvo?.endpoint,
|
||||
addedConvoModel: addedConvo?.model,
|
||||
addedConvoAgentId: addedConvo?.agent_id,
|
||||
});
|
||||
if (addedConvo == null) {
|
||||
return { userMCPAuthMap };
|
||||
}
|
||||
|
||||
logger.debug('[processAddedConvo] Processing added conversation', {
|
||||
model: addedConvo.model,
|
||||
agentId: addedConvo.agent_id,
|
||||
endpoint: addedConvo.endpoint,
|
||||
});
|
||||
|
||||
try {
|
||||
const addedAgent = await loadAddedAgent({ req, conversation: addedConvo, primaryAgent });
|
||||
if (!addedAgent) {
|
||||
|
|
|
|||
|
|
@ -204,13 +204,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => {
|
|||
);
|
||||
|
||||
logger.debug(
|
||||
`[initializeClient] Tool definitions for primary agent: ${primaryConfig.toolDefinitions?.length ?? 0}`,
|
||||
);
|
||||
|
||||
/** Store primary agent's tool context for ON_TOOL_EXECUTE callback */
|
||||
logger.debug(`[initializeClient] Storing tool context for agentId: ${primaryConfig.id}`);
|
||||
logger.debug(
|
||||
`[initializeClient] toolRegistry size: ${primaryConfig.toolRegistry?.size ?? 'undefined'}`,
|
||||
`[initializeClient] Storing tool context for ${primaryConfig.id}: ${primaryConfig.toolDefinitions?.length ?? 0} tools, registry size: ${primaryConfig.toolRegistry?.size ?? '0'}`,
|
||||
);
|
||||
agentToolContexts.set(primaryConfig.id, {
|
||||
agent: primaryAgent,
|
||||
|
|
|
|||
|
|
@ -475,13 +475,21 @@ ${memory ?? 'No existing memories'}`;
|
|||
};
|
||||
const content = await run.processStream(inputs, config);
|
||||
if (content) {
|
||||
logger.debug('Memory Agent processed memory successfully', content);
|
||||
logger.debug('[MemoryAgent] Processed successfully', {
|
||||
userId,
|
||||
conversationId,
|
||||
messageId,
|
||||
provider: llmConfig?.provider,
|
||||
});
|
||||
} else {
|
||||
logger.warn('Memory Agent processed memory but returned no content');
|
||||
logger.debug('[MemoryAgent] Returned no content', { userId, conversationId, messageId });
|
||||
}
|
||||
return await Promise.all(artifactPromises);
|
||||
} catch (error) {
|
||||
logger.error('Memory Agent failed to process memory', error);
|
||||
logger.error(
|
||||
`[MemoryAgent] Failed to process memory | userId: ${userId} | conversationId: ${conversationId} | messageId: ${messageId}`,
|
||||
{ error },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
export * from './access';
|
||||
export * from './admin';
|
||||
export * from './error';
|
||||
export * from './notFound';
|
||||
export * from './balance';
|
||||
export * from './json';
|
||||
export * from './concurrency';
|
||||
|
|
|
|||
12
packages/api/src/middleware/notFound.ts
Normal file
12
packages/api/src/middleware/notFound.ts
Normal file
|
|
@ -0,0 +1,12 @@
|
|||
import { logger } from '@librechat/data-schemas';
|
||||
import type { Request, Response } from 'express';
|
||||
|
||||
/** Safe to reuse with .replace() at module scope - does not retain lastIndex state */
|
||||
// eslint-disable-next-line no-control-regex
|
||||
const unsafeChars = /[\r\n\u0000]/g;
|
||||
|
||||
export function apiNotFound(req: Request, res: Response): void {
|
||||
const safePath = req.path.replace(unsafeChars, '_').slice(0, 200);
|
||||
logger.debug(`[API 404] ${req.method} ${safePath}`);
|
||||
res.status(404).json({ message: 'Endpoint not found' });
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue