Skip to content

Commit c589adf

Browse files
committed
fix(tools): preserve internal error body fallbacks
1 parent f70f154 commit c589adf

File tree

2 files changed

+79
-4
lines changed

2 files changed

+79
-4
lines changed

apps/sim/tools/index.test.ts

Lines changed: 77 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,81 @@ describe('Internal Tool Timeout Behavior', () => {
649649
tools.function_execute = originalFunctionTool
650650
}
651651
})
652+
653+
it('should preserve plain text error bodies for internal secure fetch responses', async () => {
654+
vi.spyOn(securityValidation, 'secureFetchWithPinnedIP').mockResolvedValue({
655+
ok: false,
656+
status: 401,
657+
statusText: 'Unauthorized',
658+
headers: new securityValidation.SecureFetchHeaders({ 'content-type': 'text/plain' }),
659+
text: async () => 'Invalid access token',
660+
json: async () => {
661+
throw new Error('Invalid JSON')
662+
},
663+
arrayBuffer: async () => new TextEncoder().encode('Invalid access token').buffer,
664+
})
665+
666+
const result = await executeTool(
667+
'function_execute',
668+
{
669+
code: 'return 1',
670+
},
671+
true
672+
)
673+
674+
expect(result.success).toBe(false)
675+
expect(result.error).toBe('Invalid access token')
676+
})
677+
678+
it('should not read a body for internal 204 responses', async () => {
679+
const arrayBufferSpy = vi.fn(async () => new ArrayBuffer(0))
680+
const secureFetchSpy = vi
681+
.spyOn(securityValidation, 'secureFetchWithPinnedIP')
682+
.mockResolvedValue({
683+
ok: true,
684+
status: 204,
685+
statusText: 'No Content',
686+
headers: new securityValidation.SecureFetchHeaders({}),
687+
text: async () => '',
688+
json: async () => {
689+
throw new Error('No response body')
690+
},
691+
arrayBuffer: arrayBufferSpy,
692+
})
693+
694+
const originalTool = (tools as any).test_internal_no_content
695+
;(tools as any).test_internal_no_content = {
696+
id: 'test_internal_no_content',
697+
name: 'Test Internal No Content',
698+
description: 'A test tool for internal 204 responses',
699+
version: '1.0.0',
700+
params: {},
701+
request: {
702+
url: '/api/test/no-content',
703+
method: 'POST',
704+
headers: () => ({ 'Content-Type': 'application/json' }),
705+
},
706+
}
707+
708+
try {
709+
const result = await executeTool('test_internal_no_content', {}, true)
710+
711+
expect(result.success).toBe(true)
712+
expect(result.output).toEqual({ status: 204 })
713+
expect(secureFetchSpy).toHaveBeenCalledWith(
714+
expect.stringContaining('/api/test/no-content'),
715+
'127.0.0.1',
716+
expect.any(Object)
717+
)
718+
expect(arrayBufferSpy).not.toHaveBeenCalled()
719+
} finally {
720+
if (originalTool) {
721+
;(tools as any).test_internal_no_content = originalTool
722+
} else {
723+
delete (tools as any).test_internal_no_content
724+
}
725+
}
726+
})
652727
})
653728

654729
describe('Automatic Internal Route Detection', () => {
@@ -1048,8 +1123,7 @@ describe('Centralized Error Handling', () => {
10481123
)
10491124

10501125
expect(result.success).toBe(false)
1051-
// Current behavior falls back to HTTP status text on JSON parsing failures.
1052-
expect(result.error).toBe('Unauthorized')
1126+
expect(result.error).toBe('Invalid access token')
10531127
})
10541128

10551129
it('should handle plain text error responses from APIs like Apollo', async () => {
@@ -1078,7 +1152,7 @@ describe('Centralized Error Handling', () => {
10781152
)
10791153

10801154
expect(result.success).toBe(false)
1081-
expect(result.error).toBe('Forbidden')
1155+
expect(result.error).toBe('Invalid API key provided')
10821156
})
10831157

10841158
it('should fall back to HTTP status text when both JSON and text parsing fail', async () => {

apps/sim/tools/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1359,8 +1359,9 @@ async function executeToolRequest(
13591359
}
13601360

13611361
let errorData: any
1362+
const jsonProbe = response.clone()
13621363
try {
1363-
errorData = await response.json()
1364+
errorData = await jsonProbe.json()
13641365
} catch (jsonError) {
13651366
// JSON parsing failed, fall back to reading as text for error extraction
13661367
logger.warn(`[${requestId}] Response is not JSON for ${toolId}, reading as text`)

0 commit comments

Comments
 (0)