Merge remote-tracking branch 'upstream/master' into interop

src/objective-c/GRPCClient/GRPCCall.h

@@ -48,11 +48,112 @@
 #import <Foundation/Foundation.h>
 #import <RxLibrary/GRXWriter.h>
 
+#pragma mark gRPC errors
+
+// Domain of NSError objects produced by gRPC.
+extern NSString *const kGRPCErrorDomain;
+
+// gRPC error codes.
+// Note that a few of these are never produced by the gRPC libraries, but are of general utility for
+// server applications to produce.
+typedef NS_ENUM(NSUInteger, GRPCErrorCode) {
+  // The operation was cancelled (typically by the caller).
+  GRPCErrorCodeCancelled = 1,
+
+  // Unknown error. Errors raised by APIs that do not return enough error information may be
+  // converted to this error.
+  GRPCErrorCodeUnknown = 2,
+
+  // The client specified an invalid argument. Note that this differs from FAILED_PRECONDITION.
+  // INVALID_ARGUMENT indicates arguments that are problematic regardless of the state of the
+  // server (e.g., a malformed file name).
+  GRPCErrorCodeInvalidArgument = 3,
+
+  // Deadline expired before operation could complete. For operations that change the state of the
+  // server, this error may be returned even if the operation has completed successfully. For
+  // example, a successful response from the server could have been delayed long enough for the
+  // deadline to expire.
+  GRPCErrorCodeDeadlineExceeded = 4,
+
+  // Some requested entity (e.g., file or directory) was not found.
+  GRPCErrorCodeNotFound = 5,
+
+  // Some entity that we attempted to create (e.g., file or directory) already exists.
+  GRPCErrorCodeAlreadyExists = 6,
+
+  // The caller does not have permission to execute the specified operation. PERMISSION_DENIED isn't
+  // used for rejections caused by exhausting some resource (RESOURCE_EXHAUSTED is used instead for
+  // those errors). PERMISSION_DENIED doesn't indicate a failure to identify the caller
+  // (UNAUTHENTICATED is used instead for those errors).
+  GRPCErrorCodePermissionDenied = 7,
+
+  // The request does not have valid authentication credentials for the operation (e.g. the caller's
+  // identity can't be verified).
+  GRPCErrorCodeUnauthenticated = 16,
+
+  // Some resource has been exhausted, perhaps a per-user quota.
+  GRPCErrorCodeResourceExhausted = 8,
+
+  // The RPC was rejected because the server is not in a state required for the procedure's
+  // execution. For example, a directory to be deleted may be non-empty, etc.
+  // The client should not retry until the server state has been explicitly fixed (e.g. by
+  // performing another RPC). The details depend on the service being called, and should be found in
+  // the NSError's userInfo.
+  GRPCErrorCodeFailedPrecondition = 9,
+
+  // The RPC was aborted, typically due to a concurrency issue like sequencer check failures,
+  // transaction aborts, etc. The client should retry at a higher-level (e.g., restarting a read-
+  // modify-write sequence).
+  GRPCErrorCodeAborted = 10,
+
+  // The RPC was attempted past the valid range. E.g., enumerating past the end of a list.
+  // Unlike INVALID_ARGUMENT, this error indicates a problem that may be fixed if the system state
+  // changes. For example, an RPC to get elements of a list will generate INVALID_ARGUMENT if asked
+  // to return the element at a negative index, but it will generate OUT_OF_RANGE if asked to return
+  // the element at an index past the current size of the list.
+  GRPCErrorCodeOutOfRange = 11,
+
+  // The procedure is not implemented or not supported/enabled in this server.
+  GRPCErrorCodeUnimplemented = 12,
+
+  // Internal error. Means some invariant expected by the server application or the gRPC library has
+  // been broken.
+  GRPCErrorCodeInternal = 13,
+
+  // The server is currently unavailable. This is most likely a transient condition and may be
+  // corrected by retrying with a backoff.
+  GRPCErrorCodeUnavailable = 14,
+
+  // Unrecoverable data loss or corruption.
+  GRPCErrorCodeDataLoss = 15,
+};
+
 // Keys used in |NSError|'s |userInfo| dictionary to store the response headers and trailers sent by
 // the server.
 extern id const kGRPCHeadersKey;
 extern id const kGRPCTrailersKey;
 
+#pragma mark GRPCCall
+
+// The container of the request headers of an RPC conforms to this protocol, which is a subset of
+// NSMutableDictionary's interface. It will become a NSMutableDictionary later on.
+// The keys of this container are the header names, which per the HTTP standard are case-
+// insensitive. They are stored in lowercase (which is how HTTP/2 mandates them on the wire), and
+// can only consist of ASCII characters.
+// A header value is a NSString object (with only ASCII characters), unless the header name has the
+// suffix "-bin", in which case the value has to be a NSData object.
+@protocol GRPCRequestHeaders <NSObject>
+
+@property(nonatomic, readonly) NSUInteger count;
+
+- (id)objectForKeyedSubscript:(NSString *)key;
+- (void)setObject:(id)obj forKeyedSubscript:(NSString *)key;
+
+- (void)removeAllObjects;
+- (void)removeObjectForKey:(NSString *)key;
+
+@end
+
 // Represents a single gRPC remote call.
 @interface GRPCCall : GRXWriter
 
@@ -70,7 +171,7 @@ extern id const kGRPCTrailersKey;
 //
 // For convenience, the property is initialized to an empty NSMutableDictionary, and the setter
 // accepts (and copies) both mutable and immutable dictionaries.
-- (NSMutableDictionary *)requestHeaders; // nonatomic
+- (id<GRPCRequestHeaders>)requestHeaders; // nonatomic
 - (void)setRequestHeaders:(NSDictionary *)requestHeaders; // nonatomic, copy
 
 // This dictionary is populated with the HTTP headers received from the server. This happens before

src/objective-c/GRPCClient/GRPCCall.m

@@ -37,6 +37,7 @@
 #include <grpc/support/time.h>
 #import <RxLibrary/GRXConcurrentWriteable.h>
 
+#import "private/GRPCRequestHeaders.h"
 #import "private/GRPCWrappedCall.h"
 #import "private/NSData+GRPC.h"
 #import "private/NSDictionary+GRPC.h"
@@ -93,7 +94,7 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey";
   // the response arrives.
   GRPCCall *_retainSelf;
 
-  NSMutableDictionary *_requestHeaders;
+  GRPCRequestHeaders *_requestHeaders;
 }
 
 @synthesize state = _state;
@@ -124,19 +125,23 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey";
 
     _requestWriter = requestWriter;
 
-    _requestHeaders = [NSMutableDictionary dictionary];
+    _requestHeaders = [[GRPCRequestHeaders alloc] initWithCall:self];
   }
   return self;
 }
 
 #pragma mark Metadata
 
-- (NSMutableDictionary *)requestHeaders {
+- (id<GRPCRequestHeaders>)requestHeaders {
   return _requestHeaders;
 }
 
 - (void)setRequestHeaders:(NSDictionary *)requestHeaders {
-  _requestHeaders = [NSMutableDictionary dictionaryWithDictionary:requestHeaders];
+  GRPCRequestHeaders *newHeaders = [[GRPCRequestHeaders alloc] initWithCall:self];
+  for (id key in requestHeaders) {
+    newHeaders[key] = requestHeaders[key];
+  }
+  _requestHeaders = newHeaders;
 }
 
 #pragma mark Finish
@@ -230,10 +235,10 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey";
 
 #pragma mark Send headers
 
-- (void)sendHeaders:(NSDictionary *)headers {
+- (void)sendHeaders:(id<GRPCRequestHeaders>)headers {
   // TODO(jcanizales): Add error handlers for async failures
-  [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendMetadata alloc]
+  [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendMetadata alloc] initWithMetadata:headers
-                                            initWithMetadata:headers ?: @{} handler:nil]]];
+                                                                                handler:nil]]];
 }
 
 #pragma mark GRXWriteable implementation

src/objective-c/GRPCClient/private/GRPCRequestHeaders.h

@@ -0,0 +1,52 @@
+/*
+ *
+ * Copyright 2015, Google Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *     * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+#import <Foundation/Foundation.h>
+#include <grpc/grpc.h>
+
+#import "GRPCCall.h"
+
+@interface GRPCRequestHeaders : NSObject<GRPCRequestHeaders>
+
+@property(nonatomic, readonly) NSUInteger count;
+@property(nonatomic, readonly) grpc_metadata *grpc_metadataArray;
+
+- (instancetype)initWithCall:(GRPCCall *)call;
+
+- (id)objectForKeyedSubscript:(NSString *)key;
+- (void)setObject:(id)obj forKeyedSubscript:(NSString *)key;
+
+- (void)removeAllObjects;
+- (void)removeObjectForKey:(NSString *)key;
+
+@end

src/objective-c/GRPCClient/private/GRPCRequestHeaders.m

@@ -0,0 +1,119 @@
+/*
+ *
+ * Copyright 2015, Google Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *     * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+#import "GRPCRequestHeaders.h"
+
+#import <Foundation/Foundation.h>
+
+#import "GRPCCall.h"
+#import "NSDictionary+GRPC.h"
+
+// Used by the setter.
+static void CheckIsNonNilASCII(NSString *name, NSString* value) {
+  if (!value) {
+    [NSException raise:NSInvalidArgumentException format:@"%@ cannot be nil", name];
+  }
+  if (![value canBeConvertedToEncoding:NSASCIIStringEncoding]) {
+    [NSException raise:NSInvalidArgumentException
+                format:@"%@ %@ contains non-ASCII characters", name, value];
+  }
+}
+
+// Precondition: key isn't nil.
+static void CheckKeyValuePairIsValid(NSString *key, id value) {
+  if ([key hasSuffix:@"-bin"]) {
+    if (![value isKindOfClass:NSData.class]) {
+      [NSException raise:NSInvalidArgumentException
+                  format:@"Expected NSData value for header %@ ending in \"-bin\", "
+       @"instead got %@", key, value];
+    }
+  } else {
+    if (![value isKindOfClass:NSString.class]) {
+      [NSException raise:NSInvalidArgumentException
+                  format:@"Expected NSString value for header %@ not ending in \"-bin\", "
+       @"instead got %@", key, value];
+    }
+    CheckIsNonNilASCII(@"Text header value", (NSString *)value);
+  }
+}
+
+@implementation GRPCRequestHeaders {
+  __weak GRPCCall *_call;
+  NSMutableDictionary *_delegate;
+}
+
+- (instancetype)initWithCall:(GRPCCall *)call {
+  if ((self = [super init])) {
+    _call = call;
+    _delegate = [NSMutableDictionary dictionary];
+  }
+  return self;
+}
+
+- (void)checkCallIsNotStarted {
+  if (_call.state != GRXWriterStateNotStarted) {
+    [NSException raise:@"Invalid modification"
+                format:@"Cannot modify request headers after call is started"];
+  }
+}
+
+- (id)objectForKeyedSubscript:(NSString *)key {
+  return _delegate[key.lowercaseString];
+}
+
+- (void)setObject:(id)obj forKeyedSubscript:(NSString *)key {
+  [self checkCallIsNotStarted];
+  CheckIsNonNilASCII(@"Header name", key);
+  key = key.lowercaseString;
+  CheckKeyValuePairIsValid(key, obj);
+  _delegate[key] = obj;
+}
+
+- (void)removeObjectForKey:(NSString *)key {
+  [self checkCallIsNotStarted];
+  [_delegate removeObjectForKey:key.lowercaseString];
+}
+
+- (void)removeAllObjects {
+  [self checkCallIsNotStarted];
+  [_delegate removeAllObjects];
+}
+
+- (NSUInteger)count {
+  return _delegate.count;
+}
+
+- (grpc_metadata *)grpc_metadataArray {
+  return _delegate.grpc_metadataArray;
+}
+@end

src/objective-c/GRPCClient/private/GRPCWrappedCall.h

@@ -35,6 +35,7 @@
 #include <grpc/grpc.h>
 
 #import "GRPCChannel.h"
+#import "GRPCRequestHeaders.h"
 
 @interface GRPCOperation : NSObject
 @property(nonatomic, readonly) grpc_op op;
@@ -44,7 +45,7 @@
 
 @interface GRPCOpSendMetadata : GRPCOperation
 
-- (instancetype)initWithMetadata:(NSDictionary *)metadata
+- (instancetype)initWithMetadata:(GRPCRequestHeaders *)metadata
                          handler:(void(^)())handler NS_DESIGNATED_INITIALIZER;
 
 @end

src/objective-c/GRPCClient/private/GRPCWrappedCall.m

@@ -65,7 +65,7 @@
   return [self initWithMetadata:nil handler:nil];
 }
 
-- (instancetype)initWithMetadata:(NSDictionary *)metadata handler:(void (^)())handler {
+- (instancetype)initWithMetadata:(GRPCRequestHeaders *)metadata handler:(void (^)())handler {
   if (self = [super init]) {
     _op.op = GRPC_OP_SEND_INITIAL_METADATA;
     _op.data.send_initial_metadata.count = metadata.count;

src/objective-c/GRPCClient/private/NSDictionary+GRPC.m

@@ -40,8 +40,8 @@
 @interface NSData (GRPCMetadata)
 + (instancetype)grpc_dataFromMetadataValue:(grpc_metadata *)metadata;
 
-// Fill a metadata object with the binary value in this NSData and the given key.
+// Fill a metadata object with the binary value in this NSData.
-- (void)grpc_initMetadata:(grpc_metadata *)metadata withKey:(NSString *)key;
+- (void)grpc_initMetadata:(grpc_metadata *)metadata;
 @end
 
 @implementation NSData (GRPCMetadata)
@@ -50,9 +50,7 @@
   return [self dataWithBytes:metadata->value length:metadata->value_length];
 }
 
-- (void)grpc_initMetadata:(grpc_metadata *)metadata withKey:(NSString *)key {
+- (void)grpc_initMetadata:(grpc_metadata *)metadata {
-  // TODO(jcanizales): Encode Unicode chars as ASCII.
-  metadata->key = [key stringByAppendingString:@"-bin"].UTF8String;
   metadata->value = self.bytes;
   metadata->value_length = self.length;
 }
@@ -63,8 +61,8 @@
 @interface NSString (GRPCMetadata)
 + (instancetype)grpc_stringFromMetadataValue:(grpc_metadata *)metadata;
 
-// Fill a metadata object with the textual value in this NSString and the given key.
+// Fill a metadata object with the textual value in this NSString.
-- (void)grpc_initMetadata:(grpc_metadata *)metadata withKey:(NSString *)key;
+- (void)grpc_initMetadata:(grpc_metadata *)metadata;
 @end
 
 @implementation NSString (GRPCMetadata)
@@ -74,22 +72,8 @@
                             encoding:NSASCIIStringEncoding];
 }
 
-- (void)grpc_initMetadata:(grpc_metadata *)metadata withKey:(NSString *)key {
+// Precondition: This object contains only ASCII characters.
-  if ([key hasSuffix:@"-bin"]) {
+- (void)grpc_initMetadata:(grpc_metadata *)metadata {
-    // Disallow this, as at best it will confuse the server. If the app really needs to send a
-    // textual header with a name ending in "-bin", it can be done by removing the suffix and
-    // encoding the NSString as a NSData object.
-    //
-    // Why raise an exception: In the most common case, the developer knows this won't happen in
-    // their code, so the exception isn't triggered. In the rare cases when the developer can't
-    // tell, it's easy enough to add a sanitizing filter before the header is set. There, the
-    // developer can choose whether to drop such a header, or trim its name. Doing either ourselves,
-    // silently, would be very unintuitive for the user.
-    [NSException raise:NSInvalidArgumentException
-                format:@"Metadata keys ending in '-bin' are reserved for NSData values."];
-  }
-  // TODO(jcanizales): Encode Unicode chars as ASCII.
-  metadata->key = key.UTF8String;
   metadata->value = self.UTF8String;
   metadata->value_length = self.length;
 }
@@ -105,8 +89,6 @@
 + (instancetype)grpc_dictionaryFromMetadata:(grpc_metadata *)entries count:(size_t)count {
   NSMutableDictionary *metadata = [NSMutableDictionary dictionaryWithCapacity:count];
   for (grpc_metadata *entry = entries; entry < entries + count; entry++) {
-    // TODO(jcanizales): Verify in a C library test that it's converting header names to lower case
-    // automatically.
     NSString *name = [NSString stringWithCString:entry->key encoding:NSASCIIStringEncoding];
     if (!name || metadata[name]) {
       // Log if name is nil?
@@ -114,7 +96,6 @@
     }
     id value;
     if ([name hasSuffix:@"-bin"]) {
-      name = [name substringToIndex:name.length - 4];
       value = [NSData grpc_dataFromMetadataValue:entry];
     } else {
       value = [NSString grpc_stringFromMetadataValue:entry];
@@ -124,19 +105,21 @@
   return metadata;
 }
 
+// Preconditions: All keys are ASCII strings. Keys ending in -bin have NSData values; the others
+// have NSString values.
 - (grpc_metadata *)grpc_metadataArray {
   grpc_metadata *metadata = gpr_malloc([self count] * sizeof(grpc_metadata));
-  int i = 0;
+  grpc_metadata *current = metadata;
-  for (id key in self) {
+  for (NSString* key in self) {
     id value = self[key];
-    grpc_metadata *current = &metadata[i];
+    current->key = key.UTF8String;
-    if ([value respondsToSelector:@selector(grpc_initMetadata:withKey:)]) {
+    if ([value respondsToSelector:@selector(grpc_initMetadata:)]) {
-      [value grpc_initMetadata:current withKey:key];
+      [value grpc_initMetadata:current];
     } else {
       [NSException raise:NSInvalidArgumentException
                   format:@"Metadata values must be NSString or NSData."];
     }
-    i += 1;
+    ++current;
   }
   return metadata;
 }

src/objective-c/GRPCClient/private/NSError+GRPC.h

@@ -34,29 +34,6 @@
 #import <Foundation/Foundation.h>
 #include <grpc/grpc.h>
 
-// TODO(jcanizales): Make the domain string public.
-extern NSString *const kGRPCErrorDomain;
-
-// TODO(jcanizales): Make this public and document each code.
-typedef NS_ENUM(NSInteger, GRPCErrorCode) {
-  GRPCErrorCodeCancelled = 1,
-  GRPCErrorCodeUnknown = 2,
-  GRPCErrorCodeInvalidArgument = 3,
-  GRPCErrorCodeDeadlineExceeded = 4,
-  GRPCErrorCodeNotFound = 5,
-  GRPCErrorCodeAlreadyExists = 6,
-  GRPCErrorCodePermissionDenied = 7,
-  GRPCErrorCodeUnauthenticated = 16,
-  GRPCErrorCodeResourceExhausted = 8,
-  GRPCErrorCodeFailedPrecondition = 9,
-  GRPCErrorCodeAborted = 10,
-  GRPCErrorCodeOutOfRange = 11,
-  GRPCErrorCodeUnimplemented = 12,
-  GRPCErrorCodeInternal = 13,
-  GRPCErrorCodeUnavailable = 14,
-  GRPCErrorCodeDataLoss = 15
-};
-
 @interface NSError (GRPC)
 // Returns nil if the status code is OK. Otherwise, a NSError whose code is one of |GRPCErrorCode|
 // and whose domain is |kGRPCErrorDomain|.

src/objective-c/ProtoRPC/ProtoRPC.m

@@ -37,6 +37,22 @@
 #import <RxLibrary/GRXWriteable.h>
 #import <RxLibrary/GRXWriter+Transformations.h>
 
+static NSError *ErrorForBadProto(id proto, Class expectedClass, NSError *parsingError) {
+  NSDictionary *info = @{
+                         NSLocalizedDescriptionKey: @"Unable to parse response from the server",
+                         NSLocalizedRecoverySuggestionErrorKey: @"If this RPC is idempotent, retry "
+                         @"with exponential backoff. Otherwise, query the server status before "
+                         @"retrying.",
+                         NSUnderlyingErrorKey: parsingError,
+                         @"Expected class": expectedClass,
+                         @"Received value": proto,
+                         };
+  // TODO(jcanizales): Use kGRPCErrorDomain and GRPCErrorCodeInternal when they're public.
+  return [NSError errorWithDomain:@"io.grpc"
+                             code:13
+                         userInfo:info];
+}
+
 @implementation ProtoRPC {
   id<GRXWriteable> _responseWriteable;
 }
@@ -65,14 +81,25 @@
   }
   // A writer that serializes the proto messages to send.
   GRXWriter *bytesWriter = [requestsWriter map:^id(GPBMessage *proto) {
-    // TODO(jcanizales): Fail with an understandable error message if the requestsWriter isn't
+    if (![proto isKindOfClass:GPBMessage.class]) {
-    // sending GPBMessages.
+      [NSException raise:NSInvalidArgumentException
+                  format:@"Request must be a proto message: %@", proto];
+    }
     return [proto data];
   }];
   if ((self = [super initWithHost:host path:method.HTTPPath requestsWriter:bytesWriter])) {
+    __weak ProtoRPC *weakSelf = self;
+
     // A writeable that parses the proto messages received.
     _responseWriteable = [[GRXWriteable alloc] initWithValueHandler:^(NSData *value) {
-      [responsesWriteable writeValue:[responseClass parseFromData:value error:NULL]];
+      // TODO(jcanizales): This is done in the main thread, and needs to happen in another thread.
+      NSError *error = nil;
+      id parsed = [responseClass parseFromData:value error:&error];
+      if (parsed) {
+        [responsesWriteable writeValue:parsed];
+      } else {
+        [weakSelf finishWithError:ErrorForBadProto(value, responseClass, error)];
+      }
     } completionHandler:^(NSError *errorOrNil) {
       [responsesWriteable writesFinishedWithError:errorOrNil];
     }];

test/cpp/end2end/thread_stress_test.cc

@@ -191,7 +191,7 @@ class End2endTest : public ::testing::Test {
   void ResetStub() {
     std::shared_ptr<Channel> channel =
         CreateChannel(server_address_.str(), InsecureCredentials());
-    stub_ = std::move(grpc::cpp::test::util::TestService::NewStub(channel));
+    stub_ = grpc::cpp::test::util::TestService::NewStub(channel);
   }
 
   std::unique_ptr<grpc::cpp::test::util::TestService::Stub> stub_;

test/cpp/qps/driver.cc

@@ -153,14 +153,14 @@ std::unique_ptr<ScenarioResult> RunScenario(
   // where class contained in std::vector must have a copy constructor
   auto* servers = new ServerData[num_servers];
   for (size_t i = 0; i < num_servers; i++) {
-    servers[i].stub = std::move(
+    servers[i].stub = Worker::NewStub(CreateChannel(workers[i],
-        Worker::NewStub(CreateChannel(workers[i], InsecureCredentials())));
+                                                    InsecureCredentials()));
     ServerArgs args;
     result_server_config = server_config;
     result_server_config.set_host(workers[i]);
     *args.mutable_setup() = server_config;
     servers[i].stream =
-        std::move(servers[i].stub->RunServer(runsc::AllocContext(&contexts)));
+        servers[i].stub->RunServer(runsc::AllocContext(&contexts));
     GPR_ASSERT(servers[i].stream->Write(args));
     ServerStatus init_status;
     GPR_ASSERT(servers[i].stream->Read(&init_status));
@@ -181,14 +181,14 @@ std::unique_ptr<ScenarioResult> RunScenario(
   // where class contained in std::vector must have a copy constructor
   auto* clients = new ClientData[num_clients];
   for (size_t i = 0; i < num_clients; i++) {
-    clients[i].stub = std::move(Worker::NewStub(
+    clients[i].stub = Worker::NewStub(CreateChannel(workers[i + num_servers],
-        CreateChannel(workers[i + num_servers], InsecureCredentials())));
+                                                    InsecureCredentials()));
     ClientArgs args;
     result_client_config = client_config;
     result_client_config.set_host(workers[i + num_servers]);
     *args.mutable_setup() = client_config;
     clients[i].stream =
-        std::move(clients[i].stub->RunTest(runsc::AllocContext(&contexts)));
+        clients[i].stub->RunTest(runsc::AllocContext(&contexts));
     GPR_ASSERT(clients[i].stream->Write(args));
     ClientStatus init_status;
     GPR_ASSERT(clients[i].stream->Read(&init_status));

test/cpp/qps/qps_worker.cc

@@ -229,7 +229,7 @@ QpsWorker::QpsWorker(int driver_port, int server_port) {
 
   gpr_free(server_address);
 
-  server_ = std::move(builder.BuildAndStart());
+  server_ = builder.BuildAndStart();
 }
 
 QpsWorker::~QpsWorker() {}

test/cpp/qps/server_async.cc

@@ -68,7 +68,7 @@ class AsyncQpsServerTest : public Server {
 
     builder.RegisterAsyncService(&async_service_);
     for (int i = 0; i < config.threads(); i++) {
-      srv_cqs_.emplace_back(std::move(builder.AddCompletionQueue()));
+      srv_cqs_.emplace_back(builder.AddCompletionQueue());
     }
 
     server_ = builder.BuildAndStart();

test/cpp/util/cli_call_test.cc

@@ -90,7 +90,7 @@ class CliCallTest : public ::testing::Test {
 
   void ResetStub() {
     channel_ = CreateChannel(server_address_.str(), InsecureCredentials());
-    stub_ = std::move(grpc::cpp::test::util::TestService::NewStub(channel_));
+    stub_ = grpc::cpp::test::util::TestService::NewStub(channel_);
   }
 
   std::shared_ptr<Channel> channel_;