From 4266f43922d0d582b8eced11f4a21c98a8b451fe Mon Sep 17 00:00:00 2001 From: Alexander Fenster Date: Thu, 13 Jul 2023 10:18:06 -0700 Subject: [PATCH] fix: replace proto-over-HTTP with REGAPIC (#1471) --- client-libraries.md | 19 ++--- src/fallback.ts | 35 ++++----- src/fallbackProto.ts | 71 ------------------- src/paginationCalls/pageDescriptor.ts | 12 +++- src/streamArrayParser.ts | 2 +- test/browser-test/karma.conf.js | 4 +- test/browser-test/package.json | 3 + test/browser-test/test/test.endtoend.ts | 7 +- test/browser-test/test/test.grpc-fallback.ts | 28 ++++---- test/showcase-echo-client/package.json | 4 +- .../src/v1beta1/echo_client.ts | 17 +++-- test/test-application/src/index.ts | 42 ++++------- test/unit/grpc-fallback.ts | 23 ++++-- 13 files changed, 93 insertions(+), 174 deletions(-) delete mode 100644 src/fallbackProto.ts diff --git a/client-libraries.md b/client-libraries.md index 47d115672..d64cea055 100644 --- a/client-libraries.md +++ b/client-libraries.md @@ -99,31 +99,20 @@ binary tranport based on HTTP/2. It's Node.js implementation, #### HTTP/1.1 REST API mode -- `options.fallback`: `true`, `"rest"`, or `false`, use HTTP fallback mode. +- `options.fallback`: `true` or `false`, use HTTP fallback mode. Default value is `false`, unless the `window` object is defined. + For compatibility, you can pass any non-empty string, it will be considered + a `true` value. If you need to use the client library in non-Node.js environment or when gRPC cannot be used for any reason, you can use the HTTP/1.1 fallback mode. In this mode, a special browser-compatible transport implementation is used instead of -gRPC transport. - -There are two supported gRPC fallback modes: - -- set `options.fallback` to `"rest"`: the library will send and receive JSON - payload, HTTP/1.1 REST API endpoints will be used. This mode is recommended - for fallback. - -- set `options.fallback` to `true`: the library will send and receive serialized - protobuf payload to special endpoints accepting serialized protobufs over - HTTP/1.1. +gRPC transport. It will send and receive JSONs over HTTP. In browser context (if the `window` object is defined) the fallback mode is enabled automatically; set `options.fallback` to `false` if you need to override this behavior. -Note that `options.fallback` accepts boolean values (`true` and `false`) for -compatibility only. We recommend using `"rest"` to use HTTP/1.1 instead of gRPC. - ## Calling API methods In all examples below we assume that `client` is an instance of the client diff --git a/src/fallback.ts b/src/fallback.ts index 0f3046e66..cbc49dbff 100644 --- a/src/fallback.ts +++ b/src/fallback.ts @@ -35,7 +35,6 @@ import {GaxCall, GRPCCall} from './apitypes'; import {Descriptor, StreamDescriptor} from './descriptor'; import {createApiCall as _createApiCall} from './createApiCall'; import {FallbackServiceError} from './googleError'; -import * as fallbackProto from './fallbackProto'; import * as fallbackRest from './fallbackRest'; import {isNodeJS} from './featureDetection'; import {generateServiceStub} from './fallbackServiceStub'; @@ -94,7 +93,7 @@ export type AuthClient = export class GrpcClient { auth?: OAuth2Client | GoogleAuth; authClient?: AuthClient; - fallback: boolean | 'rest' | 'proto'; + fallback: boolean; grpcVersion: string; private static protoCache = new Map(); httpRules?: Array; @@ -119,7 +118,11 @@ export class GrpcClient { constructor( options: (GrpcClientOptions | {auth: OAuth2Client}) & { - fallback?: boolean | 'rest' | 'proto'; + /** + * Fallback mode to use instead of gRPC. + * A string is accepted for compatibility, all non-empty string values enable the HTTP REST fallback. + */ + fallback?: boolean | string; } = {} ) { if (!isNodeJS()) { @@ -135,7 +138,7 @@ export class GrpcClient { (options.auth as GoogleAuth) || new GoogleAuth(options as GoogleAuthOptions); } - this.fallback = options.fallback !== 'rest' ? 'proto' : 'rest'; + this.fallback = options.fallback ? true : false; this.grpcVersion = require('../../package.json').version; this.httpRules = (options as GrpcClientOptions).httpRules; this.numericEnums = (options as GrpcClientOptions).numericEnums ?? false; @@ -316,14 +319,8 @@ export class GrpcClient { servicePort = 443; } - const encoder = - this.fallback === 'rest' - ? fallbackRest.encodeRequest - : fallbackProto.encodeRequest; - const decoder = - this.fallback === 'rest' - ? fallbackRest.decodeResponse - : fallbackProto.decodeResponse; + const encoder = fallbackRest.encodeRequest; + const decoder = fallbackRest.decodeResponse; const serviceStub = generateServiceStub( methods, protocol, @@ -362,7 +359,7 @@ export class GrpcClient { export function lro(options: GrpcClientOptions) { options = Object.assign({scopes: []}, options); if (options.protoJson) { - options = Object.assign(options, {fallback: 'rest'}); + options = Object.assign(options, {fallback: true}); } const gaxGrpc = new GrpcClient(options); return new OperationsClientBuilder(gaxGrpc, options.protoJson); @@ -396,10 +393,10 @@ export function createApiCall( func: Promise | GRPCCall, settings: gax.CallSettings, descriptor?: Descriptor, - fallback?: boolean | 'proto' | 'rest' + // eslint-disable-next-line @typescript-eslint/no-unused-vars + _fallback?: boolean | string // unused; for compatibility only ): GaxCall { if ( - (!fallback || fallback === 'rest') && descriptor && 'streaming' in descriptor && (descriptor as StreamDescriptor).type !== StreamType.SERVER_STREAMING @@ -410,14 +407,10 @@ export function createApiCall( ); }; } - if ( - (fallback === 'proto' || fallback === true) && // for legacy reasons, fallback === true means 'proto' - descriptor && - 'streaming' in descriptor - ) { + if (descriptor && 'streaming' in descriptor && !isNodeJS()) { return () => { throw new Error( - 'The gRPC-fallback (proto over HTTP) transport currently does not support streaming calls.' + 'Server streaming over the REST transport is only supported in Node.js.' ); }; } diff --git a/src/fallbackProto.ts b/src/fallbackProto.ts deleted file mode 100644 index bfaa5fc10..000000000 --- a/src/fallbackProto.ts +++ /dev/null @@ -1,71 +0,0 @@ -/** - * Copyright 2021 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -// proto-over-HTTP request encoding and decoding - -import {defaultToObjectOptions} from './fallback'; -import {FetchParameters} from './fallbackServiceStub'; -import {GoogleErrorDecoder} from './googleError'; - -export function encodeRequest( - rpc: protobuf.Method, - protocol: string, - servicePath: string, - servicePort: number, - request: {} -): FetchParameters { - const protoNamespaces: string[] = []; - let currNamespace = rpc.parent!; - while (currNamespace.name !== '') { - protoNamespaces.unshift(currNamespace.name); - currNamespace = currNamespace.parent!; - } - const protoServiceName = protoNamespaces.join('.'); - const rpcName = rpc.name; - - const headers: {[key: string]: string} = { - 'Content-Type': 'application/x-protobuf', - }; - - const method = 'POST'; - const requestMessage = rpc.resolvedRequestType!.fromObject(request); - const body = rpc.resolvedRequestType!.encode(requestMessage).finish(); - const url = `${protocol}://${servicePath}:${servicePort}/$rpc/${protoServiceName}/${rpcName}`; - - return { - method, - url, - headers, - body, - }; -} - -export function decodeResponse( - rpc: protobuf.Method, - ok: boolean, - response: Buffer | ArrayBuffer -): {} { - if (!ok) { - const statusDecoder = new GoogleErrorDecoder(); - const error = statusDecoder.decodeErrorFromBuffer(response); - throw error; - } - - const buffer = - response instanceof ArrayBuffer ? new Uint8Array(response) : response; - const message = rpc.resolvedResponseType!.decode(buffer); - return rpc.resolvedResponseType!.toObject(message, defaultToObjectOptions); -} diff --git a/src/paginationCalls/pageDescriptor.ts b/src/paginationCalls/pageDescriptor.ts index a4bf867b1..6bc0a9c46 100644 --- a/src/paginationCalls/pageDescriptor.ts +++ b/src/paginationCalls/pageDescriptor.ts @@ -81,7 +81,11 @@ export class PageDescriptor implements Descriptor { // emit full api response with every page. stream.emit('response', apiResp); for (let i = 0; i < resources.length; ++i) { - if ((stream as any)._readableState.ended) { + // TODO: rewrite without accessing stream internals + if ( + (stream as unknown as {_readableState: {ended: boolean}}) + ._readableState.ended + ) { return; } if (resources[i] === null) { @@ -93,7 +97,11 @@ export class PageDescriptor implements Descriptor { stream.end(); } } - if ((stream as any)._readableState.ended) { + // TODO: rewrite without accessing stream internals + if ( + (stream as unknown as {_readableState: {ended: boolean}})._readableState + .ended + ) { return; } if (!next) { diff --git a/src/streamArrayParser.ts b/src/streamArrayParser.ts index 697dc2521..7aa03c1f4 100644 --- a/src/streamArrayParser.ts +++ b/src/streamArrayParser.ts @@ -116,7 +116,7 @@ export class StreamArrayParser extends Transform { chunk.slice(objectStart, curIndex + 1), ]); try { - // HTTP reponse.ok is true. + // HTTP response.ok is true. const msgObj = decodeResponse(this.rpc, true, objBuff); this.push(msgObj); } catch (err) { diff --git a/test/browser-test/karma.conf.js b/test/browser-test/karma.conf.js index f34eacc0c..5754262b1 100644 --- a/test/browser-test/karma.conf.js +++ b/test/browser-test/karma.conf.js @@ -86,7 +86,9 @@ module.exports = function (config) { base: 'ChromeHeadless', // We must disable the Chrome sandbox when running Chrome inside Docker (Chrome's sandbox needs // more permissions than Docker allows by default) - flags: isDocker ? ['--no-sandbox'] : [], + flags: isDocker + ? ['--no-sandbox', '--disable-web-security'] + : ['--disable-web-security'], }, }, diff --git a/test/browser-test/package.json b/test/browser-test/package.json index f98e7d39b..5171769db 100644 --- a/test/browser-test/package.json +++ b/test/browser-test/package.json @@ -7,6 +7,9 @@ "files": [ "build/test" ], + "engines": { + "node": ">=14" + }, "license": "Apache-2.0", "keywords": [], "scripts": { diff --git a/test/browser-test/test/test.endtoend.ts b/test/browser-test/test/test.endtoend.ts index f07cb3213..1d9556dbc 100644 --- a/test/browser-test/test/test.endtoend.ts +++ b/test/browser-test/test/test.endtoend.ts @@ -41,7 +41,7 @@ describe('Run tests against gRPC server', () => { const opts = { auth: authStub as unknown as GoogleAuth, protocol: 'http', - port: 1337, + port: 7469, }; // eslint-disable-next-line @typescript-eslint/ban-ts-comment @@ -87,8 +87,11 @@ describe('Run tests against gRPC server', () => { const words = ['nobody', 'ever', 'reads', 'test', 'input']; const request = { content: words.join(' '), + pageSize: 2, }; - assert.throws(() => client.expand(request)); + assert.throws(() => { + client.expand(request); + }); }); it('should be able to call paging calls', async () => { diff --git a/test/browser-test/test/test.grpc-fallback.ts b/test/browser-test/test/test.grpc-fallback.ts index d9c9133b3..6cf85e725 100644 --- a/test/browser-test/test/test.grpc-fallback.ts +++ b/test/browser-test/test/test.grpc-fallback.ts @@ -23,7 +23,6 @@ import {protobuf, GoogleAuth, fallback} from 'google-gax'; import {EchoClient} from 'showcase-echo-client'; import echoProtoJson = require('showcase-echo-client/build/protos/protos.json'); -import statusProtoJson = require('google-gax/build/protos/status.json'); const authStub = { getClient: async () => { @@ -193,12 +192,13 @@ describe('grpc-fallback', () => { it('should make a request', async () => { const client = new EchoClient(opts); const requestObject = {content: 'test-content'}; - const responseType = protos.lookupType('EchoResponse'); - const response = responseType.create(requestObject); // request === response for EchoService + const response = requestObject; // response == request for Echo const fakeFetch = sinon.fake.resolves({ ok: true, arrayBuffer: () => { - return Promise.resolve(responseType.encode(response).finish()); + return Promise.resolve( + new TextEncoder().encode(JSON.stringify(response)) + ); }, }); // eslint-disable-next-line no-undef @@ -233,8 +233,7 @@ describe('grpc-fallback', () => { fallback.routingHeader.fromParams({ abc: 'def', }); - const responseType = protos.lookupType('EchoResponse'); - const response = responseType.create(requestObject); + const response = requestObject; // eslint-disable-next-line no-undef const savedFetch = window.fetch; // @ts-ignore @@ -245,7 +244,9 @@ describe('grpc-fallback', () => { return Promise.resolve({ ok: true, arrayBuffer: () => { - return Promise.resolve(responseType.encode(response).finish()); + return Promise.resolve( + new TextEncoder().encode(JSON.stringify(response)) + ); }, }); }; @@ -257,20 +258,17 @@ describe('grpc-fallback', () => { it('should handle an error', done => { const requestObject = {content: 'test-content'}; - // example of an actual google.rpc.Status error message returned by Language - // API const expectedError = Object.assign(new Error('Error message'), { - code: 3, + code: 400, statusDetails: [], }); const fakeFetch = sinon.fake.resolves({ ok: false, arrayBuffer: () => { - const root = protobuf.Root.fromJSON(statusProtoJson); - const statusType = root.lookupType('google.rpc.Status'); - const statusMessage = statusType.fromObject(expectedError); - return Promise.resolve(statusType.encode(statusMessage).finish()); + return Promise.resolve( + new TextEncoder().encode(JSON.stringify(expectedError)) + ); }, }); // eslint-disable-next-line no-undef @@ -279,8 +277,6 @@ describe('grpc-fallback', () => { gaxGrpc.createStub(echoService, stubOptions).then(echoStub => { echoStub.echo(requestObject, {}, {}, (err?: Error) => { assert(err instanceof Error); - assert.strictEqual(err.message, '3 INVALID_ARGUMENT: Error message'); - assert.strictEqual(JSON.stringify(err), JSON.stringify(expectedError)); done(); }); }); diff --git a/test/showcase-echo-client/package.json b/test/showcase-echo-client/package.json index 5a1f8a808..6c9213210 100644 --- a/test/showcase-echo-client/package.json +++ b/test/showcase-echo-client/package.json @@ -30,7 +30,7 @@ "scripts": { "compile": "tsc -p . && cp -r protos build/", "compile-protos": "compileProtos src", - "prefetch": "rm -rf node_modules package-lock.json google-gax*.tgz && cd ../.. && npm pack && mv google-gax*.tgz test/showcase-echo-client/google-gax.tgz", + "prefetch": "rm -rf node_modules package-lock.json google-gax*.tgz gapic-tools*.tgz && cd ../.. && npm pack && mv google-gax*.tgz test/showcase-echo-client/google-gax.tgz && cd tools && npm install && npm pack && mv gapic-tools*.tgz ../test/showcase-echo-client/gapic-tools.tgz", "prepare": "npm run compile-protos && npm run compile" }, "dependencies": { @@ -38,7 +38,7 @@ }, "devDependencies": { "@types/node": "^16.0.0", - "gapic-tools": "^0.1.7", + "gapic-tools": "./gapic-tools.tgz", "typescript": "^4.5.5" }, "engines": { diff --git a/test/showcase-echo-client/src/v1beta1/echo_client.ts b/test/showcase-echo-client/src/v1beta1/echo_client.ts index 898c84887..5ba299f35 100644 --- a/test/showcase-echo-client/src/v1beta1/echo_client.ts +++ b/test/showcase-echo-client/src/v1beta1/echo_client.ts @@ -166,10 +166,10 @@ export class EchoClient { } else { clientHeader.push(`gl-web/${this._gaxModule.version}`); } - if (!opts.fallback) { - clientHeader.push(`grpc/${this._gaxGrpc.grpcVersion}`); - } else if (opts.fallback === 'rest') { + if (opts.fallback) { clientHeader.push(`rest/${this._gaxGrpc.grpcVersion}`); + } else { + clientHeader.push(`grpc/${this._gaxGrpc.grpcVersion}`); } if (opts.libName && opts.libVersion) { clientHeader.push(`${opts.libName}/${opts.libVersion}`); @@ -224,15 +224,18 @@ export class EchoClient { this.descriptors.stream = { expand: new this._gaxModule.StreamDescriptor( gax.StreamType.SERVER_STREAMING, - opts.fallback === 'rest' + // legacy: opts.fallback can be a string or a boolean + opts.fallback ? true : false ), collect: new this._gaxModule.StreamDescriptor( gax.StreamType.CLIENT_STREAMING, - opts.fallback === 'rest' + // legacy: opts.fallback can be a string or a boolean + opts.fallback ? true : false ), chat: new this._gaxModule.StreamDescriptor( gax.StreamType.BIDI_STREAMING, - opts.fallback === 'rest' + // legacy: opts.fallback can be a string or a boolean + opts.fallback ? true : false ), }; @@ -244,7 +247,7 @@ export class EchoClient { auth: this.auth, grpc: 'grpc' in this._gaxGrpc ? this._gaxGrpc.grpc : undefined, }; - if (opts.fallback === 'rest') { + if (opts.fallback) { lroOptions.protoJson = protoFilesRoot; lroOptions.httpRules = [ { diff --git a/test/test-application/src/index.ts b/test/test-application/src/index.ts index cd6a9cb8c..7642dc242 100644 --- a/test/test-application/src/index.ts +++ b/test/test-application/src/index.ts @@ -41,14 +41,14 @@ async function testShowcase() { }, } as unknown as GoogleAuth; - const fallbackClientOpts = { + const restClientOpts = { fallback: true, protocol: 'http', - port: 1337, + port: 7469, auth: fakeGoogleAuth, }; - const restClientOpts = { + const restClientOptsCompat = { fallback: 'rest' as const, protocol: 'http', port: 7469, @@ -56,8 +56,8 @@ async function testShowcase() { }; const grpcClient = new EchoClient(grpcClientOpts); - const fallbackClient = new EchoClient(fallbackClientOpts); const restClient = new EchoClient(restClientOpts); + const restClientCompat = new EchoClient(restClientOptsCompat); // assuming gRPC server is started locally await testEcho(grpcClient); @@ -69,15 +69,6 @@ async function testShowcase() { await testChat(grpcClient); await testWait(grpcClient); - await testEcho(fallbackClient); - await testEchoError(fallbackClient); - await testExpandThrows(fallbackClient); // fallback does not support server streaming - await testPagedExpand(fallbackClient); - await testPagedExpandAsync(fallbackClient); - await testCollectThrows(fallbackClient); // fallback does not support client streaming - await testChatThrows(fallbackClient); // fallback does not support bidi streaming - await testWait(fallbackClient); - await testEcho(restClient); await testExpand(restClient); // REGAPIC supports server streaming await testPagedExpand(restClient); @@ -85,6 +76,14 @@ async function testShowcase() { await testCollectThrows(restClient); // REGAPIC does not support client streaming await testChatThrows(restClient); // REGAPIC does not support bidi streaming await testWait(restClient); + + await testEcho(restClientCompat); + await testExpand(restClientCompat); // REGAPIC supports server streaming + await testPagedExpand(restClientCompat); + await testPagedExpandAsync(restClientCompat); + await testCollectThrows(restClientCompat); // REGAPIC does not support client streaming + await testChatThrows(restClientCompat); // REGAPIC does not support bidi streaming + await testWait(restClientCompat); } async function testEcho(client: EchoClient) { @@ -182,23 +181,6 @@ async function testExpand(client: EchoClient) { }); } -async function testExpandThrows(client: EchoClient) { - const words = ['nobody', 'ever', 'reads', 'test', 'input']; - const request = { - content: words.join(' '), - }; - assert.throws(() => { - const stream = client.expand(request); - const result: string[] = []; - stream.on('data', (response: {content: string}) => { - result.push(response.content); - }); - stream.on('end', () => { - assert.deepStrictEqual(words, result); - }); - }, /currently does not support/); -} - async function testPagedExpand(client: EchoClient) { const words = ['nobody', 'ever', 'reads', 'test', 'input']; const request = { diff --git a/test/unit/grpc-fallback.ts b/test/unit/grpc-fallback.ts index e386c5eb6..53abfdb99 100644 --- a/test/unit/grpc-fallback.ts +++ b/test/unit/grpc-fallback.ts @@ -19,8 +19,6 @@ import * as assert from 'assert'; import {describe, it, beforeEach, afterEach, before, after} from 'mocha'; -import * as path from 'path'; -import * as fs from 'fs'; import * as nodeFetch from 'node-fetch'; import * as abortController from 'abort-controller'; import * as protobuf from 'protobufjs'; @@ -267,7 +265,7 @@ describe('grpc-fallback', () => { Promise.resolve({ ok: true, arrayBuffer: () => { - return Promise.resolve(responseType.encode(response).finish()); + return Promise.resolve(Buffer.from(JSON.stringify(response))); }, }) ); @@ -287,10 +285,23 @@ describe('grpc-fallback', () => { it('should handle an API error', done => { const requestObject = {content: 'test-content'}; // example of an actual google.rpc.Status error message returned by Language API - const fixtureName = path.resolve(__dirname, '..', 'fixtures', 'error.bin'); - const errorBin = fs.readFileSync(fixtureName); const expectedMessage = '3 INVALID_ARGUMENT: One of content, or gcs_content_uri must be set.'; + const jsonError = { + code: 400, // Bad request + message: expectedMessage, + details: [ + { + '@type': 'type.googleapis.com/google.rpc.BadRequest', + fieldViolations: [ + { + field: 'document.content', + description: 'Must have some text content to annotate.', + }, + ], + }, + ], + }; const expectedError = { code: 3, details: [ @@ -309,7 +320,7 @@ describe('grpc-fallback', () => { Promise.resolve({ ok: false, arrayBuffer: () => { - return Promise.resolve(errorBin); + return Promise.resolve(Buffer.from(JSON.stringify(jsonError))); }, }) );