Repository: grpc/grpc-go
File: stream.go (Go)

Code before:

	if md, added, ok := metadataFromOutgoingContextRaw(ctx); ok {
		// validate md
		if err := imetadata.Validate(md); err != nil {
			return nil, status.Error(codes.Internal, err.Error())
		}
		// validate added
		for _, kvs := range added {
			for i := 0; i < len(kvs); i += 2 {
				if err := imetadata.ValidatePair(kvs[i], kvs[i+1]); err != nil {
					return nil, status.Error(codes.Internal, err.Error())
				}
			}
		}
	}
	if channelz.IsOn() {
		cc.incrCallsStarted()
		defer func() {
			if err != nil {
				cc.incrCallsFailed()
			}
		}()
	}
	// Provide an opportunity for the first RPC to see the first service config
	// provided by the resolver.
	isDelayed, err := cc.waitForResolvedAddrs(ctx)
	if err != nil {
		return nil, err
	}
	if isDelayed {
		ctx = context.WithValue(ctx, nameResolutionDelayKey, isDelayed)
	}

	var mc serviceconfig.MethodConfig
	var onCommit func()
	newStream := func(ctx context.Context, done func()) (iresolver.ClientStream, error) {
		return newClientStreamWithParams(ctx, desc, cc, method, mc, onCommit, done, opts...)
	}

	rpcInfo := iresolver.RPCInfo{Context: ctx, Method: method}
	rpcConfig, err := cc.safeConfigSelector.SelectConfig(rpcInfo)
	if err != nil {
		if st, ok := status.FromError(err); ok {
			// Restrict the code to the list allowed by gRFC A54.
			if istatus.IsRestrictedControlPlaneCode(st) {
				err = status.Errorf(codes.Internal, "config selector returned illegal status: %v", err)
			}
			return nil, err
		}
		return nil, toRPCErr(err)
	}

Review comment (bug):
returning a bool and error is not good practice for go. It breaks the established pattern of error handling in Go because returned bool indicates success/failure in general. Can we do something better? It might be fine if we can't but we can try to look for better ways.

Code after:
			return nil, status.Error(codes.Internal, err.Error())
		}
		// validate added
		for _, kvs := range added {
			for i := 0; i < len(kvs); i += 2 {
				if err := imetadata.ValidatePair(kvs[i], kvs[i+1]); err != nil {
					return nil, status.Error(codes.Internal, err.Error())
				}
			}
		}
	}
	if channelz.IsOn() {
		cc.incrCallsStarted()
		defer func() {
			if err != nil {
				cc.incrCallsFailed()
			}
		}()
	}
	// Provide an opportunity for the first RPC to see the first service config
	// provided by the resolver.
	nameResolutionDelayed, err := cc.waitForResolvedAddrs(ctx)
	if err != nil {
		return nil, err
	}

	var mc serviceconfig.MethodConfig
	var onCommit func()
	newStream := func(ctx context.Context, done func()) (iresolver.ClientStream, error) {
		return newClientStreamWithParams(ctx, desc, cc, method, mc, onCommit, done, nameResolutionDelayed, opts...)
	}

	rpcInfo := iresolver.RPCInfo{Context: ctx, Method: method}
	rpcConfig, err := cc.safeConfigSelector.SelectConfig(rpcInfo)
	if err != nil {
		if st, ok := status.FromError(err); ok {
			// Restrict the code to the list allowed by gRFC A54.
			if istatus.IsRestrictedControlPlaneCode(st) {
				err = status.Errorf(codes.Internal, "config selector returned illegal status: %v", err)
			}
			return nil, err
		}
		return nil, toRPCErr(err)
	}

	if rpcConfig != nil {
		if rpcConfig.Context != nil {
			ctx = rpcConfig.Context
		}
		mc = rpcConfig.MethodConfig
		onCommit = rpcConfig.OnCommitted