Skip to content

Commit

Permalink
fix: replace proto-over-HTTP with REGAPIC (#1471)
Browse files Browse the repository at this point in the history
  • Loading branch information
alexander-fenster authored Jul 13, 2023
1 parent c1c4dc1 commit 4266f43
Show file tree
Hide file tree
Showing 13 changed files with 93 additions and 174 deletions.
19 changes: 4 additions & 15 deletions client-libraries.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 14 additions & 21 deletions src/fallback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<string, protobuf.Root>();
httpRules?: Array<google.api.IHttpRule>;
Expand All @@ -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()) {
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -396,10 +393,10 @@ export function createApiCall(
func: Promise<GRPCCall> | 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
Expand All @@ -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.'
);
};
}
Expand Down
71 changes: 0 additions & 71 deletions src/fallbackProto.ts

This file was deleted.

12 changes: 10 additions & 2 deletions src/paginationCalls/pageDescriptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/streamArrayParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 3 additions & 1 deletion test/browser-test/karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
},
},

Expand Down
3 changes: 3 additions & 0 deletions test/browser-test/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
"files": [
"build/test"
],
"engines": {
"node": ">=14"
},
"license": "Apache-2.0",
"keywords": [],
"scripts": {
Expand Down
7 changes: 5 additions & 2 deletions test/browser-test/test/test.endtoend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 () => {
Expand Down
28 changes: 12 additions & 16 deletions test/browser-test/test/test.grpc-fallback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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))
);
},
});
};
Expand All @@ -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
Expand All @@ -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();
});
});
Expand Down
4 changes: 2 additions & 2 deletions test/showcase-echo-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@
"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": {
"google-gax": "./google-gax.tgz"
},
"devDependencies": {
"@types/node": "^16.0.0",
"gapic-tools": "^0.1.7",
"gapic-tools": "./gapic-tools.tgz",
"typescript": "^4.5.5"
},
"engines": {
Expand Down
Loading

0 comments on commit 4266f43

Please sign in to comment.