Skip to content

Commit 6fe2713

Browse files
bricefclaude
andcommitted
Fix data race in webhook dispatcher tests
The capturingHandler wrote to shared body/headers variables from the HTTP server goroutine while the test goroutine read them after a time.Sleep — a classic data race. Replace with channel synchronisation: the handler signals completion, the test waits on the channel before reading captured data. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent a478a1f commit 6fe2713

1 file changed

Lines changed: 27 additions & 23 deletions

File tree

internal/webhook/dispatcher_test.go

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -78,16 +78,23 @@ func countingHandler(counter *int32, status int) http.HandlerFunc {
7878
}
7979
}
8080

81-
// capturingHandler returns an HTTP handler that captures the request body and headers.
82-
func capturingHandler(body *[]byte, headers *http.Header) http.HandlerFunc {
83-
var mu sync.Mutex
84-
return func(w http.ResponseWriter, r *http.Request) {
85-
mu.Lock()
86-
defer mu.Unlock()
87-
*body, _ = io.ReadAll(r.Body)
88-
*headers = r.Header.Clone()
81+
// capturingHandler returns an HTTP handler that captures the request body and
82+
// headers, and a channel that is sent to after the capture is complete.
83+
// The test must receive from the channel before reading body/headers.
84+
func capturingHandler() (http.HandlerFunc, *[]byte, *http.Header, <-chan struct{}) {
85+
var body []byte
86+
var headers http.Header
87+
done := make(chan struct{}, 1)
88+
handler := func(w http.ResponseWriter, r *http.Request) {
89+
body, _ = io.ReadAll(r.Body)
90+
headers = r.Header.Clone()
8991
w.WriteHeader(200)
92+
select {
93+
case done <- struct{}{}:
94+
default:
95+
}
9096
}
97+
return handler, &body, &headers, done
9198
}
9299

93100
// failThenSucceedHandler returns 500 for the first n calls, then 200.
@@ -121,24 +128,23 @@ const retryWait = 500 * time.Millisecond
121128

122129
func TestDeliversMatchingEvent(t *testing.T) {
123130
// Arrange
124-
var body []byte
125-
var headers http.Header
126-
srv := httptest.NewServer(capturingHandler(&body, &headers))
131+
handler, body, _, done := capturingHandler()
132+
srv := httptest.NewServer(handler)
127133
defer srv.Close()
128134

129135
logger := &mockLogger{}
130136
bus, _ := newTestDispatcher(t, []model.Webhook{testWebhook(srv.URL, "secret", []string{"task.created"})}, logger)
131137

132138
// Act
133139
bus.Publish(testEvent("task.created"))
134-
time.Sleep(eventWait)
140+
<-done
135141

136142
// Assert — payload is valid JSON with correct event type.
137-
if len(body) == 0 {
143+
if len(*body) == 0 {
138144
t.Fatal("expected delivery, got none")
139145
}
140146
var evt eventbus.Event
141-
if err := json.Unmarshal(body, &evt); err != nil {
147+
if err := json.Unmarshal(*body, &evt); err != nil {
142148
t.Fatalf("invalid JSON payload: %v", err)
143149
}
144150
if evt.Type != "task.created" {
@@ -148,16 +154,15 @@ func TestDeliversMatchingEvent(t *testing.T) {
148154

149155
func TestDeliveryHeaders(t *testing.T) {
150156
// Arrange
151-
var body []byte
152-
var headers http.Header
153-
srv := httptest.NewServer(capturingHandler(&body, &headers))
157+
handler, _, headers, done := capturingHandler()
158+
srv := httptest.NewServer(handler)
154159
defer srv.Close()
155160

156161
bus, _ := newTestDispatcher(t, []model.Webhook{testWebhook(srv.URL, "secret", []string{"task.created"})}, nil)
157162

158163
// Act
159164
bus.Publish(testEvent("task.created"))
160-
time.Sleep(eventWait)
165+
<-done
161166

162167
// Assert
163168
if ct := headers.Get("Content-Type"); ct != "application/json" {
@@ -173,21 +178,20 @@ func TestDeliveryHeaders(t *testing.T) {
173178

174179
func TestDeliverySignature(t *testing.T) {
175180
// Arrange
176-
var body []byte
177-
var headers http.Header
178181
secret := "test-secret"
179-
srv := httptest.NewServer(capturingHandler(&body, &headers))
182+
handler, body, headers, done := capturingHandler()
183+
srv := httptest.NewServer(handler)
180184
defer srv.Close()
181185

182186
bus, _ := newTestDispatcher(t, []model.Webhook{testWebhook(srv.URL, secret, []string{"task.created"})}, nil)
183187

184188
// Act
185189
bus.Publish(testEvent("task.created"))
186-
time.Sleep(eventWait)
190+
<-done
187191

188192
// Assert — receiver can verify the HMAC.
189193
mac := hmac.New(sha256.New, []byte(secret))
190-
mac.Write(body)
194+
mac.Write(*body)
191195
expected := fmt.Sprintf("sha256=%s", hex.EncodeToString(mac.Sum(nil)))
192196
if got := headers.Get(signatureHeader); got != expected {
193197
t.Errorf("signature mismatch:\n got: %s\n want: %s", got, expected)

0 commit comments

Comments
 (0)