Review comments

encoding/thrift/envelope.go

@@ -89,10 +89,10 @@ type disableEnvelopingNoWireReader struct {
 	Type wire.EnvelopeType
 }
 
-func (evr disableEnvelopingNoWireReader) ReadEnvelopeBegin() (stream.EnvelopeHeader, error) {
+func (evnwr disableEnvelopingNoWireReader) ReadEnvelopeBegin() (stream.EnvelopeHeader, error) {
 	return stream.EnvelopeHeader{
 		Name:  "", // we don't use the decoded name anywhere
-		Type:  evr.Type,
+		Type:  evnwr.Type,
 		SeqID: 1,
 	}, nil
 }

encoding/thrift/envelope_test.go

@@ -83,29 +83,8 @@ func TestDisableEnveloperEncode(t *testing.T) {
 	}
 }
 
-// generate generates a random value into the given pointer.
-//
-// 	var i int
-// 	generate(&i, rand)
-//
-// If the type implements the quick.Generator interface, that is used.
-func generate(v interface{}, r *rand.Rand) {
-	t := reflect.TypeOf(v)
-	if t.Kind() != reflect.Ptr {
-		panic(fmt.Sprintf("%v is not a pointer type", t))
-	}
-
-	out, ok := quick.Value(t.Elem(), r)
-	if !ok {
-		panic(fmt.Sprintf("could not generate a value for %v", t))
-	}
-
-	reflect.ValueOf(v).Elem().Set(out)
-}
-
 func TestDisableEnveloperNoWireRead(t *testing.T) {
 	cont := "some buffered contents"
-
 	buf := bytes.NewBuffer([]byte(cont))
 	evnw := disableEnvelopingNoWireProtocol{Protocol: binary.Default, Type: wire.Call}
 	sr := evnw.Reader(buf)
@@ -126,7 +105,30 @@ func TestDisableEnveloperNoWireWrite(t *testing.T) {
 	evnw := disableEnvelopingNoWireProtocol{Protocol: binary.Default, Type: wire.OneWay}
 	sw := evnw.Writer(&buf)
 
-	assert.NoError(t, sw.WriteEnvelopeBegin(stream.EnvelopeHeader{Name: "foo", Type: wire.Exception}))
-	assert.NoError(t, sw.WriteEnvelopeEnd())
+	err := sw.WriteEnvelopeBegin(stream.EnvelopeHeader{Name: "foo", Type: wire.Exception})
+	require.NoError(t, err)
+
+	err = sw.WriteEnvelopeEnd()
+	assert.NoError(t, err)
 	assert.Zero(t, buf.Len(), "writeenvelope is not supposed to write to the buffer")
 }
+
+// generate generates a random value into the given pointer.
+//
+// 	var i int
+// 	generate(&i, rand)
+//
+// If the type implements the quick.Generator interface, that is used.
+func generate(v interface{}, r *rand.Rand) {
+	t := reflect.TypeOf(v)
+	if t.Kind() != reflect.Ptr {
+		panic(fmt.Sprintf("%v is not a pointer type", t))
+	}
+
+	out, ok := quick.Value(t.Elem(), r)
+	if !ok {
+		panic(fmt.Sprintf("could not generate a value for %v", t))
+	}
+
+	reflect.ValueOf(v).Elem().Set(out)
+}

encoding/thrift/fakes_test.go

@@ -27,10 +27,7 @@ import (
 	"go.uber.org/thriftrw/wire"
 )
 
-const (
-	_irrelevant = "irrelevant"
-	_response   = "response"
-)
+const _irrelevant = "irrelevant"
 
 type fakeBodyReader struct {
 	body string
@@ -42,6 +39,8 @@ func (f *fakeBodyReader) Decode(sr stream.Reader) error {
 	return err
 }
 
+// TODO(witriew): fakeEnveloper should be created with a constructor, allowing
+// its uses to dictate the returned values for MethodName and encoded string.
 type fakeEnveloper wire.EnvelopeType
 
 func (fakeEnveloper) MethodName() string {

encoding/thrift/multiplex.go

@@ -89,7 +89,7 @@ func (w multiplexedNoWireWriter) WriteEnvelopeBegin(eh stream.EnvelopeHeader) er
 }
 
 // multiplexedNoWireReader overrides the normal ReadEnvelopeBegin with stripping
-// away the service name from the envlope.
+// away the service name from the envelope.
 type multiplexedNoWireReader struct {
 	stream.Reader
 

encoding/thrift/outbound_nowire.go

@@ -23,6 +23,7 @@ package thrift
 import (
 	"bytes"
 	"context"
+	"fmt"
 
 	"go.uber.org/thriftrw/protocol/binary"
 	"go.uber.org/thriftrw/protocol/stream"
@@ -56,7 +57,8 @@ type NoWireClient interface {
 	Enabled() bool
 }
 
-// NewNoWire creates a new Thrift client.
+// NewNoWire creates a new Thrift client that leverages ThriftRW's "streaming"
+// implementation.
 func NewNoWire(c Config, opts ...ClientOption) NoWireClient {
 	// Code generated for Thrift client instantiation will probably be something
 	// like this:
@@ -82,6 +84,10 @@ func NewNoWire(c Config, opts ...ClientOption) NoWireClient {
 	if cc.Protocol != nil {
 		if val, ok := cc.Protocol.(stream.Protocol); ok {
 			p = val
+		} else {
+			panic(fmt.Sprintf(
+				"You have found a bug in YARPC. Please file a bug report at https://github.com/yarpc/yarpc-go/issues with this message: "+
+					"NewNoWire expected provided protocol %T to implement stream.Protocol", cc.Protocol))
 		}
 	}