fix(sdk): dual bearer send via httpx #3373
Conversation
WalkthroughThe change updates the Authorization header handling in wait_for_result within packages/traceloop-sdk/traceloop/sdk/evaluator/stream_client.py, removing the hardcoded "Bearer " prefix and passing through the header value from the underlying client unchanged. No other logic or public interfaces are modified. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant StreamClient as Evaluator StreamClient
participant Service as Remote Service
Caller->>StreamClient: wait_for_result(task_id)
Note right of StreamClient: Build URL and headers
StreamClient->>Service: GET /result/{task_id}\nAuthorization: <client.headers['Authorization']>
alt Success
Service-->>StreamClient: 200 OK (streamed/result)
StreamClient-->>Caller: Return result
else Error
Service-->>StreamClient: Error status
StreamClient-->>Caller: Raise/propagate error
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
✨ Finishing touches
🧪 Generate unit tests
Comment |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to b3c06fa in 46 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/traceloop/sdk/client/client.py:57
- Draft comment:
Removed 'Bearer ' prefix from the Authorization header in AsyncClient. Ensure that the API endpoint now expects the raw API key and not a bearer token format. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_4mRUuYljxrYdNYOh
You can customize 
galzilber
changed the title
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed df20bca in 31 seconds. Click for details.
- Reviewed
26lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/traceloop/sdk/client/client.py:57
- Draft comment:
Good fix: Using 'Bearer {api_key}' directly ensures the header matches server expectations. Ensure that all clients consistently follow this format. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/traceloop-sdk/traceloop/sdk/evaluator/stream_client.py:24
- Draft comment:
Removing the redundant 'Bearer' prefix avoids sending 'Bearer Bearer ...' and fixes auth failures. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_piziuZbz6r1lZ9R7
You can customize
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/traceloop-sdk/traceloop/sdk/evaluator/stream_client.py (3)
29-29: Normalize URL join to avoid accidental double slashes and match existing pattern.Use the same pattern as your sync client (
/v2/{...}withlstrip('/')):- full_stream_url = f"{self.client.base_url}/v2{stream_url}" + full_stream_url = f"{self.client.base_url}/v2/{stream_url.lstrip('/')}"
35-35: SSE-friendly timeouts: avoid read timeout for long-lived streams.- timeout=httpx.Timeout(timeout_in_sec), + timeout=httpx.Timeout(connect=timeout_in_sec, read=None),
55-55: Add concrete type for response parameter.- async def _handle_sse_response(self, response) -> ExecutionResponse: + async def _handle_sse_response(self, response: httpx.Response) -> ExecutionResponse:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/traceloop-sdk/traceloop/sdk/evaluator/stream_client.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/traceloop-sdk/traceloop/sdk/evaluator/stream_client.py
🧬 Code graph analysis (1)
packages/traceloop-sdk/traceloop/sdk/evaluator/stream_client.py (1)
packages/traceloop-sdk/traceloop/sdk/client/http.py (1)
get(39-58)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Lint
- GitHub Check: Build Packages (3.11)
| headers = { | ||
| "Authorization": f"Bearer {self.client.headers.get('Authorization')}", | ||
| "Authorization": self.client.headers.get('Authorization'), | ||
| "Accept": "text/event-stream", | ||
| "Cache-Control": "no-cache", | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Don’t override Authorization; guard against None to avoid runtime error and rely on client auth.
If self.client was configured with auth=... (not a default header), self.client.headers.get('Authorization') will be None, and passing {"Authorization": None} can raise a TypeError in httpx. Also, you can let the client’s configured auth/header flow through without re-specifying it here, which avoids any “dual Bearer” regressions.
Preferred fix (remove explicit Authorization override and let the client send it if configured):
- headers = {
- "Authorization": self.client.headers.get('Authorization'),
- "Accept": "text/event-stream",
- "Cache-Control": "no-cache",
- }
+ headers = {
+ "Accept": "text/event-stream",
+ "Cache-Control": "no-cache",
+ }Minimal alternative (only add the header if present):
- headers = {
- "Authorization": self.client.headers.get('Authorization'),
- "Accept": "text/event-stream",
- "Cache-Control": "no-cache",
- }
+ _auth = self.client.headers.get('Authorization')
+ headers = {
+ "Accept": "text/event-stream",
+ "Cache-Control": "no-cache",
+ }
+ if _auth:
+ headers["Authorization"] = _authRun to confirm no other places still prepend "Bearer " explicitly:
🏁 Script executed:
#!/bin/bash
rg -nP -C2 --type=py '\bAuthorization\b.*Bearer'
rg -nP -C2 --type=py 'httpx\.(AsyncClient|Client)\s*\([^)]*\bheaders\s*='Length of output: 3600
Don't override Authorization — remove or guard against None and centralize auth in the client.
Passing Authorization: None can raise httpx errors and the repo already sets "Bearer ..." in multiple places; standardize on the client's auth/header. Found explicit "Bearer" usage in:
- packages/traceloop-sdk/traceloop/sdk/images/image_uploader.py
- packages/traceloop-sdk/traceloop/sdk/fetcher.py
- packages/traceloop-sdk/traceloop/sdk/init.py
- packages/traceloop-sdk/traceloop/sdk/client/http.py
- packages/traceloop-sdk/traceloop/sdk/client/client.py
Preferred fix (remove explicit Authorization override and let the client send it if configured):
- headers = {
- "Authorization": self.client.headers.get('Authorization'),
- "Accept": "text/event-stream",
- "Cache-Control": "no-cache",
- }
+ headers = {
+ "Accept": "text/event-stream",
+ "Cache-Control": "no-cache",
+ }Minimal alternative (only add the header if present):
- headers = {
- "Authorization": self.client.headers.get('Authorization'),
- "Accept": "text/event-stream",
- "Cache-Control": "no-cache",
- }
+ _auth = self.client.headers.get('Authorization')
+ headers = {
+ "Accept": "text/event-stream",
+ "Cache-Control": "no-cache",
+ }
+ if _auth:
+ headers["Authorization"] = _auth📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| headers = { | |
| "Authorization": f"Bearer {self.client.headers.get('Authorization')}", | |
| "Authorization": self.client.headers.get('Authorization'), | |
| "Accept": "text/event-stream", | |
| "Cache-Control": "no-cache", | |
| } | |
| headers = { | |
| "Accept": "text/event-stream", | |
| "Cache-Control": "no-cache", | |
| } |
🤖 Prompt for AI Agents
In packages/traceloop-sdk/traceloop/sdk/evaluator/stream_client.py around lines
23 to 27, the code overwrites the Authorization header with a possible None
value which can cause httpx errors and duplicates auth handling; remove the
explicit "Authorization" entry from the headers dict so the configured
client-level auth/headers are used, or if you prefer the minimal change only set
Authorization when self.client.headers.get('Authorization') is truthy (guard
against None); centralize authorization in the shared client implementation
instead of overriding it here.
3 participants
feat(instrumentation): ...orfix(instrumentation): ....Important
Fixes Authorization header in
wait_for_resultofstream_client.pyto resolve unauthorized errors due to dual bearer tokens.wait_for_resultfunction ofstream_client.pyto prevent dual bearer token issue causing unauthorized errors.This description was created by
for df20bca. You can customize this summary. It will automatically update as commits are pushed.
at the current state the api service receiving duel bearer
so the request been failed due unauthorized
Summary by CodeRabbit