diff --git a/api/server/services/Files/Code/crud.js b/api/server/services/Files/Code/crud.js index c7e63540d2..aaa53a82d8 100644 --- a/api/server/services/Files/Code/crud.js +++ b/api/server/services/Files/Code/crud.js @@ -84,7 +84,6 @@ async function deleteCodeEnvFile(req, file) { return; } - const missingOrUnsupportedStatuses = new Set([404, 405]); for (const [executionRouteKey, ref] of refs) { const executionProfile = ref.executionProfile ?? 'default'; const environments = @@ -127,48 +126,37 @@ async function deleteCodeEnvFile(req, file) { const bridgeWorkerId = configuredEnvironment?.workerId ?? configuredEnvironment?.pairing?.workerId; const authHeaders = await getCodeApiAuthHeaders(req, bridgeWorkerId); - const baseRequest = { - method: 'delete', - headers: { - 'User-Agent': 'LibreChat/1.0', - ...authHeaders, - ...codeExecutionHeaders({ - executionProfile, - bridgeWorkerId, - }), - }, - httpAgent: codeServerHttpAgent, - httpsAgent: codeServerHttpsAgent, - timeout: 15000, - }; - const urls = [ - `${baseURL}/sessions/${ref.storage_session_id}/objects/${ref.file_id}${query}`, - `${baseURL}/files/${ref.storage_session_id}/${ref.file_id}${query}`, - ]; - - let lastError; - let deleted = false; - for (const url of urls) { - try { - await axios({ ...baseRequest, url }); - deleted = true; - break; - } catch (error) { - lastError = error; - if (!missingOrUnsupportedStatuses.has(error.response?.status)) { - throw error; - } - } - } - if (!deleted && lastError) { - logAxiosError({ - error: lastError, - message: `Error deleting code environment file: ${lastError.message}`, + /* codeapi has mounted DELETE at `/files/:session_id/:fileId` since its + * first release. The file-server's own `/sessions/:id/objects/:fileId` + * only gained DELETE in LibreChat-AI/code-interpreter#85, so trying it + * first cost a guaranteed 404 against every older deployment. */ + try { + await axios({ + method: 'delete', + url: `${baseURL}/files/${ref.storage_session_id}/${ref.file_id}${query}`, + headers: { + 'User-Agent': 'LibreChat/1.0', + ...authHeaders, + ...codeExecutionHeaders({ + executionProfile, + bridgeWorkerId, + }), + }, + httpAgent: codeServerHttpAgent, + httpsAgent: codeServerHttpsAgent, + timeout: 15000, }); - if (lastError.response?.status === 404) { - continue; + } catch (error) { + if (error.response?.status !== 404) { + throw error; } - throw new Error(lastError.message || 'An error occurred during file deletion.'); + /* Already gone. Logged rather than swallowed: a 404 from a + * misconfigured base URL is indistinguishable from one for an absent + * object, and this branch drops the file's record either way. */ + logAxiosError({ + error, + message: `Code environment object already absent: ${error.message}`, + }); } } } diff --git a/api/server/services/Files/Code/crud.spec.js b/api/server/services/Files/Code/crud.spec.js index 4e5c1242db..e35758ee35 100644 --- a/api/server/services/Files/Code/crud.spec.js +++ b/api/server/services/Files/Code/crud.spec.js @@ -212,7 +212,7 @@ describe('Code CRUD', () => { expect(mockAxios).toHaveBeenCalledWith( expect.objectContaining({ method: 'delete', - url: 'https://code-api.example.com/sessions/session-1/objects/file-1?kind=agent&id=agent-abc', + url: 'https://code-api.example.com/files/session-1/file-1?kind=agent&id=agent-abc', headers: expect.objectContaining({ Authorization: 'Bearer codeapi-token', 'User-Agent': 'LibreChat/1.0', @@ -239,7 +239,7 @@ describe('Code CRUD', () => { expect(mockAxios).toHaveBeenCalledWith( expect.objectContaining({ - url: 'https://code-stateful.example.com/sessions/session-1/objects/file-1?kind=agent&id=agent-abc', + url: 'https://code-stateful.example.com/files/session-1/file-1?kind=agent&id=agent-abc', headers: expect.objectContaining({ 'X-CodeAPI-Expected-Profile': 'stateful', }), @@ -270,14 +270,14 @@ describe('Code CRUD', () => { expect(mockAxios).toHaveBeenNthCalledWith( 1, expect.objectContaining({ - url: expect.stringContaining('/sessions/session-1/objects/file-1'), + url: expect.stringContaining('/files/session-1/file-1'), headers: expect.objectContaining({ 'X-CodeAPI-Expected-Profile': 'default' }), }), ); expect(mockAxios).toHaveBeenNthCalledWith( 2, expect.objectContaining({ - url: expect.stringContaining('/sessions/stateful-session/objects/stateful-file'), + url: expect.stringContaining('/files/stateful-session/stateful-file'), headers: expect.objectContaining({ 'X-CodeAPI-Expected-Profile': 'stateful' }), }), ); @@ -320,33 +320,27 @@ describe('Code CRUD', () => { expect(mockAxios).not.toHaveBeenCalled(); }); - it.each([404, 405])( - 'falls back to the legacy code environment delete route after a %s', - async (status) => { - mockAxios - .mockRejectedValueOnce( - Object.assign(new Error('missing route'), { response: { status } }), - ) - .mockResolvedValueOnce({ status: 204 }); + it('never calls the file-server path that only newer codeapi mounts', async () => { + mockAxios.mockResolvedValue({ status: 204 }); - await deleteCodeEnvFile(req, file); + await deleteCodeEnvFile(req, file); - expect(mockAxios).toHaveBeenNthCalledWith( - 1, - expect.objectContaining({ - method: 'delete', - url: 'https://code-api.example.com/sessions/session-1/objects/file-1?kind=agent&id=agent-abc', - }), - ); - expect(mockAxios).toHaveBeenNthCalledWith( - 2, - expect.objectContaining({ - method: 'delete', - url: 'https://code-api.example.com/files/session-1/file-1?kind=agent&id=agent-abc', - }), - ); - }, - ); + expect(mockAxios).toHaveBeenCalledTimes(1); + expect(mockAxios).not.toHaveBeenCalledWith( + expect.objectContaining({ + url: expect.stringContaining('/sessions/session-1/objects/file-1'), + }), + ); + }); + + it('surfaces an unsupported delete method instead of retrying elsewhere', async () => { + mockAxios.mockRejectedValue( + Object.assign(new Error('method not allowed'), { response: { status: 405 } }), + ); + + await expect(deleteCodeEnvFile(req, file)).rejects.toThrow('method not allowed'); + expect(mockAxios).toHaveBeenCalledTimes(1); + }); it('skips files without a code environment ref', async () => { await deleteCodeEnvFile(req, {}); @@ -356,12 +350,12 @@ describe('Code CRUD', () => { }); it('treats missing code environment objects as already deleted', async () => { - mockAxios - .mockRejectedValueOnce(Object.assign(new Error('missing'), { response: { status: 404 } })) - .mockRejectedValueOnce(Object.assign(new Error('missing'), { response: { status: 404 } })); + mockAxios.mockRejectedValue( + Object.assign(new Error('missing'), { response: { status: 404 } }), + ); await expect(deleteCodeEnvFile(req, file)).resolves.toBeUndefined(); - expect(mockAxios).toHaveBeenCalledTimes(2); + expect(mockAxios).toHaveBeenCalledTimes(1); }); it('throws when code environment deletion fails', async () => {