Fix ReadableStream reuse in retries by fetching fresh body

[chitalian]

Ticket

N/A

Component/Service

• Worker (Proxy)

Type of Change

• Bug fix
• Performance improvement

Deployment Notes

• No special deployment steps required

Context

ReadableStream bodies can only be consumed once. When callProvider retries a request, reusing the same body reference causes a "Body has already been used" error on the second attempt. Additionally, fetching the body twice from a Remote buffer in HeliconeProxyRequestMapper leaks a ReadableStream from the container DO on every request.

Changes

1. ProviderClient.ts: Changed CallProps.body from a static ValidRequestBody to a getBody() factory function that returns a fresh body for each fetch attempt. This ensures retries get a new stream instead of reusing a consumed one.

2. ProviderClient.ts: Updated callProvider to call props.getBody() only when needed (non-GET requests), fetching a fresh body per attempt.

3. HeliconeProxyRequestMapper.ts: Removed the initial body fetch before applyTokenLimitExceptionHandler. The body is now only fetched once after the handler runs, and callers must use the getBody() factory instead of consuming the stored reference directly.

4. callPropsFromProxyRequest: Implemented the getBody() factory to safely fetch the body with fallback to text extraction.

Extra Notes

The body is retained on HeliconeProxyRequest for logging purposes but must not be consumed by callers—they must use the getBody() factory to ensure fresh streams on retries.

Test Plan

Existing tests should pass. The change is backward compatible at the call site since getBody() is invoked transparently within callProvider.

https://claude.ai/code/session_01FtHETTgsrBtPz9BLB3KmHh

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

3 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
helicone	Skipped		Apr 23, 2026 11:27pm
helicone-bifrost	Skipped		Apr 23, 2026 11:27pm
helicone-eu	Skipped		Apr 23, 2026 11:27pm

[supabase]

This pull request has been ignored for the connected project bolqqmqbrciybnypvklh because there are no changes detected in supbase/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[greptile-apps]

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

[supabase]

This pull request has been ignored for the connected project lmahfbbnchpworytrrqk because there are no changes detected in .supabase/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[claude]

Claude finished @chitalian's task —— View job

---

PR Review: ReadableStream Reuse Fix

Todo List:

• Read and analyze the modified ProviderClient.ts file
• Evaluate the technical approach for fixing ReadableStream reuse
• Check for potential side effects, edge cases, or breaking changes
• Assess code quality and adherence to best practices
• Provide detailed feedback with specific suggestions
• Assign confidence score and provide final summary

---

Review Analysis

✅ Technical Approach: Excellent

The core fix is architecturally sound:

1. Root Cause Properly Identified: ReadableStreams can only be consumed once, and the retry logic was reusing the same CallProps object across attempts.

2. Clean Factory Pattern: Replacing body: ValidRequestBody with getBody: () => Promise<ValidRequestBody> ensures each retry gets a fresh stream.

3. Proper Error Handling: The getBody() implementation correctly falls back from safelyGetBody() to unsafeGetBodyText().

✅ Code Quality: Good

Strengths:

• Clear, descriptive comments explaining the ReadableStream constraint (worker/src/lib/clients/ProviderClient.ts:13-15, 168-170)
• Consistent error handling pattern matching existing codebase conventions
• Minimal surface area change - only touches what's necessary

Performance Considerations:

• ✅ Smart Optimization: Body is only fetched for non-GET requests (worker/src/lib/clients/ProviderClient.ts:172-174)
• ✅ No Unnecessary Overhead: GET requests continue to work without body fetching

⚠️ Minor Suggestions

1. Consider Documenting the Contract (worker/src/lib/clients/ProviderClient.ts:28-34):

   getBody: async () => {
     try {
       return await proxyRequest.requestWrapper.safelyGetBody();
     } catch (e) {
       // Fallback for legacy prompt scenarios where safelyGetBody fails
       return await proxyRequest.requestWrapper.unsafeGetBodyText();
     }
   },

2. Potential Race Condition: Multiple concurrent calls to getBody() could result in multiple body fetches. Consider if caching within a single request cycle would be beneficial for performance.

✅ Backward Compatibility: Excellent

• No breaking API changes - CallProps interface change is internal
• Existing call sites continue to work unchanged
• Test compatibility maintained

✅ Security & Error Handling: Good

• Proper try/catch patterns maintained
• No new attack vectors introduced
• Error messages remain informative

Score: 8/10 - Excellent fix with solid engineering practices

Suggestions Summary:

• ✅ Ready to merge - this is a well-executed fix for a real production issue
• Consider adding a comment about the fallback behavior in getBody()
• Monitor for potential performance impact of multiple body fetches in high-retry scenarios (though this is unlikely to be significant)

Critical Issues: None found
Breaking Changes: None
Security Concerns: None

The fix correctly addresses the ReadableStream consumption issue while maintaining clean code patterns and backward compatibility. This is a production-ready solution.

[Eldeeqq]

Hi @chitalian - any estimate on when this one will be merged?