Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR fixes critical "double-read" issues in the API block tool execution system that were preventing proper HTTP request handling across multiple components. The changes address fundamental problems with HTTP response body consumption in JavaScript/Node.js, where response streams can only be read once before becoming unavailable.
The core issue stemmed from the tool execution system attempting to read HTTP response bodies multiple times - once for error checking and again for response transformation. This caused "body already read" errors that broke API block functionality. The PR implements several coordinated fixes:
In tools/utils.ts, the formatRequestParams function now properly handles dynamic HTTP method determination by supporting both function and string types for tool.request.method. This ensures compatibility with tools like the HTTP request tool that determine methods dynamically. The body handling logic was also corrected to check actual body content rather than just tool configuration.
The tools/index.ts file received the most critical fix - restructuring response handling to use a single clone for error checking while preserving the original response object for transformation. This prevents multiple consumption of the same response stream.
In tools/http/request.ts, the proxy handling mechanism was significantly simplified by removing complex URL encoding logic and routing all external requests uniformly through the POST /api/proxy endpoint. The transformResponse function was enhanced with response.clone() to safely inspect JSON responses without consuming the original body.
The proxy API route (app/api/proxy/route.ts) was updated to prevent request body parsing issues by reading requests as text first before JSON parsing, avoiding stream consumption conflicts.
Finally, test utilities in tools/__test-utils__/test-tools.ts were enhanced with proper response cloning support and a factory pattern that creates fresh response objects for each clone, ensuring tests don't fail when production code clones responses.
These changes work together to create a robust HTTP request handling system that properly manages response stream consumption while maintaining backward compatibility with existing tool configurations.
Confidence score: 4/5
- This PR addresses critical production issues with well-coordinated fixes across multiple components
- Score reflects comprehensive testing considerations and solid technical approach to HTTP stream management
- Pay close attention to the response cloning logic in
tools/index.tsand proxy routing changes intools/http/request.ts
5 files reviewed, 1 comment
| return { | ||
| success: response.ok, | ||
| output: { | ||
| data: jsonResponse ?? (await response.text()), |
There was a problem hiding this comment.
logic: Potential issue: If response.clone().json() succeeds but response.text() fails, this could throw an error since the original response body was already consumed by clone().json().
| data: jsonResponse ?? (await response.text()), | |
| data: jsonResponse ?? (await response.clone().text()), |
2 participants
Summary
Brief description of what this PR does and why.
Fixes #(issue)
Type of Change
Testing
How has this been tested? What should reviewers focus on?
Checklist
Screenshots/Videos