From 6169d4f70b44e4cdc913ac4ad28e2b264804f49d Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 27 Feb 2026 22:49:54 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=9A=A6=20fix:=20404=20JSON=20Responses=20?= =?UTF-8?q?for=20Unmatched=20API=20Routes=20(#11976)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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. --- api/app/clients/BaseClient.js | 18 ++++++++-- api/server/experimental.js | 10 ++++-- api/server/index.js | 8 ++++- api/server/index.spec.js | 34 +++++++++++++++++++ .../services/Endpoints/agents/addedConvo.js | 12 +++---- .../services/Endpoints/agents/initialize.js | 8 +---- packages/api/src/agents/memory.ts | 14 ++++++-- packages/api/src/middleware/index.ts | 1 + packages/api/src/middleware/notFound.ts | 12 +++++++ 9 files changed, 94 insertions(+), 23 deletions(-) create mode 100644 packages/api/src/middleware/notFound.ts diff --git a/api/app/clients/BaseClient.js b/api/app/clients/BaseClient.js index e5771aac55..85963aec58 100644 --- a/api/app/clients/BaseClient.js +++ b/api/app/clients/BaseClient.js @@ -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) { diff --git a/api/server/experimental.js b/api/server/experimental.js index 4a457abf61..7b60ad7fd2 100644 --- a/api/server/experimental.js +++ b/api/server/experimental.js @@ -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) { diff --git a/api/server/index.js b/api/server/index.js index 2aff26ceaf..f034f10236 100644 --- a/api/server/index.js +++ b/api/server/index.js @@ -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); diff --git a/api/server/index.spec.js b/api/server/index.spec.js index c73c605518..7b3d062fce 100644 --- a/api/server/index.spec.js +++ b/api/server/index.spec.js @@ -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 diff --git a/api/server/services/Endpoints/agents/addedConvo.js b/api/server/services/Endpoints/agents/addedConvo.js index 7e9385267a..25b1327991 100644 --- a/api/server/services/Endpoints/agents/addedConvo.js +++ b/api/server/services/Endpoints/agents/addedConvo.js @@ -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) { diff --git a/api/server/services/Endpoints/agents/initialize.js b/api/server/services/Endpoints/agents/initialize.js index 0888f23cd5..fd2e42511d 100644 --- a/api/server/services/Endpoints/agents/initialize.js +++ b/api/server/services/Endpoints/agents/initialize.js @@ -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, diff --git a/packages/api/src/agents/memory.ts b/packages/api/src/agents/memory.ts index bb4bd38282..b8f65a9772 100644 --- a/packages/api/src/agents/memory.ts +++ b/packages/api/src/agents/memory.ts @@ -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 }, + ); } } diff --git a/packages/api/src/middleware/index.ts b/packages/api/src/middleware/index.ts index a208923a49..1f0cbc16fb 100644 --- a/packages/api/src/middleware/index.ts +++ b/packages/api/src/middleware/index.ts @@ -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'; diff --git a/packages/api/src/middleware/notFound.ts b/packages/api/src/middleware/notFound.ts new file mode 100644 index 0000000000..1eac18091f --- /dev/null +++ b/packages/api/src/middleware/notFound.ts @@ -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' }); +}