fix: improve streaming error propagation and forwarded response logging (#5317) * Fix: Improve streaming error handling and forwarded response logging * Fix: fix ESLint error Strings must use singlequote quotes * fix: preserve and log forwarded stream errors * chore: narrow forwarded stream error fix scope * fix: make forwardFetchResponse awaitable and forward upstream error text * Restore original happy path handling * Remove redundant checks in forwardFetchResponse function * Don't send anything on parsing error end --------- Co-authored-by: Cohee <18619528+Cohee1207@users.noreply.github.com>
Signed| @@ -372,7 +372,7 @@ async function sendClaudeRequest(request, response) { | |||
| 372 | 372 | ||
| 373 | if (request.body.stream) { | 373 | if (request.body.stream) { |
| 374 | // Pipe remote SSE stream to Express response | 374 | // Pipe remote SSE stream to Express response |
| 375 | forwardFetchResponse(generateResponse, response); | 375 | await forwardFetchResponse(generateResponse, response); |
| 376 | } else { | 376 | } else { |
| 377 | if (!generateResponse.ok) { | 377 | if (!generateResponse.ok) { |
| 378 | const generateResponseText = await generateResponse.text(); | 378 | const generateResponseText = await generateResponse.text(); |
| @@ -682,7 +682,7 @@ async function sendMakerSuiteRequest(request, response) { | |||
| 682 | if (stream) { | 682 | if (stream) { |
| 683 | try { | 683 | try { |
| 684 | // Pipe remote SSE stream to Express response | 684 | // Pipe remote SSE stream to Express response |
| 685 | forwardFetchResponse(generateResponse, response); | 685 | await forwardFetchResponse(generateResponse, response); |
| 686 | } catch (error) { | 686 | } catch (error) { |
| 687 | console.error('Error forwarding streaming response:', error); | 687 | console.error('Error forwarding streaming response:', error); |
| 688 | if (!response.headersSent) { | 688 | if (!response.headersSent) { |
| @@ -793,7 +793,7 @@ async function sendAI21Request(request, response) { | |||
| 793 | try { | 793 | try { |
| 794 | const generateResponse = await fetch(API_AI21 + '/chat/completions', options); | 794 | const generateResponse = await fetch(API_AI21 + '/chat/completions', options); |
| 795 | if (request.body.stream) { | 795 | if (request.body.stream) { |
| 796 | forwardFetchResponse(generateResponse, response); | 796 | await forwardFetchResponse(generateResponse, response); |
| 797 | } else { | 797 | } else { |
| 798 | if (!generateResponse.ok) { | 798 | if (!generateResponse.ok) { |
| 799 | const errorText = await generateResponse.text(); | 799 | const errorText = await generateResponse.text(); |
| @@ -883,7 +883,7 @@ async function sendMistralAIRequest(request, response) { | |||
| 883 | 883 | ||
| 884 | const generateResponse = await fetch(apiUrl + '/chat/completions', config); | 884 | const generateResponse = await fetch(apiUrl + '/chat/completions', config); |
| 885 | if (request.body.stream) { | 885 | if (request.body.stream) { |
| 886 | forwardFetchResponse(generateResponse, response); | 886 | await forwardFetchResponse(generateResponse, response); |
| 887 | } else { | 887 | } else { |
| 888 | if (!generateResponse.ok) { | 888 | if (!generateResponse.ok) { |
| 889 | const errorText = await generateResponse.text(); | 889 | const errorText = await generateResponse.text(); |
| @@ -982,7 +982,7 @@ async function sendCohereRequest(request, response) { | |||
| 982 | 982 | ||
| 983 | if (request.body.stream) { | 983 | if (request.body.stream) { |
| 984 | const stream = await fetch(apiUrl, config); | 984 | const stream = await fetch(apiUrl, config); |
| 985 | forwardFetchResponse(stream, response); | 985 | await forwardFetchResponse(stream, response); |
| 986 | } else { | 986 | } else { |
| 987 | const generateResponse = await fetch(apiUrl, config); | 987 | const generateResponse = await fetch(apiUrl, config); |
| 988 | if (!generateResponse.ok) { | 988 | if (!generateResponse.ok) { |
| @@ -1093,7 +1093,7 @@ async function sendDeepSeekRequest(request, response) { | |||
| 1093 | const generateResponse = await fetch(apiUrl + '/chat/completions', config); | 1093 | const generateResponse = await fetch(apiUrl + '/chat/completions', config); |
| 1094 | 1094 | ||
| 1095 | if (request.body.stream) { | 1095 | if (request.body.stream) { |
| 1096 | forwardFetchResponse(generateResponse, response); | 1096 | await forwardFetchResponse(generateResponse, response); |
| 1097 | } else { | 1097 | } else { |
| 1098 | if (!generateResponse.ok) { | 1098 | if (!generateResponse.ok) { |
| 1099 | const errorText = await generateResponse.text(); | 1099 | const errorText = await generateResponse.text(); |
| @@ -1199,7 +1199,7 @@ async function sendXaiRequest(request, response) { | |||
| 1199 | const generateResponse = await fetch(apiUrl + '/chat/completions', config); | 1199 | const generateResponse = await fetch(apiUrl + '/chat/completions', config); |
| 1200 | 1200 | ||
| 1201 | if (request.body.stream) { | 1201 | if (request.body.stream) { |
| 1202 | forwardFetchResponse(generateResponse, response); | 1202 | await forwardFetchResponse(generateResponse, response); |
| 1203 | } else { | 1203 | } else { |
| 1204 | if (!generateResponse.ok) { | 1204 | if (!generateResponse.ok) { |
| 1205 | const errorText = await generateResponse.text(); | 1205 | const errorText = await generateResponse.text(); |
| @@ -1304,7 +1304,7 @@ async function sendAimlapiRequest(request, response) { | |||
| 1304 | const generateResponse = await fetch(apiUrl + '/chat/completions', config); | 1304 | const generateResponse = await fetch(apiUrl + '/chat/completions', config); |
| 1305 | 1305 | ||
| 1306 | if (request.body.stream) { | 1306 | if (request.body.stream) { |
| 1307 | forwardFetchResponse(generateResponse, response); | 1307 | await forwardFetchResponse(generateResponse, response); |
| 1308 | } else { | 1308 | } else { |
| 1309 | if (!generateResponse.ok) { | 1309 | if (!generateResponse.ok) { |
| 1310 | const errorText = await generateResponse.text(); | 1310 | const errorText = await generateResponse.text(); |
| @@ -1416,7 +1416,7 @@ async function sendElectronHubRequest(request, response) { | |||
| 1416 | const generateResponse = await fetch(apiUrl + '/chat/completions', config); | 1416 | const generateResponse = await fetch(apiUrl + '/chat/completions', config); |
| 1417 | 1417 | ||
| 1418 | if (request.body.stream) { | 1418 | if (request.body.stream) { |
| 1419 | forwardFetchResponse(generateResponse, response); | 1419 | await forwardFetchResponse(generateResponse, response); |
| 1420 | } else { | 1420 | } else { |
| 1421 | if (!generateResponse.ok) { | 1421 | if (!generateResponse.ok) { |
| 1422 | const errorText = await generateResponse.text(); | 1422 | const errorText = await generateResponse.text(); |
| @@ -1517,7 +1517,7 @@ async function sendChutesRequest(request, response) { | |||
| 1517 | const generateResponse = await fetch(apiUrl + '/chat/completions', config); | 1517 | const generateResponse = await fetch(apiUrl + '/chat/completions', config); |
| 1518 | 1518 | ||
| 1519 | if (request.body.stream) { | 1519 | if (request.body.stream) { |
| 1520 | forwardFetchResponse(generateResponse, response); | 1520 | await forwardFetchResponse(generateResponse, response); |
| 1521 | } else { | 1521 | } else { |
| 1522 | if (!generateResponse.ok) { | 1522 | if (!generateResponse.ok) { |
| 1523 | const errorText = await generateResponse.text(); | 1523 | const errorText = await generateResponse.text(); |
| @@ -1612,7 +1612,7 @@ async function sendAzureOpenAIRequest(request, response) { | |||
| 1612 | const fetchResponse = await fetch(endpointUrl, config); | 1612 | const fetchResponse = await fetch(endpointUrl, config); |
| 1613 | 1613 | ||
| 1614 | if (request.body.stream) { | 1614 | if (request.body.stream) { |
| 1615 | return forwardFetchResponse(fetchResponse, response); | 1615 | return await forwardFetchResponse(fetchResponse, response); |
| 1616 | } | 1616 | } |
| 1617 | 1617 | ||
| 1618 | if (fetchResponse.ok) { | 1618 | if (fetchResponse.ok) { |
| @@ -2411,7 +2411,7 @@ router.post('/generate', async function (request, response) { | |||
| 2411 | 2411 | ||
| 2412 | if (request.body.stream) { | 2412 | if (request.body.stream) { |
| 2413 | console.info('Streaming request in progress'); | 2413 | console.info('Streaming request in progress'); |
| 2414 | return forwardFetchResponse(fetchResponse, response); | 2414 | return await forwardFetchResponse(fetchResponse, response); |
| 2415 | } | 2415 | } |
| 2416 | 2416 | ||
| 2417 | if (fetchResponse.ok) { | 2417 | if (fetchResponse.ok) { |
| @@ -99,7 +99,7 @@ router.post('/generate', async function (request, response_generate) { | |||
| 99 | 99 | ||
| 100 | if (request.body.streaming) { | 100 | if (request.body.streaming) { |
| 101 | // Pipe remote SSE stream to Express response | 101 | // Pipe remote SSE stream to Express response |
| 102 | forwardFetchResponse(response, response_generate); | 102 | await forwardFetchResponse(response, response_generate); |
| 103 | return; | 103 | return; |
| 104 | } else { | 104 | } else { |
| 105 | if (!response.ok) { | 105 | if (!response.ok) { |
| @@ -404,7 +404,7 @@ router.post('/generate', async function (request, response) { | |||
| 404 | } else if (request.body.stream) { | 404 | } else if (request.body.stream) { |
| 405 | const completionsStream = await fetch(url, args); | 405 | const completionsStream = await fetch(url, args); |
| 406 | // Pipe remote SSE stream to Express response | 406 | // Pipe remote SSE stream to Express response |
| 407 | forwardFetchResponse(completionsStream, response); | 407 | await forwardFetchResponse(completionsStream, response); |
| 408 | } else { | 408 | } else { |
| 409 | const completionsReply = await fetch(url, args); | 409 | const completionsReply = await fetch(url, args); |
| 410 | 410 | ||
| @@ -270,7 +270,7 @@ router.post('/generate', async function (req, res) { | |||
| 270 | 270 | ||
| 271 | if (req.body.streaming) { | 271 | if (req.body.streaming) { |
| 272 | // Pipe remote SSE stream to Express response | 272 | // Pipe remote SSE stream to Express response |
| 273 | forwardFetchResponse(response, res); | 273 | await forwardFetchResponse(response, res); |
| 274 | } else { | 274 | } else { |
| 275 | if (!response.ok) { | 275 | if (!response.ok) { |
| 276 | const text = await response.text(); | 276 | const text = await response.text(); |
| @@ -264,7 +264,7 @@ elevenlabs.post('/synthesize', async (req, res) => { | |||
| 264 | } | 264 | } |
| 265 | 265 | ||
| 266 | res.set('Content-Type', 'audio/mpeg'); | 266 | res.set('Content-Type', 'audio/mpeg'); |
| 267 | forwardFetchResponse(response, res); | 267 | await forwardFetchResponse(response, res); |
| 268 | } catch (error) { | 268 | } catch (error) { |
| 269 | console.error(error); | 269 | console.error(error); |
| 270 | return res.sendStatus(500); | 270 | return res.sendStatus(500); |
| @@ -328,7 +328,7 @@ elevenlabs.post('/history-audio', async (req, res) => { | |||
| 328 | } | 328 | } |
| 329 | 329 | ||
| 330 | res.set('Content-Type', 'audio/mpeg'); | 330 | res.set('Content-Type', 'audio/mpeg'); |
| 331 | forwardFetchResponse(response, res); | 331 | await forwardFetchResponse(response, res); |
| 332 | } catch (error) { | 332 | } catch (error) { |
| 333 | console.error(error); | 333 | console.error(error); |
| 334 | return res.sendStatus(500); | 334 | return res.sendStatus(500); |
| @@ -35,7 +35,7 @@ export default async function corsProxyMiddleware(req, res) { | |||
| 35 | }); | 35 | }); |
| 36 | 36 | ||
| 37 | // Copy over relevant response params to the proxy response | 37 | // Copy over relevant response params to the proxy response |
| 38 | forwardFetchResponse(response, res); | 38 | await forwardFetchResponse(response, res); |
| 39 | } catch (error) { | 39 | } catch (error) { |
| 40 | res.status(500).send('Error occurred while trying to proxy to: ' + url + ' ' + error); | 40 | res.status(500).send('Error occurred while trying to proxy to: ' + url + ' ' + error); |
| 41 | } | 41 | } |
| @@ -704,15 +704,12 @@ export function getImages(directoryPath, sortBy = 'name', type = MEDIA_REQUEST_T | |||
| 704 | * Pipe a fetch() response to an Express.js Response, including status code. | 704 | * Pipe a fetch() response to an Express.js Response, including status code. |
| 705 | * @param {import('node-fetch').Response} from The Fetch API response to pipe from. | 705 | * @param {import('node-fetch').Response} from The Fetch API response to pipe from. |
| 706 | * @param {import('express').Response} to The Express response to pipe to. | 706 | * @param {import('express').Response} to The Express response to pipe to. |
| 707 | * @returns {Promise<void>} | ||
| 707 | */ | 708 | */ |
| 708 | export function forwardFetchResponse(from, to) { | 709 | export async function forwardFetchResponse(from, to) { |
| 709 | let statusCode = from.status; | 710 | let statusCode = from.status; |
| 710 | let statusText = from.statusText; | 711 | let statusText = from.statusText; |
| 711 | 712 | ||
| 712 | if (!from.ok) { | ||
| 713 | console.warn(`Streaming request failed with status ${statusCode} ${statusText}`); | ||
| 714 | } | ||
| 715 | |||
| 716 | // Avoid sending 401 responses as they reset the client Basic auth. | 713 | // Avoid sending 401 responses as they reset the client Basic auth. |
| 717 | // This can produce an interesting artifact as "400 Unauthorized", but it's not out of spec. | 714 | // This can produce an interesting artifact as "400 Unauthorized", but it's not out of spec. |
| 718 | // https://www.rfc-editor.org/rfc/rfc9110.html#name-overview-of-status-codes | 715 | // https://www.rfc-editor.org/rfc/rfc9110.html#name-overview-of-status-codes |
| @@ -725,6 +722,21 @@ export function forwardFetchResponse(from, to) { | |||
| 725 | to.statusCode = statusCode; | 722 | to.statusCode = statusCode; |
| 726 | to.statusMessage = statusText; | 723 | to.statusMessage = statusText; |
| 727 | 724 | ||
| 725 | if (!from.ok) { | ||
| 726 | try { | ||
| 727 | const rawErrorText = await from.text(); | ||
| 728 | const detail = rawErrorText || 'Unknown error occurred'; | ||
| 729 | |||
| 730 | console.warn(`Streaming request failed with status ${from.status} ${statusText}: ${detail}`); | ||
| 731 | to.end(rawErrorText, 'utf-8'); | ||
| 732 | } catch { | ||
| 733 | console.warn(`Streaming request failed with status ${from.status} ${statusText}: Unknown error occurred`); | ||
| 734 | to.end(); | ||
| 735 | } | ||
| 736 | |||
| 737 | return; | ||
| 738 | } | ||
| 739 | |||
| 728 | if (from.body && to.socket) { | 740 | if (from.body && to.socket) { |
| 729 | from.body.pipe(to); | 741 | from.body.pipe(to); |
| 730 | 742 | ||
| @@ -1,6 +1,31 @@ | |||
| 1 | import { describe, test, expect } from '@jest/globals'; | 1 | import { afterEach, describe, test, expect, jest } from '@jest/globals'; |
| 2 | import { once } from 'node:events'; | ||
| 3 | import { PassThrough } from 'node:stream'; | ||
| 4 | import { Response } from 'node-fetch'; | ||
| 2 | import { CHAT_COMPLETION_SOURCES } from '../src/constants'; | 5 | import { CHAT_COMPLETION_SOURCES } from '../src/constants'; |
| 3 | import { flattenSchema } from '../src/util'; | 6 | import { flattenSchema, forwardFetchResponse } from '../src/util'; |
| 7 | |||
| 8 | function createMockExpressResponse() { | ||
| 9 | const response = new PassThrough(); | ||
| 10 | response.statusCode = 200; | ||
| 11 | response.statusMessage = ''; | ||
| 12 | |||
| 13 | return response; | ||
| 14 | } | ||
| 15 | |||
| 16 | async function collectResponseBody(response) { | ||
| 17 | const chunks = []; | ||
| 18 | |||
| 19 | response.on('data', chunk => chunks.push(Buffer.from(chunk))); | ||
| 20 | |||
| 21 | await once(response, 'finish'); | ||
| 22 | |||
| 23 | return Buffer.concat(chunks).toString('utf8'); | ||
| 24 | } | ||
| 25 | |||
| 26 | afterEach(() => { | ||
| 27 | jest.restoreAllMocks(); | ||
| 28 | }); | ||
| 4 | 29 | ||
| 5 | describe('flattenSchema', () => { | 30 | describe('flattenSchema', () => { |
| 6 | test('should return the schema if it is not an object', () => { | 31 | test('should return the schema if it is not an object', () => { |
| @@ -105,3 +130,37 @@ describe('flattenSchema', () => { | |||
| 105 | expect(flattenSchema(schema, 'some-other-api')).toEqual(expected); | 130 | expect(flattenSchema(schema, 'some-other-api')).toEqual(expected); |
| 106 | }); | 131 | }); |
| 107 | }); | 132 | }); |
| 133 | |||
| 134 | describe('forwardFetchResponse', () => { | ||
| 135 | test('should log JSON error bodies and return the original body for non-2xx streaming responses', async () => { | ||
| 136 | const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => undefined); | ||
| 137 | const body = JSON.stringify({ error: { message: 'Forbidden by upstream policy' }, detail: 'policy_denied' }); | ||
| 138 | const response = createMockExpressResponse(); | ||
| 139 | const bodyPromise = collectResponseBody(response); | ||
| 140 | |||
| 141 | await forwardFetchResponse(new Response(body, { | ||
| 142 | status: 403, | ||
| 143 | statusText: 'Forbidden', | ||
| 144 | }), response); | ||
| 145 | |||
| 146 | expect(await bodyPromise).toBe(body); | ||
| 147 | expect(response.statusCode).toBe(403); | ||
| 148 | expect(warnSpy).toHaveBeenCalledWith(`Streaming request failed with status 403 Forbidden: ${body}`); | ||
| 149 | }); | ||
| 150 | |||
| 151 | test('should log plain text error bodies and return the original body for non-2xx streaming responses', async () => { | ||
| 152 | const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => undefined); | ||
| 153 | const body = 'Plain text upstream failure'; | ||
| 154 | const response = createMockExpressResponse(); | ||
| 155 | const bodyPromise = collectResponseBody(response); | ||
| 156 | |||
| 157 | await forwardFetchResponse(new Response(body, { | ||
| 158 | status: 502, | ||
| 159 | statusText: 'Bad Gateway', | ||
| 160 | }), response); | ||
| 161 | |||
| 162 | expect(await bodyPromise).toBe(body); | ||
| 163 | expect(response.statusCode).toBe(502); | ||
| 164 | expect(warnSpy).toHaveBeenCalledWith(`Streaming request failed with status 502 Bad Gateway: ${body}`); | ||
| 165 | }); | ||
| 166 | }); | ||