Fix 14 bugs: panics, data races, incorrect output, and dead code cleanup

[cep21]

Summary

This PR fixes 14 bugs found through deep analysis of the circuit breaker library, ranging from runtime panics and data races to incorrect metric output and unbounded recursion. All fixes are backwards-compatible and include regression tests. Benchmarks show no performance regression.

19 files changed, 332 insertions, 56 deletions

Bug Fixes

Panics & Crashes

1. close(nil) panic in waitForErrors (gowrapper.go)
   When skipCatchPanics is true, panicResults is nil. The deferred close(panicResults) panicked. Added a nil guard.

2. SimpleBadRequest.Error() panics if Err is nil (errors.go)
   Calling .Error() on SimpleBadRequest{Err: nil} caused a nil pointer dereference. Now returns "bad request".

3. MetricEventStream.Close() panics on double-close (metriceventstream/metriceventstream.go)
   Closing the event stream twice panicked on close(m.closeChan). Guarded with sync.Once.

4. RollingPercentile.AddDuration panics on pre-start times (faststats/rolling_percentile.go)
   Adding a duration with a timestamp before StartTime produced a negative index, causing an out-of-bounds panic. Added idx < 0 guard.

Correctness

5. SortedDurations.Var() wrong percentile scale (faststats/rolling_percentile.go)
   Var() called Percentile(.25) etc, but Percentile() expects [0-100], not [0-1]. This made p25/p50/p90/p99 in expvar output nearly identical to the minimum value. Fixed callers to use 25, 50, 90, 99.

6. RollingBuckets.Advance backward-time guard was dead code (faststats/rolling_bucket.go)
   The condition indexDiff >= r.NumBuckets was always false when indexDiff < 0. Fixed to -indexDiff >= r.NumBuckets.

7. OpenCircuit used time.Now() instead of c.now() (circuit.go)
   OpenCircuit ignored the configured TimeKeeper.Now, breaking mock-time testing. Changed to c.now().

8. Double now() call at timeout boundary (circuit.go)
   Two separate now() calls (endTime and runFuncDoneTime) could straddle a timeout boundary, causing a spurious timeout even though the function completed in time. Collapsed to a single call.

9. Double read of MaxConcurrentRequests in throttle check (circuit.go)
   MaxConcurrentRequests.Get() was called twice in the same condition, allowing a TOCTOU race where the value could change between reads. Read once into a local variable.

10. MockClock.AfterFunc returned nil (internal/clock/clock.go)
    Callers calling .Stop() on the returned *time.Timer would panic. Now returns a stopped real timer.

Concurrency

11. Factory closures mutated shared captured config (closers/hystrix/closer.go, closers/hystrix/opener.go)
    CloserFactory and OpenerFactory called config.Merge() on the captured config value, mutating it across concurrent invocations. Fixed by copying config before merging.

12. RollingBuckets.Advance unbounded recursion under contention (faststats/rolling_bucket.go)
    Two recursive return r.Advance(now, clearBucket) calls could stack overflow under high concurrency. Converted to an iterative loop.

Code Quality (from review)

13. MockClock.AfterFunc deadlock when d==0 — called f() while holding m.mu. Released lock first.
14. Dead cancelled field in timedCallbacks — never set to true. Removed along with the check.
15. Dead test scaffolding — removed unused type declarations and variable assignments in TestOpenCircuitUsesMockTime.
16. Various minor fixes — import ordering, extra blank lines, comment prefixes, pointer receivers, documentation.

What is NOT in this PR

• Prevent() metric gap: When ClosedToOpen.Prevent() blocks a request, no metric is emitted. An earlier version of this PR emitted ErrShortCircuit, but this is semantically wrong — Prevent operates on a closed circuit, while ErrShortCircuit implies the circuit is open. Properly fixing this requires adding ErrPrevented to the RunMetrics interface, which is a breaking change for a future major version.

Testing

• All existing tests pass
• Each bug fix includes a new regression test that fails without the fix
• go test ./... -race passes with no races detected
• Benchmarks show no significant regression (zero-allocation profile unchanged)