qstash-review

Code review checklist for qstash-stress with project-specific patterns.

Review Checklist

1. Thread Safety

Tracker access:

• • Use atomic.AddInt64/LoadInt64 for counters
• • Lock TrackingInfo.mu before accessing fields
• • Use sync.Map methods (Load, Store, Range) correctly

Metrics access:

• • Lock m.mu before modifying samples
• • Use RLock for read-only operations

Behaviors access:

• • Lock b.mu before accessing/modifying config

Example - Correct:

info.mu.Lock()
info.Status = "delivered"
info.mu.Unlock()
atomic.AddInt64(&t.totalDelivered, 1)

Example - Wrong:

info.Status = "delivered"  // No lock!
t.totalDelivered++          // Not atomic!

2. Error Handling

HTTP responses:

• • Call drainAndClose(resp.Body) to enable connection reuse
• • Check io.ReadAll errors

JSON operations:

• • Check json.Marshal and json.Unmarshal errors
• • Handle parse errors gracefully

Example - Correct:

defer drainAndClose(resp.Body)
body, err := io.ReadAll(resp.Body)
if err != nil {
    return fmt.Errorf("read body: %w", err)
}

3. QStash Header Compliance

When adding new headers, verify:

• • Header name follows Upstash-* convention
• • Value format matches QStash API spec
• • Added to PublishOptions struct
• • Set in Publish() method
• • Test case added in appropriate suite

Reference: ../qstash/features/*.mdx for header specifications.

4. Memory Bounds

• • New collections have size limits
• • Long-running operations use reservoir sampling pattern
• • No unbounded slice growth in loops

Reservoir sampling pattern:

const maxSamples = 10000
if len(samples) < maxSamples {
    samples = append(samples, value)
} else {
    // Random replacement
    idx := rand.Intn(len(samples))
    samples[idx] = value
}

5. Test Coverage

For new features:

• • Unit test in *_test.go
• • Integration test case in test-suites/*.yaml
• • Test passes with go test -race

For bug fixes:

• • Regression test added
• • Related tests still pass

6. Code Style

• • No unused imports or variables
• • Error messages are lowercase (Go convention)
• • Context is propagated for cancellation
• • Timeouts are configurable

Common Issues to Flag

Data Race Potential

// BAD: Reading without lock
status := info.Status

// GOOD: Lock before read
info.mu.RLock()
status := info.Status
info.mu.RUnlock()

Resource Leak

// BAD: Body not drained
resp, _ := http.Get(url)
return resp.StatusCode

// GOOD: Drain and close
resp, _ := http.Get(url)
defer drainAndClose(resp.Body)
return resp.StatusCode

Missing Error Check

// BAD: Error ignored
body, _ := json.Marshal(data)

// GOOD: Error checked
body, err := json.Marshal(data)
if err != nil {
    return fmt.Errorf("marshal: %w", err)
}

File-Specific Patterns