Add ctx propagation to raw api request #570
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds context propagation to the low-level newRequest helper, ensuring that trace and cancellation contexts flow through HTTP calls in the raw API client.
- Extended
newRequestto accept acontext.Contextand attach it viaWithContext - Updated
GetRawContentto pass the incomingctxintonewRequest - Adjusted error handling in
newRequestto return early on failure before setting context
Comments suppressed due to low confidence (3)
pkg/raw/raw.go:65
- Add or update unit tests to verify that the provided context is actually attached to the outgoing
http.Requestand that cancellations/timeouts are honored.
func (c *Client) GetRawContent(ctx context.Context, owner, repo, path string, opts *RawContentOpts) (*http.Response, error) {
pkg/raw/raw.go:28
- Make sure to import the "context" package at the top of this file, otherwise
context.Contextwill be undefined.
func (c *Client) newRequest(ctx context.Context, method string, urlStr string, body interface{}, opts ...gogithub.RequestOption) (*http.Request, error) {
pkg/raw/raw.go:67
- Review other methods in this file (or in downstream code) that call
newRequestto ensure they’ve been updated to passctxas the first argument, preventing build failures and ensuring consistent context propagation.
req, err := c.newRequest(ctx, "GET", url, nil)
I did test this locally and confirmed that ctx propagation is working as expected, trace info and parent-child spans are all showing up correctly. I'm a bit confused about the suggested alternative. Since we are creating a new outbound request here (not working with an incoming HTTP request), I don't think there's an existing req ctx to extend? Let me know if I'm missing something. |
Update
newRequestto accept acontext.Contextparam and attach it to the generatedhttp.RequestusingwithContext.This change ensures that trace contexts are correctly propagated, supporting distributed tracing in the remote MCP server.