fix(api): fix api post and get without stringifying (#955)

waleedlatif1 · web-flow · commit d4f412af92ef · 2025-08-13T18:49:22.000-05:00
diff --git a/apps/sim/app/api/proxy/route.ts b/apps/sim/app/api/proxy/route.ts
@@ -167,15 +167,14 @@ export async function POST(request: Request) {
 
   try {
     // Parse request body
-    const requestText = await request.text()
     let requestBody
     try {
-      requestBody = JSON.parse(requestText)
+      requestBody = await request.json()
     } catch (parseError) {
-      logger.error(`[${requestId}] Failed to parse request body: ${requestText}`, {
+      logger.error(`[${requestId}] Failed to parse request body`, {
         error: parseError instanceof Error ? parseError.message : String(parseError),
       })
-      throw new Error(`Invalid JSON in request body: ${requestText}`)
+      throw new Error('Invalid JSON in request body')
     }
 
     const { toolId, params, executionContext } = requestBody
diff --git a/apps/sim/tools/__test-utils__/test-tools.ts b/apps/sim/tools/__test-utils__/test-tools.ts
@@ -40,37 +40,22 @@ export function createMockFetch(
 ) {
   const { ok = true, status = 200, headers = { 'Content-Type': 'application/json' } } = options
 
-  // Normalize header keys to lowercase for case-insensitive access
-  const normalizedHeaders: Record<string, string> = {}
-  Object.entries(headers).forEach(([key, value]) => (normalizedHeaders[key.toLowerCase()] = value))
-
-  const makeResponse = () => {
-    const jsonMock = vi.fn().mockResolvedValue(responseData)
-    const textMock = vi
+  const mockFn = vi.fn().mockResolvedValue({
+    ok,
+    status,
+    headers: {
+      get: (key: string) => headers[key.toLowerCase()],
+      forEach: (callback: (value: string, key: string) => void) => {
+        Object.entries(headers).forEach(([key, value]) => callback(value, key))
+      },
+    },
+    json: vi.fn().mockResolvedValue(responseData),
+    text: vi
       .fn()
       .mockResolvedValue(
         typeof responseData === 'string' ? responseData : JSON.stringify(responseData)
-      )
-
-    const res: any = {
-      ok,
-      status,
-      headers: {
-        get: (key: string) => normalizedHeaders[key.toLowerCase()],
-        forEach: (callback: (value: string, key: string) => void) => {
-          Object.entries(normalizedHeaders).forEach(([key, value]) => callback(value, key))
-        },
-      },
-      json: jsonMock,
-      text: textMock,
-    }
-
-    // Implement clone() so production code that clones responses keeps working in tests
-    res.clone = vi.fn().mockImplementation(() => makeResponse())
-    return res
-  }
-
-  const mockFn = vi.fn().mockResolvedValue(makeResponse())
+      ),
+  })
 
   // Add preconnect property to satisfy TypeScript
 
diff --git a/apps/sim/tools/http/request.ts b/apps/sim/tools/http/request.ts
@@ -1,5 +1,5 @@
 import type { RequestParams, RequestResponse } from '@/tools/http/types'
-import { getDefaultHeaders, processUrl, shouldUseProxy, transformTable } from '@/tools/http/utils'
+import { getDefaultHeaders, processUrl, transformTable } from '@/tools/http/utils'
 import type { ToolConfig } from '@/tools/types'
 
 export const requestTool: ToolConfig<RequestParams, RequestResponse> = {
@@ -53,34 +53,18 @@ export const requestTool: ToolConfig<RequestParams, RequestResponse> = {
 
   request: {
     url: (params: RequestParams) => {
-      // Process the URL first to handle path/query params
-      const processedUrl = processUrl(params.url, params.pathParams, params.params)
-
-      // For external URLs that need proxying in the browser, we still return the
-      // external URL here and let executeTool route through the POST /api/proxy
-      // endpoint uniformly. This avoids querystring body encoding and prevents
-      // the proxy GET route from being hit from the client.
-      if (shouldUseProxy(processedUrl)) {
-        return processedUrl
-      }
-
-      return processedUrl
+      // Process the URL once and cache the result
+      return processUrl(params.url, params.pathParams, params.params)
     },
 
-    method: (params: RequestParams) => params.method || 'GET',
+    method: (params: RequestParams) => {
+      // Always return the user's intended method - executeTool handles proxy routing
+      return params.method || 'GET'
+    },
 
     headers: (params: RequestParams) => {
       const headers = transformTable(params.headers || null)
       const processedUrl = processUrl(params.url, params.pathParams, params.params)
-
-      // For proxied requests, we only need minimal headers
-      if (shouldUseProxy(processedUrl)) {
-        return {
-          'Content-Type': 'application/json',
-        }
-      }
-
-      // For direct requests, add all our standard headers
       const allHeaders = getDefaultHeaders(headers, processedUrl)
 
       // Set appropriate Content-Type
@@ -96,13 +80,6 @@ export const requestTool: ToolConfig<RequestParams, RequestResponse> = {
     },
 
     body: (params: RequestParams) => {
-      const processedUrl = processUrl(params.url, params.pathParams, params.params)
-
-      // For proxied requests, we don't need a body
-      if (shouldUseProxy(processedUrl)) {
-        return undefined
-      }
-
       if (params.formData) {
         const formData = new FormData()
         Object.entries(params.formData).forEach(([key, value]) => {
@@ -120,63 +97,46 @@ export const requestTool: ToolConfig<RequestParams, RequestResponse> = {
   },
 
   transformResponse: async (response: Response) => {
-    // Build headers once for consistent return structures
+    const contentType = response.headers.get('content-type') || ''
+
+    // Standard response handling
     const headers: Record<string, string> = {}
     response.headers.forEach((value, key) => {
       headers[key] = value
     })
 
-    const contentType = response.headers.get('content-type') || ''
-    const isJson = contentType.includes('application/json')
-
-    if (isJson) {
-      // Use a clone to safely inspect JSON without consuming the original body
-      let jsonResponse: any
-      try {
-        jsonResponse = await response.clone().json()
-      } catch (_e) {
-        jsonResponse = undefined
-      }
-
-      // Proxy responses wrap the real payload
-      if (jsonResponse && jsonResponse.data !== undefined && jsonResponse.status !== undefined) {
-        return {
-          success: jsonResponse.success,
-          output: {
-            data: jsonResponse.data,
-            status: jsonResponse.status,
-            headers: jsonResponse.headers || {},
-          },
-          error: jsonResponse.success
-            ? undefined
-            : jsonResponse.data && typeof jsonResponse.data === 'object' && jsonResponse.data.error
-              ? `HTTP error ${jsonResponse.status}: ${jsonResponse.data.error.message || JSON.stringify(jsonResponse.data.error)}`
-              : jsonResponse.error || `HTTP error ${jsonResponse.status}`,
-        }
-      }
-
-      // Non-proxy JSON response: return parsed JSON directly
+    const data = await (contentType.includes('application/json')
+      ? response.json()
+      : response.text())
+
+    // Check if this is a proxy response (structured response from /api/proxy)
+    if (
+      contentType.includes('application/json') &&
+      typeof data === 'object' &&
+      data !== null &&
+      data.data !== undefined &&
+      data.status !== undefined
+    ) {
       return {
-        success: response.ok,
+        success: data.success,
         output: {
-          data: jsonResponse ?? (await response.text()),
-          status: response.status,
-          headers,
+          data: data.data,
+          status: data.status,
+          headers: data.headers || {},
         },
-        error: response.ok ? undefined : `HTTP error ${response.status}: ${response.statusText}`,
+        error: data.success ? undefined : data.error,
       }
     }
 
-    // Non-JSON response: return text
-    const textData = await response.text()
+    // Direct response handling
     return {
       success: response.ok,
       output: {
-        data: textData,
+        data,
         status: response.status,
         headers,
       },
-      error: response.ok ? undefined : `HTTP error ${response.status}: ${response.statusText}`,
+      error: undefined, // Errors are handled upstream in executeTool
     }
   },
 
diff --git a/apps/sim/tools/index.ts b/apps/sim/tools/index.ts
@@ -200,8 +200,7 @@ export async function executeTool(
       }
     }
 
-    // For external APIs, always use the proxy POST, and ensure the tool request
-    // builds a direct external URL (not the querystring proxy variant)
+    // For external APIs, use the proxy
     const result = await handleProxyRequest(toolId, contextParams, executionContext)
 
     // Apply post-processing if available and not skipped
@@ -396,13 +395,10 @@ async function handleInternalRequest(
 
     const response = await fetch(fullUrl, requestOptions)
 
-    // Clone the response for error checking while preserving original for transformResponse
-    const responseForErrorCheck = response.clone()
-
-    // Parse response data for error checking
+    // Parse response data once
     let responseData
     try {
-      responseData = await responseForErrorCheck.json()
+      responseData = await response.json()
     } catch (jsonError) {
       logger.error(`[${requestId}] JSON parse error for ${toolId}:`, {
         error: jsonError instanceof Error ? jsonError.message : String(jsonError),
@@ -411,7 +407,7 @@ async function handleInternalRequest(
     }
 
     // Check for error conditions
-    const { isError, errorInfo } = isErrorResponse(responseForErrorCheck, responseData)
+    const { isError, errorInfo } = isErrorResponse(response, responseData)
 
     if (isError) {
       // Handle error case
@@ -465,7 +461,18 @@ async function handleInternalRequest(
     // Success case: use transformResponse if available
     if (tool.transformResponse) {
       try {
-        const data = await tool.transformResponse(response, params)
+        // Create a mock response object that provides the methods transformResponse needs
+        const mockResponse = {
+          ok: response.ok,
+          status: response.status,
+          statusText: response.statusText,
+          headers: response.headers,
+          json: async () => responseData,
+          text: async () =>
+            typeof responseData === 'string' ? responseData : JSON.stringify(responseData),
+        } as Response
+
+        const data = await tool.transformResponse(mockResponse, params)
         return data
       } catch (transformError) {
         logger.error(`[${requestId}] Transform response error for ${toolId}:`, {
diff --git a/apps/sim/tools/utils.ts b/apps/sim/tools/utils.ts
@@ -45,17 +45,18 @@ export function formatRequestParams(tool: ToolConfig, params: Record<string, any
   // Process URL
   const url = typeof tool.request.url === 'function' ? tool.request.url(params) : tool.request.url
 
-  // Process method (support function or string on tool.request.method)
-  const methodFromTool =
-    typeof tool.request.method === 'function' ? tool.request.method(params) : tool.request.method
-  const method = (params.method || methodFromTool || 'GET').toUpperCase()
+  // Process method
+  const method =
+    typeof tool.request.method === 'function'
+      ? tool.request.method(params)
+      : params.method || tool.request.method || 'GET'
 
   // Process headers
   const headers = tool.request.headers ? tool.request.headers(params) : {}
 
   // Process body
+  const hasBody = method !== 'GET' && method !== 'HEAD' && !!tool.request.body
   const bodyResult = tool.request.body ? tool.request.body(params) : undefined
-  const hasBody = method !== 'GET' && method !== 'HEAD' && bodyResult !== undefined
 
   // Special handling for NDJSON content type or 'application/x-www-form-urlencoded'
   const isPreformattedContent =
