Skip to content

Commit 4107948

Browse files
authored
Merge pull request #954 from simstudioai/staging
fix
2 parents f7573fa + 7ebc875 commit 4107948

File tree

5 files changed

+78
-66
lines changed

5 files changed

+78
-66
lines changed

apps/sim/app/api/proxy/route.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,14 +167,15 @@ export async function POST(request: Request) {
167167

168168
try {
169169
// Parse request body
170+
const requestText = await request.text()
170171
let requestBody
171172
try {
172-
requestBody = await request.json()
173+
requestBody = JSON.parse(requestText)
173174
} catch (parseError) {
174-
logger.error(`[${requestId}] Failed to parse request body`, {
175+
logger.error(`[${requestId}] Failed to parse request body: ${requestText}`, {
175176
error: parseError instanceof Error ? parseError.message : String(parseError),
176177
})
177-
throw new Error('Invalid JSON in request body')
178+
throw new Error(`Invalid JSON in request body: ${requestText}`)
178179
}
179180

180181
const { toolId, params, executionContext } = requestBody

apps/sim/tools/__test-utils__/test-tools.ts

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,22 +40,37 @@ export function createMockFetch(
4040
) {
4141
const { ok = true, status = 200, headers = { 'Content-Type': 'application/json' } } = options
4242

43-
const mockFn = vi.fn().mockResolvedValue({
44-
ok,
45-
status,
46-
headers: {
47-
get: (key: string) => headers[key.toLowerCase()],
48-
forEach: (callback: (value: string, key: string) => void) => {
49-
Object.entries(headers).forEach(([key, value]) => callback(value, key))
50-
},
51-
},
52-
json: vi.fn().mockResolvedValue(responseData),
53-
text: vi
43+
// Normalize header keys to lowercase for case-insensitive access
44+
const normalizedHeaders: Record<string, string> = {}
45+
Object.entries(headers).forEach(([key, value]) => (normalizedHeaders[key.toLowerCase()] = value))
46+
47+
const makeResponse = () => {
48+
const jsonMock = vi.fn().mockResolvedValue(responseData)
49+
const textMock = vi
5450
.fn()
5551
.mockResolvedValue(
5652
typeof responseData === 'string' ? responseData : JSON.stringify(responseData)
57-
),
58-
})
53+
)
54+
55+
const res: any = {
56+
ok,
57+
status,
58+
headers: {
59+
get: (key: string) => normalizedHeaders[key.toLowerCase()],
60+
forEach: (callback: (value: string, key: string) => void) => {
61+
Object.entries(normalizedHeaders).forEach(([key, value]) => callback(value, key))
62+
},
63+
},
64+
json: jsonMock,
65+
text: textMock,
66+
}
67+
68+
// Implement clone() so production code that clones responses keeps working in tests
69+
res.clone = vi.fn().mockImplementation(() => makeResponse())
70+
return res
71+
}
72+
73+
const mockFn = vi.fn().mockResolvedValue(makeResponse())
5974

6075
// Add preconnect property to satisfy TypeScript
6176

apps/sim/tools/http/request.ts

Lines changed: 37 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -56,29 +56,12 @@ export const requestTool: ToolConfig<RequestParams, RequestResponse> = {
5656
// Process the URL first to handle path/query params
5757
const processedUrl = processUrl(params.url, params.pathParams, params.params)
5858

59-
// For external URLs that need proxying
59+
// For external URLs that need proxying in the browser, we still return the
60+
// external URL here and let executeTool route through the POST /api/proxy
61+
// endpoint uniformly. This avoids querystring body encoding and prevents
62+
// the proxy GET route from being hit from the client.
6063
if (shouldUseProxy(processedUrl)) {
61-
let proxyUrl = `/api/proxy?url=${encodeURIComponent(processedUrl)}`
62-
63-
if (params.method) {
64-
proxyUrl += `&method=${encodeURIComponent(params.method)}`
65-
}
66-
67-
if (params.body && ['POST', 'PUT', 'PATCH'].includes(params.method?.toUpperCase() || '')) {
68-
const bodyStr =
69-
typeof params.body === 'string' ? params.body : JSON.stringify(params.body)
70-
proxyUrl += `&body=${encodeURIComponent(bodyStr)}`
71-
}
72-
73-
// Forward all headers as URL parameters
74-
const userHeaders = transformTable(params.headers || null)
75-
for (const [key, value] of Object.entries(userHeaders)) {
76-
if (value !== undefined && value !== null) {
77-
proxyUrl += `&header.${encodeURIComponent(key)}=${encodeURIComponent(String(value))}`
78-
}
79-
}
80-
81-
return proxyUrl
64+
return processedUrl
8265
}
8366

8467
return processedUrl
@@ -137,13 +120,26 @@ export const requestTool: ToolConfig<RequestParams, RequestResponse> = {
137120
},
138121

139122
transformResponse: async (response: Response) => {
140-
// For proxy responses, we need to parse the JSON and extract the data
123+
// Build headers once for consistent return structures
124+
const headers: Record<string, string> = {}
125+
response.headers.forEach((value, key) => {
126+
headers[key] = value
127+
})
128+
141129
const contentType = response.headers.get('content-type') || ''
142-
if (contentType.includes('application/json')) {
143-
const jsonResponse = await response.json()
130+
const isJson = contentType.includes('application/json')
131+
132+
if (isJson) {
133+
// Use a clone to safely inspect JSON without consuming the original body
134+
let jsonResponse: any
135+
try {
136+
jsonResponse = await response.clone().json()
137+
} catch (_e) {
138+
jsonResponse = undefined
139+
}
144140

145-
// Check if this is a proxy response
146-
if (jsonResponse.data !== undefined && jsonResponse.status !== undefined) {
141+
// Proxy responses wrap the real payload
142+
if (jsonResponse && jsonResponse.data !== undefined && jsonResponse.status !== undefined) {
147143
return {
148144
success: jsonResponse.success,
149145
output: {
@@ -153,32 +149,30 @@ export const requestTool: ToolConfig<RequestParams, RequestResponse> = {
153149
},
154150
error: jsonResponse.success
155151
? undefined
156-
: // Extract and display the actual API error message from the response if available
157-
jsonResponse.data && typeof jsonResponse.data === 'object' && jsonResponse.data.error
152+
: jsonResponse.data && typeof jsonResponse.data === 'object' && jsonResponse.data.error
158153
? `HTTP error ${jsonResponse.status}: ${jsonResponse.data.error.message || JSON.stringify(jsonResponse.data.error)}`
159154
: jsonResponse.error || `HTTP error ${jsonResponse.status}`,
160155
}
161156
}
162-
}
163-
164-
// Standard response handling
165-
const headers: Record<string, string> = {}
166-
response.headers.forEach((value, key) => {
167-
headers[key] = value
168-
})
169157

170-
let data
171-
try {
172-
data = await (contentType.includes('application/json') ? response.json() : response.text())
173-
} catch (error) {
174-
// If response body reading fails, we can't retry reading - just use error message
175-
data = `Failed to parse response: ${error instanceof Error ? error.message : String(error)}`
158+
// Non-proxy JSON response: return parsed JSON directly
159+
return {
160+
success: response.ok,
161+
output: {
162+
data: jsonResponse ?? (await response.text()),
163+
status: response.status,
164+
headers,
165+
},
166+
error: response.ok ? undefined : `HTTP error ${response.status}: ${response.statusText}`,
167+
}
176168
}
177169

170+
// Non-JSON response: return text
171+
const textData = await response.text()
178172
return {
179173
success: response.ok,
180174
output: {
181-
data,
175+
data: textData,
182176
status: response.status,
183177
headers,
184178
},

apps/sim/tools/index.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,8 @@ export async function executeTool(
204204
}
205205
}
206206

207-
// For external APIs, use the proxy
207+
// For external APIs, always use the proxy POST, and ensure the tool request
208+
// builds a direct external URL (not the querystring proxy variant)
208209
const result = await handleProxyRequest(toolId, contextParams, executionContext)
209210

210211
// Apply post-processing if available and not skipped
@@ -399,9 +400,8 @@ async function handleInternalRequest(
399400

400401
const response = await fetch(fullUrl, requestOptions)
401402

402-
// Clone the response immediately before any body consumption
403+
// Clone the response for error checking while preserving original for transformResponse
403404
const responseForErrorCheck = response.clone()
404-
const responseForTransform = response.clone()
405405

406406
// Parse response data for error checking
407407
let responseData
@@ -469,7 +469,7 @@ async function handleInternalRequest(
469469
// Success case: use transformResponse if available
470470
if (tool.transformResponse) {
471471
try {
472-
const data = await tool.transformResponse(responseForTransform, params)
472+
const data = await tool.transformResponse(response, params)
473473
return data
474474
} catch (transformError) {
475475
logger.error(`[${requestId}] Transform response error for ${toolId}:`, {

apps/sim/tools/utils.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,17 @@ export function formatRequestParams(tool: ToolConfig, params: Record<string, any
4545
// Process URL
4646
const url = typeof tool.request.url === 'function' ? tool.request.url(params) : tool.request.url
4747

48-
// Process method
49-
const method = params.method || tool.request.method || 'GET'
48+
// Process method (support function or string on tool.request.method)
49+
const methodFromTool =
50+
typeof tool.request.method === 'function' ? tool.request.method(params) : tool.request.method
51+
const method = (params.method || methodFromTool || 'GET').toUpperCase()
5052

5153
// Process headers
5254
const headers = tool.request.headers ? tool.request.headers(params) : {}
5355

5456
// Process body
55-
const hasBody = method !== 'GET' && method !== 'HEAD' && !!tool.request.body
5657
const bodyResult = tool.request.body ? tool.request.body(params) : undefined
58+
const hasBody = method !== 'GET' && method !== 'HEAD' && bodyResult !== undefined
5759

5860
// Special handling for NDJSON content type or 'application/x-www-form-urlencoded'
5961
const isPreformattedContent =

0 commit comments

Comments
 (0)