From c3fa981012501610d190f90ac450cbbf3483a9ae Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Mon, 11 Jul 2022 18:05:57 -0700 Subject: [PATCH 01/20] fix: return Google Error when there is a missing required parameter --- src/fallbackRest.ts | 8 ++------ src/fallbackServiceStub.ts | 20 ++++++++++++-------- src/transcoding.ts | 5 +++-- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/fallbackRest.ts b/src/fallbackRest.ts index baaa9cd74..7e4919ae1 100644 --- a/src/fallbackRest.ts +++ b/src/fallbackRest.ts @@ -61,12 +61,8 @@ export function encodeRequest( rpc.parsedOptions, rpc.resolvedRequestType!.fields ); - if (!transcoded) { - throw new Error( - `Cannot build HTTP request for ${JSON.stringify(json)}, method: ${ - rpc.name - }` - ); + if (transcoded instanceof GoogleError) { + throw transcoded; } const method = transcoded.httpMethod; const body = JSON.stringify(transcoded.data); diff --git a/src/fallbackServiceStub.ts b/src/fallbackServiceStub.ts index 78936d218..404cbf9c8 100644 --- a/src/fallbackServiceStub.ts +++ b/src/fallbackServiceStub.ts @@ -91,14 +91,18 @@ export function generateServiceStub( : new NodeAbortController(); const cancelSignal = cancelController.signal as AbortSignal; let cancelRequested = false; - - const fetchParameters = requestEncoder( - rpc, - protocol, - servicePath, - servicePort, - request - ); + let fetchParameters: FetchParameters; + try { + fetchParameters = requestEncoder( + rpc, + protocol, + servicePath, + servicePort, + request + ); + } catch (err) { + return Promise.resolve(callback(err)); + } const url = fetchParameters.url; const headers = fetchParameters.headers; for (const key of Object.keys(options)) { diff --git a/src/transcoding.ts b/src/transcoding.ts index 742ebfa00..6fdd39436 100644 --- a/src/transcoding.ts +++ b/src/transcoding.ts @@ -20,6 +20,7 @@ import {JSONObject, JSONValue} from 'proto3-json-serializer'; import {Field} from 'protobufjs'; import {google} from '../protos/http'; +import { GoogleError } from './googleError'; import {camelToSnakeCase, toCamelCase as snakeToCamelCase} from './util'; export interface TranscodedRequest { @@ -297,13 +298,13 @@ export function transcode( request: JSONObject, parsedOptions: ParsedOptionsType, requestFields?: {[k: string]: Field} -): TranscodedRequest | undefined { +): TranscodedRequest | GoogleError { const {requiredFields, optionalFields} = getFieldNameOnBehavior(requestFields); // all fields annotated as REQUIRED MUST be emitted in the body. for (const requiredField of requiredFields) { if (!(requiredField in request) || request[requiredField] === 'undefined') { - throw new Error( + return new GoogleError( `Required field ${requiredField} is not present in the request.` ); } From 13cb3ede3a82b186dc30f1683695596a2c8a96cb Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Mon, 11 Jul 2022 18:16:27 -0700 Subject: [PATCH 02/20] allow for undefined --- src/fallbackRest.ts | 8 ++++++++ src/transcoding.ts | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/fallbackRest.ts b/src/fallbackRest.ts index 7e4919ae1..6a2fb596e 100644 --- a/src/fallbackRest.ts +++ b/src/fallbackRest.ts @@ -64,6 +64,14 @@ export function encodeRequest( if (transcoded instanceof GoogleError) { throw transcoded; } + + if (!transcoded) { + throw new Error( + `Cannot build HTTP request for ${JSON.stringify(json)}, method: ${ + rpc.name + }` + ); + } const method = transcoded.httpMethod; const body = JSON.stringify(transcoded.data); const url = `${protocol}://${servicePath}:${servicePort}/${transcoded.url.replace( diff --git a/src/transcoding.ts b/src/transcoding.ts index 6fdd39436..361738aea 100644 --- a/src/transcoding.ts +++ b/src/transcoding.ts @@ -298,7 +298,7 @@ export function transcode( request: JSONObject, parsedOptions: ParsedOptionsType, requestFields?: {[k: string]: Field} -): TranscodedRequest | GoogleError { +): TranscodedRequest | GoogleError | undefined { const {requiredFields, optionalFields} = getFieldNameOnBehavior(requestFields); // all fields annotated as REQUIRED MUST be emitted in the body. From 19e1618299f241ad8e50df5ac6e6ba2cd4e1a167 Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Mon, 11 Jul 2022 18:50:28 -0700 Subject: [PATCH 03/20] fix: add import --- src/fallbackServiceStub.ts | 3 ++- test/unit/transcoding.ts | 28 ++++++++++++++++++++-------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/fallbackServiceStub.ts b/src/fallbackServiceStub.ts index 404cbf9c8..8d20e5b52 100644 --- a/src/fallbackServiceStub.ts +++ b/src/fallbackServiceStub.ts @@ -25,6 +25,7 @@ import {hasWindowFetch, hasAbortController} from './featureDetection'; import {AuthClient} from './fallback'; import {StreamArrayParser} from './streamArrayParser'; import {pipeline, PipelineSource} from 'stream'; +import {GoogleError} from './googleError'; interface NodeFetchType { (url: RequestInfo, init?: RequestInit): Promise; @@ -37,7 +38,7 @@ export interface FallbackServiceStub { options: {}, metadata: {}, callback: (err?: Error, response?: {} | undefined) => void - ) => StreamArrayParser | {cancel: () => void}; + ) => StreamArrayParser | {cancel: () => void} | Promise; } export interface FetchParameters { diff --git a/test/unit/transcoding.ts b/test/unit/transcoding.ts index 98ec4f5a3..01b6e9c76 100644 --- a/test/unit/transcoding.ts +++ b/test/unit/transcoding.ts @@ -32,6 +32,7 @@ import { buildQueryStringComponents, requestChangeCaseAndCleanup, overrideHttpRules, + TranscodedRequest, } from '../../src/transcoding'; import * as assert from 'assert'; import { @@ -594,7 +595,7 @@ describe('validate proto3 field with default value', () => { ]; const transcoded = transcode(request, parsedOptions, badTestMessageFields); assert.deepStrictEqual( - transcoded?.url, + (transcoded as TranscodedRequest)?.url, 'projects/test-project/contents/test-content' ); }); @@ -640,8 +641,13 @@ describe('validate proto3 field with default value', () => { }, ]; const transcoded = transcode(request, parsedOptions, testMessageFields); - assert.deepStrictEqual(transcoded?.url, 'projects/test-project'); - assert.deepStrictEqual(transcoded?.data, {content: 'test-content'}); + assert.deepStrictEqual( + (transcoded as TranscodedRequest)?.url, + 'projects/test-project' + ); + assert.deepStrictEqual((transcoded as TranscodedRequest)?.data, { + content: 'test-content', + }); }); it('when body="*", unset optional field should remove from body', () => { const requests: RequestType[] = [ @@ -666,10 +672,10 @@ describe('validate proto3 field with default value', () => { for (const request of requests) { const transcoded = transcode(request, parsedOptions, testMessageFields); assert.deepStrictEqual( - transcoded?.url, + (transcoded as TranscodedRequest)?.url, 'projects/test-project/contents/test-content' ); - assert.deepStrictEqual(transcoded?.data, {}); + assert.deepStrictEqual((transcoded as TranscodedRequest)?.data, {}); } }); it('unset optional fields should not appear in query params', () => { @@ -697,9 +703,15 @@ describe('validate proto3 field with default value', () => { ]; for (const request of requests) { const transcoded = transcode(request, parsedOptions, testMessageFields); - assert.deepStrictEqual(transcoded?.url, 'projects/test-project'); - assert.deepStrictEqual(transcoded?.data, 'test-content'); - assert.deepStrictEqual(transcoded.queryString, ''); + assert.deepStrictEqual( + (transcoded as TranscodedRequest)?.url, + 'projects/test-project' + ); + assert.deepStrictEqual( + (transcoded as TranscodedRequest)?.data, + 'test-content' + ); + assert.deepStrictEqual((transcoded as TranscodedRequest).queryString, ''); } }); }); From 170450030addc885b78fb8498f62b7915067f00b Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Mon, 11 Jul 2022 18:54:17 -0700 Subject: [PATCH 04/20] add type --- test/unit/grpc-fallback.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/unit/grpc-fallback.ts b/test/unit/grpc-fallback.ts index d806271e6..543b4b1c9 100644 --- a/test/unit/grpc-fallback.ts +++ b/test/unit/grpc-fallback.ts @@ -28,6 +28,7 @@ import * as sinon from 'sinon'; import {echoProtoJson} from '../fixtures/echoProtoJson'; import {GrpcClient} from '../../src/fallback'; import {GoogleError} from '../../src'; +import {StreamArrayParser} from '../../src/streamArrayParser'; // @ts-ignore const hasAbortController = typeof AbortController !== 'undefined'; @@ -402,7 +403,7 @@ describe('grpc-fallback', () => { const request = {content: 'content' + new Date().toString()}; const call = echoStub.echo(request, {}, {}, () => {}); - call.cancel(); + (call as StreamArrayParser).cancel(); // @ts-ignore assert.strictEqual(createdAbortControllers[0].abortCalled, true); From 606a5ae45e37413a533693ef006f2e4fbb36d037 Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Mon, 11 Jul 2022 18:57:05 -0700 Subject: [PATCH 05/20] fix type --- test/browser-test/test.grpc-fallback.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/browser-test/test.grpc-fallback.ts b/test/browser-test/test.grpc-fallback.ts index 9858e78d6..17aa01e10 100644 --- a/test/browser-test/test.grpc-fallback.ts +++ b/test/browser-test/test.grpc-fallback.ts @@ -23,6 +23,7 @@ import * as protobuf from 'protobufjs'; import * as sinon from 'sinon'; import * as fallback from '../../src/fallback'; +import { StreamArrayParser } from '../../src/streamArrayParser'; import {echoProtoJson} from '../fixtures/echoProtoJson'; import {EchoClient} from '../fixtures/google-gax-packaging-test-app/src/v1beta1/echo_client'; @@ -220,7 +221,7 @@ describe('grpc-fallback', () => { const request = {content: 'content' + new Date().toString()}; const call = echoStub.echo(request, {}, {}, () => {}); - call.cancel(); + (call as StreamArrayParser).cancel(); // @ts-ignore assert.strictEqual(createdAbortControllers[0].abortCalled, true); From 33b65ce802aa227d1aa08adad4ece309852069d8 Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Mon, 11 Jul 2022 18:59:29 -0700 Subject: [PATCH 06/20] run lint --- src/transcoding.ts | 2 +- test/browser-test/test.grpc-fallback.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/transcoding.ts b/src/transcoding.ts index 361738aea..bf8c2ced2 100644 --- a/src/transcoding.ts +++ b/src/transcoding.ts @@ -20,7 +20,7 @@ import {JSONObject, JSONValue} from 'proto3-json-serializer'; import {Field} from 'protobufjs'; import {google} from '../protos/http'; -import { GoogleError } from './googleError'; +import {GoogleError} from './googleError'; import {camelToSnakeCase, toCamelCase as snakeToCamelCase} from './util'; export interface TranscodedRequest { diff --git a/test/browser-test/test.grpc-fallback.ts b/test/browser-test/test.grpc-fallback.ts index 17aa01e10..5a3cbdd98 100644 --- a/test/browser-test/test.grpc-fallback.ts +++ b/test/browser-test/test.grpc-fallback.ts @@ -23,7 +23,7 @@ import * as protobuf from 'protobufjs'; import * as sinon from 'sinon'; import * as fallback from '../../src/fallback'; -import { StreamArrayParser } from '../../src/streamArrayParser'; +import {StreamArrayParser} from '../../src/streamArrayParser'; import {echoProtoJson} from '../fixtures/echoProtoJson'; import {EchoClient} from '../fixtures/google-gax-packaging-test-app/src/v1beta1/echo_client'; From 5da7e401107ef1969dc4b4287aaf5a89a14a5ed4 Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Mon, 11 Jul 2022 19:39:26 -0700 Subject: [PATCH 07/20] add transcoding test --- test/unit/transcoding.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/unit/transcoding.ts b/test/unit/transcoding.ts index 01b6e9c76..5d37cf333 100644 --- a/test/unit/transcoding.ts +++ b/test/unit/transcoding.ts @@ -43,6 +43,7 @@ import * as protobuf from 'protobufjs'; import {testMessageJson} from '../fixtures/fallbackOptional'; import {echoProtoJson} from '../fixtures/echoProtoJson'; import {google} from '../../protos/http'; +import {GoogleError} from 'google-gax'; describe('gRPC to HTTP transcoding', () => { const parsedOptions: ParsedOptionsType = [ @@ -144,6 +145,14 @@ describe('gRPC to HTTP transcoding', () => { } ); + assert.ok( + transcode( + {parent: 'get/project', field: 'value', a: 42}, + parsedOptions, + undefined + ) instanceof GoogleError + ); + assert.deepStrictEqual( transcode({parent: 'get/project', field: 'value', a: 42}, parsedOptions), { From ff333e84a9655798e960bb5e84e93be2ae82e35e Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Mon, 11 Jul 2022 19:42:17 -0700 Subject: [PATCH 08/20] add type --- test/unit/transcoding.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/transcoding.ts b/test/unit/transcoding.ts index 5d37cf333..a3fa3f791 100644 --- a/test/unit/transcoding.ts +++ b/test/unit/transcoding.ts @@ -43,7 +43,7 @@ import * as protobuf from 'protobufjs'; import {testMessageJson} from '../fixtures/fallbackOptional'; import {echoProtoJson} from '../fixtures/echoProtoJson'; import {google} from '../../protos/http'; -import {GoogleError} from 'google-gax'; +import {GoogleError} from '../../src'; describe('gRPC to HTTP transcoding', () => { const parsedOptions: ParsedOptionsType = [ From bcd3616cfa2e758c4b2070d6de08c4692f7a2cb7 Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Mon, 11 Jul 2022 19:49:30 -0700 Subject: [PATCH 09/20] fix test --- test/unit/transcoding.ts | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/test/unit/transcoding.ts b/test/unit/transcoding.ts index a3fa3f791..8b28173bb 100644 --- a/test/unit/transcoding.ts +++ b/test/unit/transcoding.ts @@ -145,14 +145,6 @@ describe('gRPC to HTTP transcoding', () => { } ); - assert.ok( - transcode( - {parent: 'get/project', field: 'value', a: 42}, - parsedOptions, - undefined - ) instanceof GoogleError - ); - assert.deepStrictEqual( transcode({parent: 'get/project', field: 'value', a: 42}, parsedOptions), { @@ -630,9 +622,9 @@ describe('validate proto3 field with default value', () => { }, ]; for (const request of requests) { - assert.throws( - () => transcode(request, parsedOptions, testMessageFields), - Error + assert.ok( + transcode(request, parsedOptions, testMessageFields) instanceof + GoogleError ); } }); From 753f17ea01037b2fa10974348303725f263c5369 Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Tue, 12 Jul 2022 13:06:37 -0700 Subject: [PATCH 10/20] respond to comments --- src/fallbackRest.ts | 15 +++++++++------ src/transcoding.ts | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/fallbackRest.ts b/src/fallbackRest.ts index 6a2fb596e..56941c8ca 100644 --- a/src/fallbackRest.ts +++ b/src/fallbackRest.ts @@ -56,12 +56,15 @@ export function encodeRequest( if (typeof json !== 'object' || Array.isArray(json)) { throw new Error(`Request to RPC ${rpc.name} must be an object.`); } - const transcoded = transcode( - json, - rpc.parsedOptions, - rpc.resolvedRequestType!.fields - ); - if (transcoded instanceof GoogleError) { + + let transcoded; + try { + transcoded = transcode( + json, + rpc.parsedOptions, + rpc.resolvedRequestType!.fields + ); + } catch (err) { throw transcoded; } diff --git a/src/transcoding.ts b/src/transcoding.ts index bf8c2ced2..5c3e15548 100644 --- a/src/transcoding.ts +++ b/src/transcoding.ts @@ -304,7 +304,7 @@ export function transcode( // all fields annotated as REQUIRED MUST be emitted in the body. for (const requiredField of requiredFields) { if (!(requiredField in request) || request[requiredField] === 'undefined') { - return new GoogleError( + throw new GoogleError( `Required field ${requiredField} is not present in the request.` ); } From aac6f49da216a06bab8449ffc42b32f96e772f3c Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Tue, 12 Jul 2022 13:34:07 -0700 Subject: [PATCH 11/20] remove return GoogleError --- src/transcoding.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transcoding.ts b/src/transcoding.ts index 5c3e15548..dc149dba4 100644 --- a/src/transcoding.ts +++ b/src/transcoding.ts @@ -298,7 +298,7 @@ export function transcode( request: JSONObject, parsedOptions: ParsedOptionsType, requestFields?: {[k: string]: Field} -): TranscodedRequest | GoogleError | undefined { +): TranscodedRequest | undefined { const {requiredFields, optionalFields} = getFieldNameOnBehavior(requestFields); // all fields annotated as REQUIRED MUST be emitted in the body. From 4f80ab400fbca99b92518675713646d6c1038729 Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Tue, 12 Jul 2022 13:42:43 -0700 Subject: [PATCH 12/20] assert transcode throws --- test/unit/transcoding.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unit/transcoding.ts b/test/unit/transcoding.ts index 8b28173bb..768817e8f 100644 --- a/test/unit/transcoding.ts +++ b/test/unit/transcoding.ts @@ -622,9 +622,9 @@ describe('validate proto3 field with default value', () => { }, ]; for (const request of requests) { - assert.ok( - transcode(request, parsedOptions, testMessageFields) instanceof - GoogleError + assert.throws( + () => transcode(request, parsedOptions, testMessageFields), + /Error: Required field content is not present in the request/ ); } }); From 414379ac5b2950b7a7a7028fb8a849d8da5ddec5 Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Tue, 12 Jul 2022 15:32:39 -0700 Subject: [PATCH 13/20] add test --- test/unit/grpc-fallback.ts | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/unit/grpc-fallback.ts b/test/unit/grpc-fallback.ts index 543b4b1c9..092df2090 100644 --- a/test/unit/grpc-fallback.ts +++ b/test/unit/grpc-fallback.ts @@ -328,6 +328,31 @@ describe('grpc-fallback', () => { }); }); + it.only('should handle an error occurring during encoding', done => { + const requestObject = {content: 'test-content'}; + const expectedMessage = + 'Required field ${requiredField} is not present in the request.'; + + //@ts-ignore + sinon.stub(nodeFetch, 'Promise').returns( + Promise.resolve({ + ok: false, + arrayBuffer: () => { + return Promise.resolve( + 'Required field ${requiredField} is not present in the request.' + ); + }, + }) + ); + gaxGrpc.createStub(echoService, stubOptions).then(echoStub => { + echoStub.echo(requestObject, {}, {}, (err?: Error) => { + assert(err instanceof GoogleError); + assert.strictEqual(err.message, expectedMessage); + done(); + }); + }); + }); + it('should promote ErrorInfo if exist in fallback-rest error', done => { const requestObject = {content: 'test-content'}; // example of an actual google.rpc.Status error message returned by Translate API From 76db51f9c7dfbd5015d9f037ae9523cdaaeb4144 Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Tue, 12 Jul 2022 15:38:33 -0700 Subject: [PATCH 14/20] remove only --- test/unit/grpc-fallback.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/grpc-fallback.ts b/test/unit/grpc-fallback.ts index 092df2090..9aeb6a851 100644 --- a/test/unit/grpc-fallback.ts +++ b/test/unit/grpc-fallback.ts @@ -328,7 +328,7 @@ describe('grpc-fallback', () => { }); }); - it.only('should handle an error occurring during encoding', done => { + it('should handle an error occurring during encoding', done => { const requestObject = {content: 'test-content'}; const expectedMessage = 'Required field ${requiredField} is not present in the request.'; From ea975facd6f61dc0a1c2c99bc982a43bd89e2030 Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Tue, 12 Jul 2022 15:47:45 -0700 Subject: [PATCH 15/20] rerun tests --- test/unit/grpc-fallback.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/unit/grpc-fallback.ts b/test/unit/grpc-fallback.ts index 9aeb6a851..933ee7318 100644 --- a/test/unit/grpc-fallback.ts +++ b/test/unit/grpc-fallback.ts @@ -339,7 +339,9 @@ describe('grpc-fallback', () => { ok: false, arrayBuffer: () => { return Promise.resolve( - 'Required field ${requiredField} is not present in the request.' + Buffer.from( + 'Required field ${requiredField} is not present in the request.' + ) ); }, }) From 453f96be2750209c783522fd761b2643badee5b8 Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Tue, 12 Jul 2022 16:32:14 -0700 Subject: [PATCH 16/20] fix test --- test/unit/grpc-fallback.ts | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/test/unit/grpc-fallback.ts b/test/unit/grpc-fallback.ts index 933ee7318..c892b9774 100644 --- a/test/unit/grpc-fallback.ts +++ b/test/unit/grpc-fallback.ts @@ -29,6 +29,7 @@ import {echoProtoJson} from '../fixtures/echoProtoJson'; import {GrpcClient} from '../../src/fallback'; import {GoogleError} from '../../src'; import {StreamArrayParser} from '../../src/streamArrayParser'; +import * as fallbackServiceStub from '../../src/fallbackServiceStub'; // @ts-ignore const hasAbortController = typeof AbortController !== 'undefined'; @@ -334,18 +335,14 @@ describe('grpc-fallback', () => { 'Required field ${requiredField} is not present in the request.'; //@ts-ignore - sinon.stub(nodeFetch, 'Promise').returns( - Promise.resolve({ - ok: false, - arrayBuffer: () => { - return Promise.resolve( - Buffer.from( - 'Required field ${requiredField} is not present in the request.' - ) - ); - }, - }) - ); + sinon + .stub(fallbackServiceStub, 'generateServiceStub') + .throws( + new GoogleError( + 'Required field ${requiredField} is not present in the request.' + ) + ); + gaxGrpc.createStub(echoService, stubOptions).then(echoStub => { echoStub.echo(requestObject, {}, {}, (err?: Error) => { assert(err instanceof GoogleError); From 2695c410362efc4a436f8b334c1d5dbdd5def786 Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Tue, 12 Jul 2022 16:39:05 -0700 Subject: [PATCH 17/20] rejects promise --- test/unit/grpc-fallback.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/grpc-fallback.ts b/test/unit/grpc-fallback.ts index c892b9774..a1069e731 100644 --- a/test/unit/grpc-fallback.ts +++ b/test/unit/grpc-fallback.ts @@ -337,7 +337,7 @@ describe('grpc-fallback', () => { //@ts-ignore sinon .stub(fallbackServiceStub, 'generateServiceStub') - .throws( + .rejects( new GoogleError( 'Required field ${requiredField} is not present in the request.' ) From 206b7c4278ba5117e0f1b371d568d23d4f79cc2c Mon Sep 17 00:00:00 2001 From: Sofia Leon Date: Wed, 13 Jul 2022 11:02:02 -0700 Subject: [PATCH 18/20] revert grpc-fallback changes --- test/unit/grpc-fallback.ts | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/test/unit/grpc-fallback.ts b/test/unit/grpc-fallback.ts index a1069e731..543b4b1c9 100644 --- a/test/unit/grpc-fallback.ts +++ b/test/unit/grpc-fallback.ts @@ -29,7 +29,6 @@ import {echoProtoJson} from '../fixtures/echoProtoJson'; import {GrpcClient} from '../../src/fallback'; import {GoogleError} from '../../src'; import {StreamArrayParser} from '../../src/streamArrayParser'; -import * as fallbackServiceStub from '../../src/fallbackServiceStub'; // @ts-ignore const hasAbortController = typeof AbortController !== 'undefined'; @@ -329,29 +328,6 @@ describe('grpc-fallback', () => { }); }); - it('should handle an error occurring during encoding', done => { - const requestObject = {content: 'test-content'}; - const expectedMessage = - 'Required field ${requiredField} is not present in the request.'; - - //@ts-ignore - sinon - .stub(fallbackServiceStub, 'generateServiceStub') - .rejects( - new GoogleError( - 'Required field ${requiredField} is not present in the request.' - ) - ); - - gaxGrpc.createStub(echoService, stubOptions).then(echoStub => { - echoStub.echo(requestObject, {}, {}, (err?: Error) => { - assert(err instanceof GoogleError); - assert.strictEqual(err.message, expectedMessage); - done(); - }); - }); - }); - it('should promote ErrorInfo if exist in fallback-rest error', done => { const requestObject = {content: 'test-content'}; // example of an actual google.rpc.Status error message returned by Translate API From 42f07f8ddb27a1627bb6dbe36a5d2dbd8a2af757 Mon Sep 17 00:00:00 2001 From: Alexander Fenster Date: Wed, 17 Aug 2022 16:31:18 -0700 Subject: [PATCH 19/20] fix: simplify promises and callbacks --- src/fallbackRest.ts | 15 +++++---------- src/fallbackServiceStub.ts | 20 +++++++++++++------- src/index.ts | 1 - test/browser-test/test/test.grpc-fallback.ts | 4 ++-- test/unit/grpc-fallback.ts | 3 +-- 5 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/fallbackRest.ts b/src/fallbackRest.ts index 8ab824b7e..2f5b7022e 100644 --- a/src/fallbackRest.ts +++ b/src/fallbackRest.ts @@ -46,16 +46,11 @@ export function encodeRequest( throw new Error(`Request to RPC ${rpc.name} must be an object.`); } - let transcoded; - try { - transcoded = transcode( - json, - rpc.parsedOptions, - rpc.resolvedRequestType!.fields - ); - } catch (err) { - throw transcoded; - } + const transcoded = transcode( + json, + rpc.parsedOptions, + rpc.resolvedRequestType!.fields + ); if (!transcoded) { throw new Error( diff --git a/src/fallbackServiceStub.ts b/src/fallbackServiceStub.ts index b0f468a98..e9ab8432c 100644 --- a/src/fallbackServiceStub.ts +++ b/src/fallbackServiceStub.ts @@ -38,7 +38,7 @@ export interface FallbackServiceStub { options: {}, metadata: {}, callback: (err?: Error, response?: {} | undefined) => void - ) => StreamArrayParser | {cancel: () => void} | Promise; + ) => StreamArrayParser | {cancel: () => void}; } export interface FetchParameters { @@ -87,11 +87,6 @@ export function generateServiceStub( // We cannot use async-await in this function because we need to return the canceller object as soon as possible. // Using plain old promises instead. - const cancelController = hasAbortController() - ? new AbortController() - : new NodeAbortController(); - const cancelSignal = cancelController.signal as AbortSignal; - let cancelRequested = false; let fetchParameters: FetchParameters; try { fetchParameters = requestEncoder( @@ -102,8 +97,19 @@ export function generateServiceStub( request ); } catch (err) { - return Promise.resolve(callback(err)); + // we could not encode parameters; pass error to the callback + // and return a no-op canceler object. + callback(err); + return { + cancel() {} + }; } + + const cancelController = hasAbortController() + ? new AbortController() + : new NodeAbortController(); + const cancelSignal = cancelController.signal as AbortSignal; + let cancelRequested = false; const url = fetchParameters.url; const headers = fetchParameters.headers; for (const key of Object.keys(options)) { diff --git a/src/index.ts b/src/index.ts index 1585fab60..aca9bdfb0 100644 --- a/src/index.ts +++ b/src/index.ts @@ -112,4 +112,3 @@ export {warn} from './warnings'; import * as serializer from 'proto3-json-serializer'; export {serializer}; -export {StreamArrayParser} from './streamArrayParser'; diff --git a/test/browser-test/test/test.grpc-fallback.ts b/test/browser-test/test/test.grpc-fallback.ts index 20621206c..d9c9133b3 100644 --- a/test/browser-test/test/test.grpc-fallback.ts +++ b/test/browser-test/test/test.grpc-fallback.ts @@ -19,7 +19,7 @@ import * as assert from 'assert'; import {after, afterEach, before, beforeEach, describe, it} from 'mocha'; import * as sinon from 'sinon'; -import {protobuf, GoogleAuth, fallback, StreamArrayParser} from 'google-gax'; +import {protobuf, GoogleAuth, fallback} from 'google-gax'; import {EchoClient} from 'showcase-echo-client'; import echoProtoJson = require('showcase-echo-client/build/protos/protos.json'); @@ -216,7 +216,7 @@ describe('grpc-fallback', () => { const request = {content: 'content' + new Date().toString()}; const call = echoStub.echo(request, {}, {}, () => {}); - (call as StreamArrayParser).cancel(); + call.cancel(); // @ts-ignore assert.strictEqual(createdAbortControllers[0].abortCalled, true); diff --git a/test/unit/grpc-fallback.ts b/test/unit/grpc-fallback.ts index 526496603..e386c5eb6 100644 --- a/test/unit/grpc-fallback.ts +++ b/test/unit/grpc-fallback.ts @@ -28,7 +28,6 @@ import * as sinon from 'sinon'; import echoProtoJson = require('../fixtures/echo.json'); import {GrpcClient} from '../../src/fallback'; import {GoogleError} from '../../src'; -import {StreamArrayParser} from '../../src/streamArrayParser'; // @ts-ignore const hasAbortController = typeof AbortController !== 'undefined'; @@ -415,7 +414,7 @@ describe('grpc-fallback', () => { const request = {content: 'content' + new Date().toString()}; const call = echoStub.echo(request, {}, {}, () => {}); - (call as StreamArrayParser).cancel(); + call.cancel(); // @ts-ignore assert.strictEqual(createdAbortControllers[0].abortCalled, true); From 96330689f1ed8f2aa3831041be2b866b5fad0050 Mon Sep 17 00:00:00 2001 From: Alexander Fenster Date: Wed, 17 Aug 2022 16:45:40 -0700 Subject: [PATCH 20/20] fix: lint --- src/fallbackServiceStub.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/fallbackServiceStub.ts b/src/fallbackServiceStub.ts index e9ab8432c..658bb7cf5 100644 --- a/src/fallbackServiceStub.ts +++ b/src/fallbackServiceStub.ts @@ -25,7 +25,6 @@ import {hasWindowFetch, hasAbortController} from './featureDetection'; import {AuthClient} from './fallback'; import {StreamArrayParser} from './streamArrayParser'; import {pipeline, PipelineSource} from 'stream'; -import {GoogleError} from './googleError'; interface NodeFetchType { (url: RequestInfo, init?: RequestInit): Promise; @@ -101,7 +100,7 @@ export function generateServiceStub( // and return a no-op canceler object. callback(err); return { - cancel() {} + cancel() {}, }; }