-
Couldn't load subscription status.
- Fork 241
Description
🐛 Describe the Bug
The StreamableClientTransport breaks Go's standard context propagation pattern. Inside its Connect method, it creates a new, detached context with context.Background() instead of deriving from the parent context provided by the caller.
This oversight prevents request-scoped values—such as authentication tokens, tracing IDs, and other middleware-injected data—from being propagated to background HTTP operations managed by the transport, specifically the SSE (Server-Sent Events) connection and the final DELETE request on close.
Reproduce the Behavior
- Create a parent context containing a value:
ctx := context.WithValue(context.Background(), "my-key", "my-value")- Initialize and connect the client using this context:
client.Connect(ctx, streamableTransport, nil)- In a middleware or handler, attempt to access "my-key" from the context within the HTTP requests initiated by
StreamableClientTransport. - Observe that the value is missing because the original context was discarded.
Root Cause Analysis
The issue stems from three distinct violations of context handling within streamable.go.
Violation 1: Detached Context Creation
The context chain is broken at the very beginning of the connection process. A new context is created from context.Background(), discarding the parent ctx and all its associated values and deadlines.
File: streamable.go:1021
-- connCtx, cancel := context.WithCancel(context.Background()) // ❌ BREAKS CONTEXT CHAIN
++ connCtx, cancel := context.WithCancel(ctx) // ✅ PRESERVES CONTEXT CHAINViolation 2: SSE Requests Use the Detached Context
The establishSSE method correctly uses http.NewRequestWithContext, but it receives the detached c.ctx created in Violation #1, which lacks the necessary request-scoped values.
File: streamable.go:1422
// This code is functionally correct, but c.ctx is the wrong context.
req, err := http.NewRequestWithContext(c.ctx, http.MethodGet, c.url, nil)Violation 3: DELETE Request Ignores Context Entirely
The Close method creates a DELETE request using http.NewRequest instead of its context-aware counterpart, http.NewRequestWithContext. This means it can't be cancelled and carries no scoped data.
File: streamable.go:1404
-- req, err := http.NewRequest(http.MethodDelete, c.url, nil) // ❌ NO CONTEXT
++ req, err := http.NewRequestWithContext(ctx, http.MethodDelete, c.url, nil) // ✅ USES CONTEXTExpected Behavior
All background HTTP operations initiated by the client transport should honor the context provided during the Connect call. This ensures that deadlines, cancellation signals, and request-scoped values are propagated correctly, adhering to standard Go patterns.
💥 Impact
This bug can silently break critical application functionality that relies on context propagation, including:
- Authentication: Middleware-injected auth tokens won't be sent on background requests.
- Distributed Tracing: Trace and span IDs are lost, breaking request correlation in observability platforms.
- Request Scoping: Any request-specific data (e.g., user IDs, tenant info) becomes unavailable.
- Cancellation: Operations cannot be properly cancelled via the parent context.
✅ Proposed Solution
The fix involves correcting the three violations identified above to ensure the parent context is respected throughout the connection's lifecycle.
- Preserve the parent context in Connect (line 1021):
// streamable.go:1021
connCtx, cancel := context.WithCancel(ctx)-
No change is needed for SSE requests (line 1422), as fixing README: remove toc, update target date #1 ensures c.ctx is the correct, derived context.
-
Pass the context to the DELETE request in Close (line 1404). Note: The original ctx from Connect must be stored on the struct to be accessible here.
// streamable.go:1404
req, err := http.NewRequestWithContext(c.ctx, http.MethodDelete, c.url, nil)Additional Context
This behavior is particularly subtle because standard RPC-style calls on the client work as expected—they accept a context at call time. The bug is confined to the long-lived, client-initiated background operations (GET for SSE and DELETE for cleanup) that incorrectly manage the context provided at initialization.