Skip to content

Commit

Permalink
fix(nice-grpc): fix server signal abort on call end (#229)
Browse files Browse the repository at this point in the history
Closes #218
  • Loading branch information
aikoven committed Nov 2, 2022
1 parent a3a3f79 commit 04c1713
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 6 deletions.
2 changes: 1 addition & 1 deletion packages/nice-grpc/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"ts-proto": "^1.112.0"
},
"dependencies": {
"@grpc/grpc-js": "^1.6.1",
"@grpc/grpc-js": "^1.7.3",
"abort-controller-x": "^0.4.0",
"nice-grpc-common": "^2.0.0"
}
Expand Down
20 changes: 18 additions & 2 deletions packages/nice-grpc/src/__tests__/bidiStreaming.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,15 @@ import {throwUnimplemented} from './utils/throwUnimplemented';
test('basic', async () => {
const server = createServer();

let serverSignal: AbortSignal;

server.add(TestService, {
testUnary: throwUnimplemented,
testServerStream: throwUnimplemented,
testClientStream: throwUnimplemented,
async *testBidiStream(request: AsyncIterable<TestRequest>) {
async *testBidiStream(request: AsyncIterable<TestRequest>, context) {
serverSignal = context.signal;

for await (const req of request) {
yield new TestResponse().setId(req.getId());
}
Expand Down Expand Up @@ -55,6 +59,7 @@ test('basic', async () => {
},
]
`);
expect(serverSignal!.aborted).toBe(false);

channel.close();

Expand Down Expand Up @@ -185,11 +190,14 @@ test('metadata', async () => {
test('error', async () => {
const server = createServer();

let serverSignal: AbortSignal;

server.add(TestService, {
testUnary: throwUnimplemented,
testServerStream: throwUnimplemented,
testClientStream: throwUnimplemented,
async *testBidiStream(request: AsyncIterable<TestRequest>, context) {
serverSignal = context.signal;
context.trailer.set('test', ['test-value-1', 'test-value-2']);

for await (const item of request) {
Expand Down Expand Up @@ -249,6 +257,8 @@ test('error', async () => {

await requestIterableFinish.promise;

expect(serverSignal!.aborted).toBe(false);

expect(trailer?.getAll('test')).toMatchInlineSnapshot(`
Array [
"test-value-1, test-value-2",
Expand Down Expand Up @@ -357,11 +367,15 @@ test('cancel', async () => {
test('early response', async () => {
const server = createServer();

let serverSignal: AbortSignal;

server.add(TestService, {
testUnary: throwUnimplemented,
testServerStream: throwUnimplemented,
testClientStream: throwUnimplemented,
async *testBidiStream(request: AsyncIterable<TestRequest>) {
async *testBidiStream(request: AsyncIterable<TestRequest>, context) {
serverSignal = context.signal;

for await (const item of request) {
yield new TestResponse().setId(item.getId());
return;
Expand Down Expand Up @@ -415,6 +429,8 @@ test('early response', async () => {

await requestIterableFinish.promise;

expect(serverSignal!.aborted).toBe(false);

channel.close();

await server.shutdown();
Expand Down
20 changes: 18 additions & 2 deletions packages/nice-grpc/src/__tests__/clientStreaming.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,14 @@ import {throwUnimplemented} from './utils/throwUnimplemented';
test('basic', async () => {
const server = createServer();

let serverSignal: AbortSignal;

server.add(TestService, {
testUnary: throwUnimplemented,
testServerStream: throwUnimplemented,
async testClientStream(request: AsyncIterable<TestRequest>) {
async testClientStream(request: AsyncIterable<TestRequest>, context) {
serverSignal = context.signal;

const requests: TestRequest[] = [];

for await (const req of request) {
Expand Down Expand Up @@ -52,6 +56,7 @@ test('basic', async () => {
"id": "test-1 test-2",
}
`);
expect(serverSignal!.aborted).toBe(false);

channel.close();

Expand Down Expand Up @@ -172,10 +177,13 @@ test('metadata', async () => {
test('error', async () => {
const server = createServer();

let serverSignal: AbortSignal;

server.add(TestService, {
testUnary: throwUnimplemented,
testServerStream: throwUnimplemented,
async testClientStream(request: AsyncIterable<TestRequest>, context) {
serverSignal = context.signal;
context.trailer.set('test', ['test-value-1', 'test-value-2']);

for await (const item of request) {
Expand Down Expand Up @@ -226,6 +234,8 @@ test('error', async () => {

await requestIterableFinish.promise;

expect(serverSignal!.aborted).toBe(false);

expect(trailer?.getAll('test')).toMatchInlineSnapshot(`
Array [
"test-value-1, test-value-2",
Expand Down Expand Up @@ -313,10 +323,14 @@ test('cancel', async () => {
test('early response', async () => {
const server = createServer();

let serverSignal: AbortSignal;

server.add(TestService, {
testUnary: throwUnimplemented,
testServerStream: throwUnimplemented,
async testClientStream(request: AsyncIterable<TestRequest>) {
async testClientStream(request: AsyncIterable<TestRequest>, context) {
serverSignal = context.signal;

for await (const item of request) {
return new TestResponse().setId(item.getId());
}
Expand Down Expand Up @@ -361,6 +375,8 @@ test('early response', async () => {

await requestIterableFinish.promise;

expect(serverSignal!.aborted).toBe(false);

channel.close();

await server.shutdown();
Expand Down
9 changes: 9 additions & 0 deletions packages/nice-grpc/src/__tests__/serverStreaming.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,12 @@ import {throwUnimplemented} from './utils/throwUnimplemented';
test('basic', async () => {
const server = createServer();

let serverSignal: AbortSignal;

server.add(TestService, {
async *testServerStream(request: TestRequest, context) {
serverSignal = context.signal;

yield new TestResponse().setId(`${request.getId()}-1`);
yield new TestResponse().setId(`${request.getId()}-2`);
},
Expand Down Expand Up @@ -51,6 +55,7 @@ test('basic', async () => {
},
]
`);
expect(serverSignal!.aborted).toBe(false);

channel.close();

Expand Down Expand Up @@ -177,9 +182,12 @@ test('metadata', async () => {
test('error', async () => {
const server = createServer();

let serverSignal: AbortSignal;

server.add(TestService, {
testUnary: throwUnimplemented,
async *testServerStream(request: TestRequest, context) {
serverSignal = context.signal;
yield new TestResponse().setId(request.getId());

context.trailer.set('test', 'test-value');
Expand Down Expand Up @@ -228,6 +236,7 @@ test('error', async () => {
},
]
`);
expect(serverSignal!.aborted).toBe(false);

expect(trailer?.getAll('test')).toMatchInlineSnapshot(`
Array [
Expand Down
11 changes: 10 additions & 1 deletion packages/nice-grpc/src/__tests__/unary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,12 @@ import {throwUnimplemented} from './utils/throwUnimplemented';
test('basic', async () => {
const server = createServer();

let serverSignal: AbortSignal;

server.add(TestService, {
async testUnary(request: TestRequest) {
async testUnary(request: TestRequest, context) {
serverSignal = context.signal;

return new TestResponse().setId(request.getId());
},
testServerStream: throwUnimplemented,
Expand All @@ -38,6 +42,7 @@ test('basic', async () => {
"id": "test",
}
`);
expect(serverSignal!.aborted).toBe(false);

channel.close();

Expand Down Expand Up @@ -154,8 +159,11 @@ test('metadata', async () => {
test('error', async () => {
const server = createServer();

let serverSignal: AbortSignal;

server.add(TestService, {
async testUnary(request: TestRequest, context) {
serverSignal = context.signal;
context.trailer.set('test', ['test-value-1', 'test-value-2']);
throw new ServerError(Status.NOT_FOUND, request.getId());
},
Expand All @@ -182,6 +190,7 @@ test('error', async () => {
).rejects.toMatchInlineSnapshot(
`[ClientError: /nice_grpc.test.Test/TestUnary NOT_FOUND: test]`,
);
expect(serverSignal!.aborted).toBe(false);

expect(trailer?.getAll('test')).toMatchInlineSnapshot(`
Array [
Expand Down
8 changes: 8 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,14 @@
"@grpc/proto-loader" "^0.7.0"
"@types/node" ">=12.12.47"

"@grpc/grpc-js@^1.7.3":
version "1.7.3"
resolved "https://registry.yarnpkg.com/@grpc/grpc-js/-/grpc-js-1.7.3.tgz#f2ea79f65e31622d7f86d4b4c9ae38f13ccab99a"
integrity sha512-H9l79u4kJ2PVSxUNA08HMYAnUBLj9v6KjYQ7SQ71hOZcEXhShE/y5iQCesP8+6/Ik/7i2O0a10bPquIcYfufog==
dependencies:
"@grpc/proto-loader" "^0.7.0"
"@types/node" ">=12.12.47"

"@grpc/proto-loader@^0.7.0":
version "0.7.0"
resolved "https://registry.yarnpkg.com/@grpc/proto-loader/-/proto-loader-0.7.0.tgz#743cc8a941cc251620c66ebe0d330e1411a33535"
Expand Down

0 comments on commit 04c1713

Please sign in to comment.