Skip to content

fix(webhook): propagate request context in batchTransformLoop - #6988

Closed
reki7o7 wants to merge 1 commit into
rudderlabs:masterfrom
reki7o7:fix-webhook-context-timeout
Closed

fix(webhook): propagate request context in batchTransformLoop#6988
reki7o7 wants to merge 1 commit into
rudderlabs:masterfrom
reki7o7:fix-webhook-context-timeout

Conversation

@reki7o7

@reki7o7 reki7o7 commented May 23, 2026

Copy link
Copy Markdown

This PR resolves #6787.

The previous implementation used a hardcoded timeout context when calling the source transformer adapter, which detached execution from the original request lifecycle and ignored gateway-level timeout/cancellation handling.
This change replaces that with the HTTP request context from the batch request so transformer calls respect the existing gateway timeout behavior.
Note: I was unable to fully run the package tests locally because the repository currently has Windows-specific build incompatibilities (syscall.Stat_t in utils/misc), but the change is minimal and scoped to the requested fix.This is a clean resubmission of the previously stale PR #6788 with the requested minimal change.

@contributor-support

Copy link
Copy Markdown

Thank you @reki7o7 for contributing this PR.
Please sign the Contributor License Agreement (CLA) before merging.

// If unable to fetch features from transformer, send GatewayTimeout to all requests
// TODO: Remove timeout from here after timeout handler is added in gateway
ctx, cancel := context.WithTimeout(context.Background(), config.GetDurationVar(10, time.Second, "WriteTimeout", "WriteTimeOutInSec"))
ctx := breq.batchRequest[0].request.Context()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn’t really improve things. In fact, it introduces a new bug by tying the webhook context to the first request’s context. Since a batch can contain multiple requests, if the client from the first request disconnects, all other clients in the batch will receive an error response.

Maybe we should just remove the TODO to avoid confusing people and agents.

@atzoum atzoum closed this May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore: resolve TODO by replacing manual context timeout in webhook batch processing

2 participants