Better attempt at an appengine wrapper.

go/src/infra/gae/libs/wrapper/featurebreaker.go

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package wrapper
+
+import (
+        "runtime"
+        "strings"
+        "sync"
+)
+
+// FeatureBreaker allows a fake implementation to set and unset broken features.
+// A feature is the Name of some method on the fake. So if you had:
+//   var fake interface{ FeatureBreaker, MCSingleReadWriter } = ...
+//
+// you could do:
+//   fake.SetBrokenFeatures("Add", "Set")
+//
+// and then
+//   fake.Add(...) and fake.Set(...)
+//
+// would return an error (likely memcache.ErrServerError in this case), as if
+// the service were disconnected or broken.
+type FeatureBreaker interface {
+        SetBrokenFeatures(feature ...string)
+        UnsetBrokenFeatures(feature ...string)

[M-A Ruel, 2015/05/25 17:14:51]

UnbreakFeatures() ?

[iannucci, 2015/05/26 18:25:06]

And BreakFeatures? sgtm.

+}
+
+// BrokenFeatures implements the FeatureBreaker interface, and is suitable for
+// embedding within a fake service.
+type BrokenFeatures struct {
+        lock sync.Mutex
+
+        broken map[string]struct{}

[M-A Ruel, 2015/05/25 17:14:51]

broken map[string]bool

or much better:

broken map[string]error

and remove member err. This mean changing the interface above.

[iannucci, 2015/05/26 18:25:06]

I thought map[string]struct{} was better because sizeof(struct{}) == 0. That
said, I like map[string]error, as it was something I was planning on changing
this to eventually. I just thought of a way to make it useful right now though
(see new interface and commonErrors package).

+        err    error
+}
+
+// NewBrokenFeatures creates an embeddable *BrokenFeatures which is set to
+// return err for a given broken feature.
+func NewBrokenFeatures(err error) *BrokenFeatures {
+        return &BrokenFeatures{broken: map[string]struct{}{}, err: err}
+}
+
+// SetBrokenFeatures allows you to specify an MCSingleReadWriter function name
+// to cause it to return memcache.ErrServerError. e.g.
+//
+//   m.SetBrokenFeatures("Add")
+//
+// would return memcache.ErrServerError. You can reverse this by calling
+// UnsetBrokenFeatures("Add").
+func (b *BrokenFeatures) SetBrokenFeatures(feature ...string) {
+        b.lock.Lock()
+        defer b.lock.Unlock()
+        for _, f := range feature {

[M-A Ruel, 2015/05/25 17:14:51]

it'd be good to add a check:
if b.broken == nil {
  b.broken = map[string]error{}
}

this way you don't need NewBrokenFeatures(). The main reason is that you also
export BrokenFeatures, so the odd of making an error and forgetting to
initialize the embedded struct is high.

[iannucci, 2015/05/26 18:25:06]

good point, added initLocked().

+                b.broken[f] = struct{}{}

[M-A Ruel, 2015/05/25 17:14:51]

b.broken[f] = true

is faster.

[iannucci, 2015/05/26 18:25:06]

did the error map.

Also, do you have benchmarks handy? I was under the impression that struct{}{}
was faster and smaller, though we chatted and you pointed out that the struct{}
might need to be addressable (and thus: allocated), which would definitely be
bad.

+        }
+}
+
+// UnsetBrokenFeatures is the inverse of SetBrokenFeatures.
+func (b *BrokenFeatures) UnsetBrokenFeatures(feature ...string) {
+        b.lock.Lock()
+        defer b.lock.Unlock()
+        for _, f := range feature {
+                delete(b.broken, f)
+        }
+}
+
+// IsBroken is to be called internally by the fake service on every
+// publically-facing method. If it returns an error, the fake should return
+// the error.
+//
+// Example:
+//   type MyService struct { *BrokenFeatures }
+//   func (ms *MyService) Thingy() error {
+//     if err := ms.IsBroken(); err != nil {
+//       return err
+//     }
+//               ...
+//   }
+//
+//  You can now do ms.SetBrokenFeatures("Thingy"), and Thingy will return an
+//  error.
+func (b *BrokenFeatures) IsBroken() error {
+        b.lock.Lock()
+        defer b.lock.Unlock()
+        if len(b.broken) == 0 {
+                return nil
+        }
+        off := 1
+
+        for {
+                fn, _, _, _ := runtime.Caller(off)
+                name := runtime.FuncForPC(fn).Name()
+                toks := strings.Split(name, ".")

[M-A Ruel, 2015/05/25 17:14:51]

toks := strings.Split(runtime.FuncForPC(fn).Name(), ".")

otherwise line 96 is confusing because you reuse name for two different
purposes.

[iannucci, 2015/05/26 18:25:06]

ew gross, good catch.

+                name = toks[len(toks)-1]
+                if name == "IsBroken" {
+                        // Allow users to override IsBroken

[M-A Ruel, 2015/05/25 17:14:51]

This should be in docstring then.

[iannucci, 2015/05/26 18:25:06]

done

+                        // ITS MAGIC!!!!!
+                        off++
+                        continue
+                }
+                if _, ok := b.broken[name]; ok {

[M-A Ruel, 2015/05/25 17:14:51]

I don't like that it's not checking the package. It's easy to get a false
positive.

[iannucci, 2015/05/26 18:25:06]

Discussed in chat, it's not exactly clear what to check the package against, and
I think it's OK to ignore the package because this struct is all about checking
the /interface/, which is agnostic to the package.

+                        return b.err
+                }
+                break
+        }
+
+        return nil
+}