Add more null checks to call methods
diff --git a/src/node/ext/call.cc b/src/node/ext/call.cc
index 2cc9f63..e2185da 100644
--- a/src/node/ext/call.cc
+++ b/src/node/ext/call.cc
@@ -562,10 +562,12 @@
Call::Call(grpc_call *call) : wrapped_call(call),
pending_batches(0),
has_final_op_completed(false) {
+ peer = grpc_call_get_peer(call);
}
Call::~Call() {
DestroyCall();
+ gpr_free(peer);
}
void Call::Init(Local<Object> exports) {
@@ -794,6 +796,11 @@
return Nan::ThrowTypeError("cancel can only be called on Call objects");
}
Call *call = ObjectWrap::Unwrap<Call>(info.This());
+ if (call->wrapped_call == NULL) {
+ /* Cancel is supposed to be idempotent. If the call has already finished,
+ * cancel should just complete silently */
+ return;
+ }
grpc_call_error error = grpc_call_cancel(call->wrapped_call, NULL);
if (error != GRPC_CALL_OK) {
return Nan::ThrowError(nanErrorWithCode("cancel failed", error));
@@ -814,6 +821,11 @@
"cancelWithStatus's second argument must be a string");
}
Call *call = ObjectWrap::Unwrap<Call>(info.This());
+ if (call->wrapped_call == NULL) {
+ /* Cancel is supposed to be idempotent. If the call has already finished,
+ * cancel should just complete silently */
+ return;
+ }
grpc_status_code code = static_cast<grpc_status_code>(
Nan::To<uint32_t>(info[0]).FromJust());
if (code == GRPC_STATUS_OK) {
@@ -830,9 +842,7 @@
return Nan::ThrowTypeError("getPeer can only be called on Call objects");
}
Call *call = ObjectWrap::Unwrap<Call>(info.This());
- char *peer = grpc_call_get_peer(call->wrapped_call);
- Local<Value> peer_value = Nan::New(peer).ToLocalChecked();
- gpr_free(peer);
+ Local<Value> peer_value = Nan::New(call->peer).ToLocalChecked();
info.GetReturnValue().Set(peer_value);
}
@@ -847,6 +857,10 @@
"setCredentials' first argument must be a CallCredentials");
}
Call *call = ObjectWrap::Unwrap<Call>(info.This());
+ if (call->wrapped_call == NULL) {
+ return Nan::ThrowError(
+ "Cannot set credentials on a call that has already started");
+ }
CallCredentials *creds_object = ObjectWrap::Unwrap<CallCredentials>(
Nan::To<Object>(info[0]).ToLocalChecked());
grpc_call_credentials *creds = creds_object->GetWrappedCredentials();
diff --git a/src/node/ext/call.h b/src/node/ext/call.h
index 340e326..45b448e 100644
--- a/src/node/ext/call.h
+++ b/src/node/ext/call.h
@@ -97,6 +97,7 @@
call, this is GRPC_OP_RECV_STATUS_ON_CLIENT and for a server call, this
is GRPC_OP_SEND_STATUS_FROM_SERVER */
bool has_final_op_completed;
+ char *peer;
};
class Op {
diff --git a/src/node/test/common_test.js b/src/node/test/common_test.js
index b7c2c6a..db80207 100644
--- a/src/node/test/common_test.js
+++ b/src/node/test/common_test.js
@@ -100,7 +100,6 @@
var longNumDeserialize = deserializeCls(messages_proto.LongValues,
num_options);
var serialized = longSerialize({int_64: pos_value});
- console.log(longDeserialize(serialized));
assert.strictEqual(typeof longDeserialize(serialized).int_64, 'string');
/* With the longsAsStrings option disabled, long values are represented as
* objects with 3 keys: low, high, and unsigned */
diff --git a/src/node/test/surface_test.js b/src/node/test/surface_test.js
index d2f0511..0696e7a 100644
--- a/src/node/test/surface_test.js
+++ b/src/node/test/surface_test.js
@@ -1110,6 +1110,18 @@
done();
});
});
+ it('after the call has fully completed', function(done) {
+ var peer;
+ var call = client.unary({error: false}, function(err, data) {
+ assert.ifError(err);
+ setImmediate(function() {
+ assert.strictEqual(peer, call.getPeer());
+ done();
+ });
+ });
+ peer = call.getPeer();
+ assert.strictEqual(typeof peer, 'string');
+ });
});
});
describe('Call propagation', function() {
@@ -1352,4 +1364,17 @@
});
call.cancel();
});
+ it('Should be idempotent', function(done) {
+ var call = client.div({'divisor': 0, 'dividend': 0}, function(err, resp) {
+ assert.strictEqual(err.code, surface_client.status.CANCELLED);
+ // Call asynchronously to try cancelling after call is fully completed
+ setImmediate(function() {
+ assert.doesNotThrow(function() {
+ call.cancel();
+ });
+ done();
+ });
+ });
+ call.cancel();
+ });
});