Repository navigation
Interop test helpers: allow passing call options (e.g. deadline) to the underlying RPCs #3125
Description
Is your feature request related to a problem? Please describe.
Location (grpc/grpc-node @ default branch ca7f83f):
test/interop/interop_client.js:88-95—emptyUnarytest/interop/interop_client.js:226-238—emptyStreamtest/interop/interop_client.js:502-545—test_casestest/stress/stress_client.js:36-45— stress client
function emptyUnary(client, done) {
client.emptyCall({}, function(err, resp) {function emptyStream(client, done) {
var call = client.fullDuplexCall();Problem
The test-case functions in test/interop/interop_client.js (emptyUnary, largeUnary, clientStreaming, serverStreaming, pingPong, emptyStream, cancelAfter*, customMetadata, statusCodeAndMessage, …) build every RPC with fixed arguments. Code that reuses these cases through exports.test_cases[...].run therefore cannot set a deadline (or other call options) on the RPCs. This includes test/stress/stress_client.js, which calls them in a loop, and ad-hoc mocha tests. If the server stops responding, the call waits indefinitely instead of failing with DEADLINE_EXCEEDED. Only timeoutOnSleepingServer sets a deadline, and it is hard-coded.
This only affects the repository's test helpers. Published @grpc/grpc-js APIs are not involved.
Reproduction
I called test_cases.empty_unary.run(client, done, { deadline }) and test_cases.empty_stream.run(...) the same way, using a client wrapper that records the arguments. The deadline was not passed to emptyCall/fullDuplexCall:
AssertionError [ERR_ASSERTION]: emptyUnary should forward call options (including deadline) to the underlying emptyCall RPC
AssertionError [ERR_ASSERTION]: emptyStream should forward call options (including deadline) to the underlying fullDuplexCall RPC
Environment:
- Commit:
6741e13;test/interop/interop_client.jshas the same blob on currentmasterca7f83f
Describe the solution you'd like
Already implemented in grpc/grpc-go
PR: grpc/grpc-go#6827 — "interop: Replace context.Background() with passed ctx" (merged 2024-01-31T22:23:27Z, merge commit a3f5ed6931c). It changed the grpc-go interop helpers to take a ctx from the caller instead of using context.Background(), so callers can give every case a deadline. The helpers already took call options. The same change runs through all cases and the clients that call them; two examples are quoted.
How grpc-go fixed it:
--- a/interop/test_utils.go
+++ b/interop/test_utils.go
@@ -79,8 +79,8 @@ func ClientNewPayload(t testpb.PayloadType, size int) *testpb.Payload {
}
// DoEmptyUnaryCall performs a unary RPC with empty request and response messages.
-func DoEmptyUnaryCall(tc testgrpc.TestServiceClient, args ...grpc.CallOption) {
- reply, err := tc.EmptyCall(context.Background(), &testpb.Empty{}, args...)
+func DoEmptyUnaryCall(ctx context.Context, tc testgrpc.TestServiceClient, args ...grpc.CallOption) {
+ reply, err := tc.EmptyCall(ctx, &testpb.Empty{}, args...)
if err != nil {
logger.Fatal("/TestService/EmptyCall RPC failed: ", err)
}
...
@@ -223,8 +223,8 @@ func DoPingPong(tc testgrpc.TestServiceClient, args ...grpc.CallOption) {
}
// DoEmptyStream sets up a bi-directional streaming with zero message.
-func DoEmptyStream(tc testgrpc.TestServiceClient, args ...grpc.CallOption) {
- stream, err := tc.FullDuplexCall(context.Background(), args...)
+func DoEmptyStream(ctx context.Context, tc testgrpc.TestServiceClient, args ...grpc.CallOption) {
+ stream, err := tc.FullDuplexCall(ctx, args...)
if err != nil {
logger.Fatalf("%v.FullDuplexCall(_) = _, %v", tc, err)
}Suggested fix
Read the call options from extra.callOptions, keep the positional signatures, and pass them to every RPC the test cases start, as grpc-go's helpers do with the caller's ctx. With no options each RPC gets {}. customMetadata keeps its Metadata argument, and timeoutOnSleepingServer and per_rpc_creds keep their own deadline and credentials values:
Patch
diff --git a/test/interop/interop_client.js b/test/interop/interop_client.js
--- a/test/interop/interop_client.js
+++ b/test/interop/interop_client.js
@@ -47,6 +47,15 @@ try {
var ECHO_INITIAL_KEY = 'x-grpc-test-echo-initial';
var ECHO_TRAILING_KEY = 'x-grpc-test-echo-trailing-bin';
+/**
+ * Get the call options to pass to each RPC of a test case
+ * @param {object=} extra Extra options for the test
+ * @return {object} The value of extra.callOptions, or an empty object
+ */
+function getCallOptions(extra) {
+ return (extra && extra.callOptions) || {};
+}
+
/**
* Create a buffer filled with size zeroes
* @param {number} size The length of the buffer
@@ -85,8 +94,8 @@ function multiDone(done, count) {
* @param {function} done Callback to call when the test is completed. Included
* primarily for use with mocha
*/
-function emptyUnary(client, done) {
- client.emptyCall({}, function(err, resp) {
+function emptyUnary(client, done, extra) {
+ client.emptyCall({}, getCallOptions(extra), function(err, resp) {
assert.ifError(err);
if (done) {
done();
@@ -100,7 +109,7 @@ function emptyUnary(client, done) {
* @param {function} done Callback to call when the test is completed. Included
* primarily for use with mocha
*/
-function largeUnary(client, done) {
+function largeUnary(client, done, extra) {
var arg = {
response_type: 'COMPRESSABLE',
response_size: 314159,
@@ -108,7 +117,7 @@ function largeUnary(client, done) {
body: zeroBuffer(271828)
}
};
- client.unaryCall(arg, function(err, resp) {
+ client.unaryCall(arg, getCallOptions(extra), function(err, resp) {
assert.ifError(err);
assert.strictEqual(resp.payload.type, 'COMPRESSABLE');
assert.strictEqual(resp.payload.body.length, 314159);
@@ -124,8 +133,8 @@ function largeUnary(client, done) {
* @param {function} done Callback to call when the test is completed. Included
* primarily for use with mocha
*/
-function clientStreaming(client, done) {
- var call = client.streamingInputCall(function(err, resp) {
+function clientStreaming(client, done, extra) {
+ var call = client.streamingInputCall(getCallOptions(extra), function(err, resp) {
assert.ifError(err);
assert.strictEqual(resp.aggregated_payload_size, 74922);
if (done) {
@@ -145,7 +154,7 @@ function clientStreaming(client, done) {
* @param {function} done Callback to call when the test is completed. Included
* primarily for use with mocha
*/
-function serverStreaming(client, done) {
+function serverStreaming(client, done, extra) {
var arg = {
response_type: 'COMPRESSABLE',
response_parameters: [
@@ -155,7 +164,7 @@ function serverStreaming(client, done) {
{size: 58979}
]
};
- var call = client.streamingOutputCall(arg);
+ var call = client.streamingOutputCall(arg, getCallOptions(extra));
var resp_index = 0;
call.on('data', function(value) {
assert(resp_index < 4);
@@ -181,10 +190,10 @@ function serverStreaming(client, done) {
* @param {function} done Callback to call when the test is completed. Included
* primarily for use with mocha
*/
-function pingPong(client, done) {
+function pingPong(client, done, extra) {
var payload_sizes = [27182, 8, 1828, 45904];
var response_sizes = [31415, 9, 2653, 58979];
- var call = client.fullDuplexCall();
+ var call = client.fullDuplexCall(getCallOptions(extra));
call.on('status', function(status) {
assert.strictEqual(status.code, grpc.status.OK);
if (done) {
@@ -223,8 +232,8 @@ function pingPong(client, done) {
* @param {function} done Callback to call when the test is completed. Included
* primarily for use with mocha
*/
-function emptyStream(client, done) {
- var call = client.fullDuplexCall();
+function emptyStream(client, done, extra) {
+ var call = client.fullDuplexCall(getCallOptions(extra));
call.on('status', function(status) {
assert.strictEqual(status.code, grpc.status.OK);
if (done) {
@@ -243,8 +252,8 @@ function emptyStream(client, done) {
* @param {function} done Callback to call when the test is completed. Included
* primarily for use with mocha
*/
-function cancelAfterBegin(client, done) {
- var call = client.streamingInputCall(function(err, resp) {
+function cancelAfterBegin(client, done, extra) {
+ var call = client.streamingInputCall(getCallOptions(extra), function(err, resp) {
assert.strictEqual(err.code, grpc.status.CANCELLED);
done();
});
@@ -257,8 +266,8 @@ function cancelAfterBegin(client, done) {
* @param {function} done Callback to call when the test is completed. Included
* primarily for use with mocha
*/
-function cancelAfterFirstResponse(client, done) {
- var call = client.fullDuplexCall();
+function cancelAfterFirstResponse(client, done, extra) {
+ var call = client.fullDuplexCall(getCallOptions(extra));
call.write({
response_type: 'COMPRESSABLE',
response_parameters: [
@@ -275,10 +284,11 @@ function cancelAfterFirstResponse(client, done) {
});
}
-function timeoutOnSleepingServer(client, done) {
+function timeoutOnSleepingServer(client, done, extra) {
var deadline = new Date();
deadline.setMilliseconds(deadline.getMilliseconds() + 1);
- var call = client.fullDuplexCall({deadline: deadline});
+ var call = client.fullDuplexCall(
+ Object.assign({}, getCallOptions(extra), {deadline: deadline}));
call.write({
payload: {body: zeroBuffer(27182)}
});
@@ -291,7 +301,7 @@ function timeoutOnSleepingServer(client, done) {
});
}
-function customMetadata(client, done) {
+function customMetadata(client, done, extra) {
done = multiDone(done, 5);
var metadata = new grpc.Metadata();
metadata.set(ECHO_INITIAL_KEY, 'test_initial_metadata_value');
@@ -311,7 +321,8 @@ function customMetadata(client, done) {
body: zeroBuffer(271828)
}
};
- var unary = client.unaryCall(arg, metadata, function(err, resp) {
+ var unary = client.unaryCall(arg, metadata, getCallOptions(extra),
+ function(err, resp) {
assert.ifError(err);
done();
});
@@ -326,7 +337,7 @@ function customMetadata(client, done) {
assert.strictEqual(echo_trailer[0].toString('hex'), 'ababab');
done();
});
- var stream = client.fullDuplexCall(metadata);
+ var stream = client.fullDuplexCall(metadata, getCallOptions(extra));
stream.on('metadata', function(metadata) {
assert.deepEqual(metadata.get(ECHO_INITIAL_KEY),
['test_initial_metadata_value']);
@@ -343,7 +354,7 @@ function customMetadata(client, done) {
stream.end();
}
-function statusCodeAndMessage(client, done) {
+function statusCodeAndMessage(client, done, extra) {
done = multiDone(done, 2);
var arg = {
response_status: {
@@ -351,13 +362,13 @@ function statusCodeAndMessage(client, done) {
message: 'test status message'
}
};
- client.unaryCall(arg, function(err, resp) {
+ client.unaryCall(arg, getCallOptions(extra), function(err, resp) {
assert(err);
assert.strictEqual(err.code, 2);
assert.strictEqual(err.details, 'test status message');
done();
});
- var duplex = client.fullDuplexCall();
+ var duplex = client.fullDuplexCall(getCallOptions(extra));
duplex.on('data', function() {});
duplex.on('status', function(status) {
assert(status);
@@ -370,7 +381,7 @@ function statusCodeAndMessage(client, done) {
duplex.end();
}
-function specialStatusMessage(client, done) {
+function specialStatusMessage(client, done, extra) {
let expectedMessage = '\t\ntest with whitespace\r\nand Unicode BMP ☺ and non-BMP 😈\t\n';
let arg = {
response_status: {
@@ -378,7 +389,7 @@ function specialStatusMessage(client, done) {
message: expectedMessage
}
};
- client.unaryCall(arg, function(err, resp) {
+ client.unaryCall(arg, getCallOptions(extra), function(err, resp) {
assert(err);
assert.strictEqual(err.code, 2);
assert.strictEqual(err.details, expectedMessage);
@@ -387,8 +398,8 @@ function specialStatusMessage(client, done) {
}
// NOTE: the client param to this function is from UnimplementedService
-function unimplementedService(client, done) {
- client.unimplementedCall({}, function(err, resp) {
+function unimplementedService(client, done, extra) {
+ client.unimplementedCall({}, getCallOptions(extra), function(err, resp) {
assert(err);
assert.strictEqual(err.code, grpc.status.UNIMPLEMENTED);
done();
@@ -396,8 +407,8 @@ function unimplementedService(client, done) {
}
// NOTE: the client param to this function is from TestService
-function unimplementedMethod(client, done) {
- client.unimplementedCall({}, function(err, resp) {
+function unimplementedMethod(client, done, extra) {
+ client.unimplementedCall({}, getCallOptions(extra), function(err, resp) {
assert(err);
assert.strictEqual(err.code, grpc.status.UNIMPLEMENTED);
done();
@@ -411,8 +422,9 @@ function unimplementedMethod(client, done) {
* @param {?string} scope The scope to apply to the credentials
* @param {function} done Callback to call when the test is completed. Included
* primarily for use with mocha
+ * @param {object=} extra Extra options for the test
*/
-function authTest(expected_user, scope, client, done) {
+function authTest(expected_user, scope, client, done, extra) {
var arg = {
response_type: 'COMPRESSABLE',
response_size: 314159,
@@ -422,7 +434,7 @@ function authTest(expected_user, scope, client, done) {
fill_username: true,
fill_oauth_scope: true
};
- client.unaryCall(arg, function(err, resp) {
+ client.unaryCall(arg, getCallOptions(extra), function(err, resp) {
assert.ifError(err);
assert.strictEqual(resp.payload.type, 'COMPRESSABLE');
assert.strictEqual(resp.payload.body.length, 314159);
@@ -437,15 +449,15 @@ function authTest(expected_user, scope, client, done) {
}
function computeEngineCreds(client, done, extra) {
- authTest(extra.service_account, null, client, done);
+ authTest(extra.service_account, null, client, done, extra);
}
function serviceAccountCreds(client, done, extra) {
- authTest(SERVICE_ACCOUNT_EMAIL, extra.oauth_scope, client, done);
+ authTest(SERVICE_ACCOUNT_EMAIL, extra.oauth_scope, client, done, extra);
}
function jwtTokenCreds(client, done, extra) {
- authTest(SERVICE_ACCOUNT_EMAIL, null, client, done);
+ authTest(SERVICE_ACCOUNT_EMAIL, null, client, done, extra);
}
function oauth2Test(client, done, extra) {
@@ -453,7 +465,7 @@ function oauth2Test(client, done, extra) {
fill_username: true,
fill_oauth_scope: true
};
- client.unaryCall(arg, function(err, resp) {
+ client.unaryCall(arg, getCallOptions(extra), function(err, resp) {
assert.ifError(err);
assert.strictEqual(resp.username, SERVICE_ACCOUNT_EMAIL);
assert(extra.oauth_scope.indexOf(resp.oauth_scope) > -1);
@@ -469,7 +481,9 @@ function perRpcAuthTest(client, done, extra) {
fill_oauth_scope: true
};
const creds = grpc.credentials.createFromGoogleCredential(new GoogleAuth({scopes: extra.oauth_scope}));
- client.unaryCall(arg, {credentials: creds}, function(err, resp) {
+ client.unaryCall(
+ arg, Object.assign({}, getCallOptions(extra), {credentials: creds}),
+ function(err, resp) {
assert.ifError(err);
assert.strictEqual(resp.username, SERVICE_ACCOUNT_EMAIL);
assert(extra.oauth_scope.indexOf(resp.oauth_scope) > -1);
@@ -556,7 +570,8 @@ exports.test_cases = test_cases;
* @param {bool} tls Indicates that a secure channel should be used
* @param {function} done Callback to call when the test is completed. Included
* primarily for use with mocha
- * @param {object=} extra Extra options for some tests
+ * @param {object=} extra Extra options for some tests. The callOptions field,
+ * if set, is passed as the call options of each RPC
*/
function runTest(address, host_override, test_case, tls, test_ca, done, extra) {
// TODO(mlumish): enable TLS functionalityVerified on 6741e13 (Node.js v22.23.4, mocha 10.8.2): with { callOptions: { deadline } } as the third argument in place of { deadline }, the test above fails without this patch and passes with it; no other test results change in test/api (including the interop sanity run).
I can submit a PR for this.
Describe alternatives you've considered
- Wrapping the client to inject a default deadline into every method works, but each caller has to reimplement it.
- A client interceptor can set a deadline. That changes the client used by every case, though, rather than the options of one run.
Additional context
Related: I searched issues and PRs (interop client deadline, interop_client options, interop test metadata call options) and found nothing related. The hits (#2677, #1158, #533) are unrelated.
Activity