Conversation
The `race` condition directive was broken due to a strict dependency on `threads > 0` for parallel execution, causing templates with `race` directive enabled but no explicit threads to fall back to seq execution. This regression was introduced in v3.2.0 (#4868), which restricted parallel execution to only when `payloads` were present. Fixes #5713 to allow race conditions even w/o explicit `payloads`, and add a default thread count when race is enabled but threads is 0. Signed-off-by: Dwi Siswanto <git@dw1.io>
Signed-off-by: Dwi Siswanto <git@dw1.io>
WalkthroughFixes HTTP race condition execution by adding the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
integration_tests/protocols/http/race-condition-with-delay.yamlis excluded by!**/*.yaml
📒 Files selected for processing (2)
cmd/integration-test/http.gopkg/protocols/http/request.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Format Go code usinggo fmt ./...
Run static analysis usinggo vet ./...on Go code
Files:
pkg/protocols/http/request.gocmd/integration-test/http.go
pkg/protocols/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
pkg/protocols/**/*.go: Each protocol implementation should implement the Request interface with Compile(), ExecuteWithResults(), Match(), and Extract() methods
Protocol implementations should embed Operators for matching/extraction functionality
Files:
pkg/protocols/http/request.go
⏰ 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). (1)
- GitHub Check: Lint
🔇 Additional comments (3)
pkg/protocols/http/request.go (1)
504-506: LGTM! Fix correctly restores race condition parallel execution.The added
|| request.Racecondition allows race templates withthreads > 0but no explicit payloads to enter parallel execution mode, addressing the regression introduced in v3.2.0.cmd/integration-test/http.go (2)
14-14: LGTM!The
syncimport is necessary for the mutex used in the new race delay test.
66-66: LGTM!Test case registration follows the existing pattern and is appropriately placed alongside other race condition tests.
Proposed changes
fix(http): race condition regression
The
racecondition directive was broken due toa strict dependency on
threads > 0for parallelexecution, causing templates with
racedirectiveenabled but no explicit threads to fall back to
seq execution.
This regression was introduced in v3.2.0 (#4868),
which restricted parallel execution to only when
payloadswere present.Fixes #5713 to allow race conditions even w/o
explicit
payloads, and add a default threadcount when race is enabled but threads is 0.
Proof
Server:
# tty1 $ python3 repro_server.pytty2 stdio:
Checklist
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.